Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-introduces internal fsapi.File with non-blocking methods #1613

Merged
merged 1 commit into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions experimental/sys/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,6 @@ func (DirFile) SetAppend(bool) Errno {
return EISDIR
}

// IsNonblock implements File.IsNonblock
func (DirFile) IsNonblock() bool {
return false
}

// SetNonblock implements File.SetNonblock
func (DirFile) SetNonblock(bool) Errno {
return EISDIR
}

// IsDir implements File.IsDir
func (DirFile) IsDir() (bool, Errno) {
return true, 0
Expand All @@ -86,11 +76,6 @@ func (DirFile) Pread([]byte, int64) (int, Errno) {
return 0, EISDIR
}

// Poll implements File.Poll
func (DirFile) Poll(Pflag, int32) (ready bool, errno Errno) {
return false, ENOSYS
}

// Write implements File.Write
func (DirFile) Write([]byte) (int, Errno) {
return 0, EISDIR
Expand Down
54 changes: 0 additions & 54 deletions experimental/sys/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,6 @@ type File interface {
// - Implementations should cache this result.
IsDir() (bool, Errno)

// IsNonblock returns true if the file was opened with O_NONBLOCK, or
// SetNonblock was successfully enabled on this file.
//
// # Notes
//
// - This might not match the underlying state of the file descriptor if
// the file was not opened via OpenFile.
IsNonblock() bool

// SetNonblock toggles the non-blocking mode (O_NONBLOCK) of this file.
//
// # Errors
//
// A zero Errno is success. The below are expected otherwise:
// - ENOSYS: the implementation does not support this function.
// - EBADF: the file or directory was closed.
//
// # Notes
//
// - This is like syscall.SetNonblock and `fcntl` with O_NONBLOCK in
// POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
SetNonblock(enable bool) Errno

// IsAppend returns true if the file was opened with O_APPEND, or
// SetAppend was successfully enabled on this file.
//
Expand Down Expand Up @@ -200,37 +177,6 @@ type File interface {
// of io.Seeker. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/fseek.html
Seek(offset int64, whence int) (newOffset int64, errno Errno)

// Poll returns if the file has data ready to be read or written.
//
// # Parameters
//
// The `flag` parameter determines which event to await, such as POLLIN,
// POLLOUT, or a combination like `POLLIN|POLLOUT`.
//
// The `timeoutMillis` parameter is how long to block for an event, or
// interrupted, in milliseconds. There are two special values:
// - zero returns immediately
// - any negative value blocks any amount of time
//
// # Results
//
// `ready` means there was data ready to read or written. False can mean no
// event was ready or `errno` is not zero.
//
// A zero `errno` is success. The below are expected otherwise:
// - ENOSYS: the implementation does not support this function.
// - ENOTSUP: the implementation does not the flag combination.
// - EINTR: the call was interrupted prior to an event.
//
// # Notes
//
// - This is like `poll` in POSIX, for a single file.
// See https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html
// - No-op files, such as those which read from /dev/null, should return
// immediately true, as data will never become available.
// - See /RATIONALE.md for detailed notes including impact of blocking.
Poll(flag Pflag, timeoutMillis int32) (ready bool, errno Errno)

// Readdir reads the contents of the directory associated with file and
// returns a slice of up to n Dirent values in an arbitrary order. This is
// a stateful function, so subsequent calls return any next values.
Expand Down
15 changes: 0 additions & 15 deletions experimental/sys/unimplemented.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,6 @@ func (UnimplementedFile) SetAppend(bool) Errno {
return ENOSYS
}

// IsNonblock implements File.IsNonblock
func (UnimplementedFile) IsNonblock() bool {
return false
}

// SetNonblock implements File.SetNonblock
func (UnimplementedFile) SetNonblock(bool) Errno {
return ENOSYS
}

// Stat implements File.Stat
func (UnimplementedFile) Stat() (sys.Stat_t, Errno) {
return sys.Stat_t{}, ENOSYS
Expand All @@ -136,11 +126,6 @@ func (UnimplementedFile) Readdir(int) (dirents []Dirent, errno Errno) {
return nil, ENOSYS
}

// Poll implements File.Poll
func (UnimplementedFile) Poll(Pflag, int32) (ready bool, errno Errno) {
return false, ENOSYS
}

// Write implements File.Write
func (UnimplementedFile) Write([]byte) (int, Errno) {
return 0, ENOSYS
Expand Down
3 changes: 2 additions & 1 deletion imports/wasi_snapshot_preview1/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
experimentalsys "github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/internal/fsapi"
"github.com/tetratelabs/wazero/internal/fstest"
"github.com/tetratelabs/wazero/internal/platform"
"github.com/tetratelabs/wazero/internal/sys"
Expand Down Expand Up @@ -247,7 +248,7 @@ func Test_fdFdstatGet(t *testing.T) {
}}}).OpenFile("stdin", 0, 0)
require.EqualErrno(t, 0, errno)

stdin.File = stdinFile
stdin.File = fsapi.Adapt(stdinFile)

// Make this file writeable, to ensure flags read-back correctly.
fileFD, errno := fsc.OpenFile(preopen, file, experimentalsys.O_RDWR, 0)
Expand Down
3 changes: 2 additions & 1 deletion imports/wasi_snapshot_preview1/poll.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/internal/fsapi"
internalsys "github.com/tetratelabs/wazero/internal/sys"
"github.com/tetratelabs/wazero/internal/wasip1"
"github.com/tetratelabs/wazero/internal/wasm"
Expand Down Expand Up @@ -177,7 +178,7 @@ func pollOneoffFn(_ context.Context, mod api.Module, params []uint64) sys.Errno
}
// Wait for the timeout to expire, or for some data to become available on Stdin.

if stdinReady, errno := stdin.File.Poll(sys.POLLIN, int32(timeout.Milliseconds())); errno != 0 {
if stdinReady, errno := stdin.File.Poll(fsapi.POLLIN, int32(timeout.Milliseconds())); errno != 0 {
return errno
} else if stdinReady {
// stdin has data ready to for reading, write back all the events
Expand Down
13 changes: 7 additions & 6 deletions imports/wasi_snapshot_preview1/poll_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
experimentalsys "github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/internal/fsapi"
"github.com/tetratelabs/wazero/internal/sys"
"github.com/tetratelabs/wazero/internal/testing/require"
"github.com/tetratelabs/wazero/internal/wasip1"
Expand Down Expand Up @@ -161,7 +162,7 @@ func Test_pollOneoff_Stdin(t *testing.T) {
name string
in, out, nsubscriptions, resultNevents uint32
mem []byte // at offset in
stdin experimentalsys.File
stdin fsapi.File
expectedErrno wasip1.Errno
expectedMem []byte // at offset out
expectedLog string
Expand Down Expand Up @@ -442,7 +443,7 @@ func Test_pollOneoff_Stdin(t *testing.T) {
}
}

func setStdin(t *testing.T, mod api.Module, stdin experimentalsys.File) {
func setStdin(t *testing.T, mod api.Module, stdin fsapi.File) {
fsc := mod.(*wasm.ModuleInstance).Sys.FS()
f, ok := fsc.LookupFile(sys.FdStdin)
require.True(t, ok)
Expand Down Expand Up @@ -613,8 +614,8 @@ type neverReadyTtyStdinFile struct {
}

// Poll implements the same method as documented on sys.File
func (neverReadyTtyStdinFile) Poll(flag experimentalsys.Pflag, timeoutMillis int32) (ready bool, errno experimentalsys.Errno) {
if flag != experimentalsys.POLLIN {
func (neverReadyTtyStdinFile) Poll(flag fsapi.Pflag, timeoutMillis int32) (ready bool, errno experimentalsys.Errno) {
if flag != fsapi.POLLIN {
return false, experimentalsys.ENOTSUP
}
switch {
Expand All @@ -632,8 +633,8 @@ type pollStdinFile struct {
}

// Poll implements the same method as documented on sys.File
func (p *pollStdinFile) Poll(flag experimentalsys.Pflag, timeoutMillis int32) (ready bool, errno experimentalsys.Errno) {
if flag != experimentalsys.POLLIN {
func (p *pollStdinFile) Poll(flag fsapi.Pflag, timeoutMillis int32) (ready bool, errno experimentalsys.Errno) {
if flag != fsapi.POLLIN {
return false, experimentalsys.ENOTSUP
}
return p.ready, 0
Expand Down
4 changes: 2 additions & 2 deletions imports/wasi_snapshot_preview1/wasi_stdlib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import (
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
experimentalsock "github.com/tetratelabs/wazero/experimental/sock"
experimentalsys "github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
"github.com/tetratelabs/wazero/internal/fsapi"
"github.com/tetratelabs/wazero/internal/fstest"
internalsys "github.com/tetratelabs/wazero/internal/sys"
"github.com/tetratelabs/wazero/internal/testing/require"
Expand Down Expand Up @@ -329,7 +329,7 @@ func Test_Poll(t *testing.T) {
tests := []struct {
name string
args []string
stdin experimentalsys.File
stdin fsapi.File
expectedOutput string
expectedTimeout time.Duration
}{
Expand Down
69 changes: 69 additions & 0 deletions internal/fsapi/file.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package fsapi

import experimentalsys "github.com/tetratelabs/wazero/experimental/sys"

// File includes methods not yet ready to document for end users, notably
// non-blocking functionality.
//
// Particularly, Poll is subject to debate. For example, whether a user should
// be able to choose how to implement timeout or not. Currently, this interface
// allows the user to choose to sleep or use native polling, and which choice
// they make impacts thread behavior as summarized here:
// https://github.com/tetratelabs/wazero/pull/1606#issuecomment-1665475516
type File interface {
experimentalsys.File

// IsNonblock returns true if the file was opened with O_NONBLOCK, or
// SetNonblock was successfully enabled on this file.
//
// # Notes
//
// - This might not match the underlying state of the file descriptor if
// the file was not opened via OpenFile.
IsNonblock() bool

// SetNonblock toggles the non-blocking mode (O_NONBLOCK) of this file.
//
// # Errors
//
// A zero Errno is success. The below are expected otherwise:
// - ENOSYS: the implementation does not support this function.
// - EBADF: the file or directory was closed.
//
// # Notes
//
// - This is like syscall.SetNonblock and `fcntl` with O_NONBLOCK in
// POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
SetNonblock(enable bool) experimentalsys.Errno

// Poll returns if the file has data ready to be read or written.
//
// # Parameters
//
// The `flag` parameter determines which event to await, such as POLLIN,
// POLLOUT, or a combination like `POLLIN|POLLOUT`.
//
// The `timeoutMillis` parameter is how long to block for an event, or
// interrupted, in milliseconds. There are two special values:
// - zero returns immediately
// - any negative value blocks any amount of time
//
// # Results
//
// `ready` means there was data ready to read or written. False can mean no
// event was ready or `errno` is not zero.
//
// A zero `errno` is success. The below are expected otherwise:
// - ENOSYS: the implementation does not support this function.
// - ENOTSUP: the implementation does not the flag combination.
// - EINTR: the call was interrupted prior to an event.
//
// # Notes
//
// - This is like `poll` in POSIX, for a single file.
// See https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html
// - No-op files, such as those which read from /dev/null, should return
// immediately true, as data will never become available.
// - See /RATIONALE.md for detailed notes including impact of blocking.
Poll(flag Pflag, timeoutMillis int32) (ready bool, errno experimentalsys.Errno)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the thing that tripped the whole debate was me noticing a PR that basically obviated this parameter based on concerns about blocking. Rather than get through that debate, which seems still murky, this moves the whole feature set internal. This allows users to still supply filesystems even though there remain plenty of blocking methods, such as Read.

}
2 changes: 1 addition & 1 deletion experimental/sys/poll.go → internal/fsapi/poll.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package sys
package fsapi

// Pflag are bit flags used for File.Poll. Values, including zero, should not
// be interpreted numerically. Instead, use by constants prefixed with 'POLL'.
Expand Down
27 changes: 27 additions & 0 deletions internal/fsapi/unimplemented.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package fsapi

import experimentalsys "github.com/tetratelabs/wazero/experimental/sys"

func Adapt(f experimentalsys.File) File {
if f, ok := f.(File); ok {
return f
}
return unimplementedFile{f}
}

type unimplementedFile struct{ experimentalsys.File }

// IsNonblock implements File.IsNonblock
func (unimplementedFile) IsNonblock() bool {
return false
}

// SetNonblock implements File.SetNonblock
func (unimplementedFile) SetNonblock(bool) experimentalsys.Errno {
return experimentalsys.ENOSYS
}

// Poll implements File.Poll
func (unimplementedFile) Poll(Pflag, int32) (ready bool, errno experimentalsys.Errno) {
return false, experimentalsys.ENOSYS
}
21 changes: 12 additions & 9 deletions internal/sys/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/internal/descriptor"
"github.com/tetratelabs/wazero/internal/fsapi"
socketapi "github.com/tetratelabs/wazero/internal/sock"
"github.com/tetratelabs/wazero/internal/sysfs"
)
Expand Down Expand Up @@ -50,7 +51,7 @@ type FileEntry struct {
FS sys.FS

// File is always non-nil.
File sys.File
File fsapi.File

// direntCache is nil until DirentCache was called.
direntCache *DirentCache
Expand Down Expand Up @@ -287,7 +288,7 @@ func (c *FSContext) OpenFile(fs sys.FS, path string, flag sys.Oflag, perm fs.Fil
if f, errno := fs.OpenFile(path, flag, perm); errno != 0 {
return 0, errno
} else {
fe := &FileEntry{FS: fs, File: f}
fe := &FileEntry{FS: fs, File: fsapi.Adapt(f)}
if path == "/" || path == "." {
fe.Name = ""
} else {
Expand Down Expand Up @@ -339,18 +340,20 @@ func (c *FSContext) SockAccept(sockFD int32, nonblock bool) (int32, sys.Errno) {
return 0, sys.EBADF // Not a sock
}

var conn socketapi.TCPConn
var errno sys.Errno
if conn, errno = sock.Accept(); errno != 0 {
conn, errno := sock.Accept()
if errno != 0 {
return 0, errno
} else if nonblock {
if errno = conn.SetNonblock(true); errno != 0 {
}

fe := &FileEntry{File: fsapi.Adapt(conn)}

if nonblock {
if errno = fe.File.SetNonblock(true); errno != 0 {
_ = conn.Close()
return 0, errno
}
}

fe := &FileEntry{File: conn}
if newFD, ok := c.openedFiles.Insert(fe); !ok {
return 0, sys.EBADF
} else {
Expand Down Expand Up @@ -426,7 +429,7 @@ func (c *Context) InitFSContext(
}

for _, tl := range tcpListeners {
c.fsc.openedFiles.Insert(&FileEntry{IsPreopen: true, File: sysfs.NewTCPListenerFile(tl)})
c.fsc.openedFiles.Insert(&FileEntry{IsPreopen: true, File: fsapi.Adapt(sysfs.NewTCPListenerFile(tl))})
}
return nil
}
Expand Down
Loading
Loading