From b01ba67fdcf32997d22a0f56b723a71c1c40cd75 Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 15 Jun 2023 01:08:44 +0200 Subject: [PATCH] wasi: add nonblock_test.go from gotip, fix nonblock read on Unix-like (#1517) Signed-off-by: Edoardo Vacchi --- .../testdata/gotip/wasi.go | 60 +++++++++++ .../wasi_stdlib_unix_test.go | 101 ++++++++++++++++++ internal/sysfs/file_test.go | 50 ++++++++- internal/sysfs/file_unix.go | 21 ++++ internal/sysfs/file_unsupported.go | 12 +++ internal/sysfs/open_file_windows.go | 2 +- internal/sysfs/osfile.go | 21 ++-- 7 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 internal/sysfs/file_unix.go create mode 100644 internal/sysfs/file_unsupported.go diff --git a/imports/wasi_snapshot_preview1/testdata/gotip/wasi.go b/imports/wasi_snapshot_preview1/testdata/gotip/wasi.go index 2ff5a01961..50612cb871 100644 --- a/imports/wasi_snapshot_preview1/testdata/gotip/wasi.go +++ b/imports/wasi_snapshot_preview1/testdata/gotip/wasi.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "os" + "sync" "syscall" ) @@ -19,6 +20,10 @@ func main() { if err := mainHTTP(); err != nil { panic(err) } + case "nonblock": + if err := mainNonblock(os.Args[2], os.Args[3:]); err != nil { + panic(err) + } } } @@ -100,3 +105,58 @@ func (e echoOnce) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Once one request was served, close the channel. close(e.ch) } + +// Adapted from nonblock.go +// https://github.com/golang/go/blob/0fcc70ecd56e3b5c214ddaee4065ea1139ae16b5/src/runtime/internal/wasitest/testdata/nonblock.go +func mainNonblock(mode string, files []string) error { + ready := make(chan struct{}) + + var wg sync.WaitGroup + for _, path := range files { + f, err := os.Open(path) + if err != nil { + return err + } + switch mode { + case "open": + case "create": + fd := f.Fd() + if err = syscall.SetNonblock(int(fd), true); err != nil { + return err + } + f = os.NewFile(fd, path) + default: + return fmt.Errorf("invalid test mode") + } + + spawnWait := make(chan struct{}) + + wg.Add(1) + go func(f *os.File) { + defer f.Close() + defer wg.Done() + + // Signal the routine has been spawned. + close(spawnWait) + + // Wait until ready. + <-ready + + var buf [256]byte + + if n, err := f.Read(buf[:]); err != nil { + panic(err) + } else { + os.Stderr.Write(buf[:n]) + } + }(f) + + // Spawn one goroutine at a time. + <-spawnWait + } + + println("waiting") + close(ready) + wg.Wait() + return nil +} diff --git a/imports/wasi_snapshot_preview1/wasi_stdlib_unix_test.go b/imports/wasi_snapshot_preview1/wasi_stdlib_unix_test.go index 3c63dbcdc9..3bd04ab42d 100644 --- a/imports/wasi_snapshot_preview1/wasi_stdlib_unix_test.go +++ b/imports/wasi_snapshot_preview1/wasi_stdlib_unix_test.go @@ -3,14 +3,22 @@ package wasi_snapshot_preview1_test import ( + "bufio" + "bytes" + "fmt" + "io" + "math/rand" "os" + "path/filepath" "strings" "syscall" "testing" "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" + "github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1" "github.com/tetratelabs/wazero/internal/testing/require" + "github.com/tetratelabs/wazero/sys" ) func Test_NonblockingFile(t *testing.T) { @@ -53,3 +61,96 @@ func Test_NonblockingFile(t *testing.T) { require.True(t, strings.HasPrefix(lines[0], ".")) require.Equal(t, "wazero", lines[1]) } + +type fifo struct { + file *os.File + path string +} + +func Test_NonblockGotip(t *testing.T) { + // - Create `numFifos` FIFOs. + // - Instantiate `wasmGotip` with the names of the FIFO in the order of creation + // - The test binary opens the FIFOs in the given order and spawns a goroutine for each + // - The unit test writes to the FIFO in reverse order. + // - Each goroutine reads from the given FIFO and writes the contents to stderr + // + // The test verifies that the output order matches the write order (i.e. reverse order). + // + // If I/O was blocking, all goroutines would be blocked waiting for one read call + // to return, and the output order wouldn't match. + // + // Adapted from https://github.com/golang/go/blob/0fcc70ecd56e3b5c214ddaee4065ea1139ae16b5/src/runtime/internal/wasitest/nonblock_test.go + + if wasmGotip == nil { + t.Skip("skipping because wasi.go was not compiled (gotip missing or compilation error)") + } + const numFifos = 8 + + for _, mode := range []string{"open", "create"} { + t.Run(mode, func(t *testing.T) { + tempDir := t.TempDir() + + args := []string{"wasi", "nonblock", mode} + fifos := make([]*fifo, numFifos) + for i := range fifos { + tempFile := fmt.Sprintf("wasip1-nonblock-fifo-%d-%d", rand.Uint32(), i) + path := filepath.Join(tempDir, tempFile) + err := syscall.Mkfifo(path, 0o666) + require.NoError(t, err) + + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer file.Close() + + args = append(args, tempFile) + fifos[len(fifos)-i-1] = &fifo{file, path} + } + + pr, pw := io.Pipe() + defer pw.Close() + + var consoleBuf bytes.Buffer + + moduleConfig := wazero.NewModuleConfig(). + WithArgs(args...). + WithFSConfig( // Mount the tempDir as root. + wazero.NewFSConfig().WithDirMount(tempDir, "/")). + WithStderr(pw). // Write Stderr to pw + WithStdout(&consoleBuf). + WithStartFunctions(). + WithSysNanosleep() + + ch := make(chan string, 1) + go func() { + r := wazero.NewRuntime(testCtx) + defer r.Close(testCtx) + + _, err := wasi_snapshot_preview1.Instantiate(testCtx, r) + require.NoError(t, err) + + mod, err := r.InstantiateWithConfig(testCtx, wasmGotip, moduleConfig) // clear + require.NoError(t, err) + + _, err = mod.ExportedFunction("_start").Call(testCtx) + if exitErr, ok := err.(*sys.ExitError); ok { + require.Zero(t, exitErr.ExitCode(), consoleBuf.String()) + } + ch <- consoleBuf.String() + }() + + scanner := bufio.NewScanner(pr) + require.True(t, scanner.Scan(), fmt.Sprintf("expected line: %s", scanner.Err())) + require.Equal(t, "waiting", scanner.Text(), fmt.Sprintf("unexpected output: %s", scanner.Text())) + + for _, fifo := range fifos { + _, err := fifo.file.WriteString(fifo.path + "\n") + require.NoError(t, err) + require.True(t, scanner.Scan(), fmt.Sprintf("expected line: %s", scanner.Err())) + require.Equal(t, fifo.path, scanner.Text(), fmt.Sprintf("unexpected line: %s", scanner.Text())) + } + + s := <-ch + require.Equal(t, "", s) + }) + } +} diff --git a/internal/sysfs/file_test.go b/internal/sysfs/file_test.go index 36909681b6..3f10c61269 100644 --- a/internal/sysfs/file_test.go +++ b/internal/sysfs/file_test.go @@ -28,7 +28,7 @@ var ( emptyFile = "empty.txt" ) -func TestFileSetNonblock(t *testing.T) { +func TestStdioFileSetNonblock(t *testing.T) { // Test using os.Pipe as it is known to support non-blocking reads. r, w, err := os.Pipe() require.NoError(t, err) @@ -47,6 +47,54 @@ func TestFileSetNonblock(t *testing.T) { require.False(t, rF.IsNonblock()) } +func TestRegularFileSetNonblock(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Nonblock on regular files is not supported on Windows") + } + + // Test using os.Pipe as it is known to support non-blocking reads. + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + defer w.Close() + + rF := newOsFile("", syscall.O_RDONLY, 0, r) + + errno := rF.SetNonblock(true) + require.EqualErrno(t, 0, errno) + require.True(t, rF.IsNonblock()) + + // Read from the file without ever writing to it should not block. + buf := make([]byte, 8) + _, e := rF.Read(buf) + require.EqualErrno(t, syscall.EAGAIN, e) + + errno = rF.SetNonblock(false) + require.EqualErrno(t, 0, errno) + require.False(t, rF.IsNonblock()) +} + +func TestReadFdNonblock(t *testing.T) { + // Test using os.Pipe as it is known to support non-blocking reads. + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + defer w.Close() + + fd := r.Fd() + err = setNonblock(fd, true) + require.NoError(t, err) + + // Read from the file without ever writing to it should not block. + buf := make([]byte, 8) + _, e := readFd(fd, buf) + if runtime.GOOS == "windows" { + require.EqualErrno(t, syscall.ENOSYS, e) + } else { + require.EqualErrno(t, syscall.EAGAIN, e) + } +} + func TestFileSetAppend(t *testing.T) { tmpDir := t.TempDir() diff --git a/internal/sysfs/file_unix.go b/internal/sysfs/file_unix.go new file mode 100644 index 0000000000..e451df820b --- /dev/null +++ b/internal/sysfs/file_unix.go @@ -0,0 +1,21 @@ +//go:build unix || darwin || linux + +package sysfs + +import ( + "syscall" + + "github.com/tetratelabs/wazero/internal/platform" +) + +const NonBlockingFileIoSupported = true + +// readFd exposes syscall.Read. +func readFd(fd uintptr, buf []byte) (int, syscall.Errno) { + if len(buf) == 0 { + return 0, 0 // Short-circuit 0-len reads. + } + n, err := syscall.Read(int(fd), buf) + errno := platform.UnwrapOSError(err) + return n, errno +} diff --git a/internal/sysfs/file_unsupported.go b/internal/sysfs/file_unsupported.go new file mode 100644 index 0000000000..cb4bddb339 --- /dev/null +++ b/internal/sysfs/file_unsupported.go @@ -0,0 +1,12 @@ +//go:build !unix && !linux && !darwin + +package sysfs + +import "syscall" + +const NonBlockingFileIoSupported = false + +// readFd returns ENOSYS on unsupported platforms. +func readFd(fd uintptr, buf []byte) (int, syscall.Errno) { + return -1, syscall.ENOSYS +} diff --git a/internal/sysfs/open_file_windows.go b/internal/sysfs/open_file_windows.go index 0800dc7bad..d9297d7e8b 100644 --- a/internal/sysfs/open_file_windows.go +++ b/internal/sysfs/open_file_windows.go @@ -13,7 +13,7 @@ import ( func newOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File { return &windowsOsFile{ - osFile: osFile{path: openPath, flag: openFlag, perm: openPerm, file: f}, + osFile: osFile{path: openPath, flag: openFlag, perm: openPerm, file: f, fd: f.Fd()}, } } diff --git a/internal/sysfs/osfile.go b/internal/sysfs/osfile.go index e919a23380..95798f4820 100644 --- a/internal/sysfs/osfile.go +++ b/internal/sysfs/osfile.go @@ -12,7 +12,7 @@ import ( ) func newDefaultOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File { - return &osFile{path: openPath, flag: openFlag, perm: openPerm, file: f} + return &osFile{path: openPath, flag: openFlag, perm: openPerm, file: f, fd: f.Fd()} } // osFile is a file opened with this package, and uses os.File or syscalls to @@ -22,6 +22,7 @@ type osFile struct { flag int perm fs.FileMode file *os.File + fd uintptr // closed is true when closed was called. This ensures proper syscall.EBADF closed bool @@ -92,7 +93,7 @@ func (f *osFile) SetNonblock(enable bool) (errno syscall.Errno) { } else { f.flag &= ^fsapi.O_NONBLOCK } - if err := setNonblock(f.file.Fd(), enable); err != nil { + if err := setNonblock(f.fd, enable); err != nil { return fileError(f, f.closed, platform.UnwrapOSError(err)) } return 0 @@ -126,7 +127,15 @@ func (f *osFile) Stat() (fsapi.Stat_t, syscall.Errno) { // Read implements the same method as documented on fsapi.File func (f *osFile) Read(buf []byte) (n int, errno syscall.Errno) { - if n, errno = read(f.file, buf); errno != 0 { + if len(buf) == 0 { + return 0, 0 // Short-circuit 0-len reads. + } + if NonBlockingFileIoSupported && f.IsNonblock() { + n, errno = readFd(f.fd, buf) + } else { + n, errno = read(f.file, buf) + } + if errno != 0 { // Defer validation overhead until we've already had an error. errno = fileError(f, f.closed, errno) } @@ -160,7 +169,7 @@ func (f *osFile) Seek(offset int64, whence int) (newOffset int64, errno syscall. // PollRead implements the same method as documented on fsapi.File func (f *osFile) PollRead(timeout *time.Duration) (ready bool, errno syscall.Errno) { fdSet := platform.FdSet{} - fd := int(f.file.Fd()) + fd := int(f.fd) fdSet.Set(fd) nfds := fd + 1 // See https://man7.org/linux/man-pages/man2/select.2.html#:~:text=condition%20has%20occurred.-,nfds,-This%20argument%20should count, err := _select(nfds, &fdSet, nil, nil, timeout) @@ -232,7 +241,7 @@ func (f *osFile) Chown(uid, gid int) syscall.Errno { return syscall.EBADF } - return fchown(f.file.Fd(), uid, gid) + return fchown(f.fd, uid, gid) } // Utimens implements the same method as documented on fsapi.File @@ -241,7 +250,7 @@ func (f *osFile) Utimens(times *[2]syscall.Timespec) syscall.Errno { return syscall.EBADF } - err := futimens(f.file.Fd(), times) + err := futimens(f.fd, times) return platform.UnwrapOSError(err) }