Repository: tetratelabs/wazero
Pull request #1131 (2023-02-16 01:48:09)

wasi: implements sched_yield with sys.Osyield
This implements WASI `sched_yield` with `sys.Osyield` that defaults to return immediately. This is intentionally left without a built-in alternative as common platforms such as darwin implement `runtime.osyield` by sleeping for a microsecond. If we implemented that, user code would be slowed down without a clear reason why.

Thanks @achille-roussel and @ckaznocha for the brainstorm


Patch:
diff --git a/RATIONALE.md b/RATIONALE.md
index a6504fc2..1cb45515 100644
--- a/RATIONALE.md
+++ b/RATIONALE.md
@@ -843,6 +843,32 @@ used in the common case, even if it isn't used by Go, because this gives an
 easy and efficient closure over a common program function. We also documented
 `sys.Nanotime` to warn users that some compilers don't optimize sleep.
 
+## sys.Osyield
+
+We expose `sys.Osyield`, to allow users to control the behavior of WASI's
+`sched_yield` without a new build of wazero. This is mainly for parity with
+all other related features which we allow users to implement, including
+`sys.Nanosleep`. Unlike others, we don't provide an out-of-box implementation
+primarily because it will cause performance problems when accessed.
+
+For example, the below implementation uses CGO, which might result in a 1us
+delay per invocation depending on the platform.
+
+See https://github.com/golang/go/issues/19409#issuecomment-284788196
+```go
+//go:noescape
+//go:linkname osyield runtime.osyield
+func osyield()
+```
+
+In practice, a request to customize this is unlikely to happen until other
+thread based functions are implemented. That said, as of early 2023, there are
+a few signs of implementation interest and cross-referencing:
+
+See https://github.com/WebAssembly/stack-switching/discussions/38
+See https://github.com/WebAssembly/wasi-threads#what-can-be-skipped
+See https://slinkydeveloper.com/Kubernetes-controllers-A-New-Hope/
+
 ## Signed encoding of integer global constant initializers
 
 wazero treats integer global constant initializers signed as their interpretation is not known at declaration time. For
diff --git a/config.go b/config.go
index d39ee1cf..4f7d0fd5 100644
--- a/config.go
+++ b/config.go
@@ -554,7 +554,7 @@ type ModuleConfig interface {
 	//
 	// This example uses a custom sleep function:
 	//	moduleConfig = moduleConfig.
-	//		WithNanosleep(func(ctx context.Context, ns int64) {
+	//		WithNanosleep(func(ns int64) {
 	//			rel := unix.NsecToTimespec(ns)
 	//			remain := unix.Timespec{}
 	//			for { // loop until no more time remaining
@@ -569,6 +569,14 @@ type ModuleConfig interface {
 	//   - Use WithSysNanosleep for a usable implementation.
 	WithNanosleep(sys.Nanosleep) ModuleConfig
 
+	// WithOsyield yields the processor, typically to implement spin-wait
+	// loops. Defaults to return immediately.
+	//
+	// # Notes:
+	//   - This primarily supports `sched_yield` in WASI
+	//   - This does not default to runtime.osyield as that violates sandboxing.
+	WithOsyield(sys.Osyield) ModuleConfig
+
 	// WithSysNanosleep uses time.Sleep for sys.Nanosleep.
 	//
 	// See WithNanosleep
@@ -598,6 +606,7 @@ type moduleConfig struct {
 	nanotime           *sys.Nanotime
 	nanotimeResolution sys.ClockResolution
 	nanosleep          *sys.Nanosleep
+	osyield            *sys.Osyield
 	args               [][]byte
 	// environ is pair-indexed to retain order similar to os.Environ.
 	environ [][]byte
@@ -740,6 +749,13 @@ func (c *moduleConfig) WithNanosleep(nanosleep sys.Nanosleep) ModuleConfig {
 	return &ret
 }
 
+// WithOsyield implements ModuleConfig.WithOsyield
+func (c *moduleConfig) WithOsyield(osyield sys.Osyield) ModuleConfig {
+	ret := *c // copy
+	ret.osyield = &osyield
+	return &ret
+}
+
 // WithSysNanosleep implements ModuleConfig.WithSysNanosleep
 func (c *moduleConfig) WithSysNanosleep() ModuleConfig {
 	return c.WithNanosleep(platform.Nanosleep)
@@ -796,7 +812,7 @@ func (c *moduleConfig) toSysContext() (sysCtx *internalsys.Context, err error) {
 		c.randSource,
 		c.walltime, c.walltimeResolution,
 		c.nanotime, c.nanotimeResolution,
-		c.nanosleep,
+		c.nanosleep, c.osyield,
 		fs,
 	)
 }
diff --git a/imports/wasi_snapshot_preview1/sched.go b/imports/wasi_snapshot_preview1/sched.go
index cc97a4a1..3a135814 100644
--- a/imports/wasi_snapshot_preview1/sched.go
+++ b/imports/wasi_snapshot_preview1/sched.go
@@ -1,11 +1,21 @@
 package wasi_snapshot_preview1
 
 import (
+	"context"
+
+	"github.com/tetratelabs/wazero/api"
 	. "github.com/tetratelabs/wazero/internal/wasi_snapshot_preview1"
+	"github.com/tetratelabs/wazero/internal/wasm"
 )
 
 // schedYield is the WASI function named SchedYieldName which temporarily
 // yields execution of the calling thread.
 //
 // See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-sched_yield---errno
-var schedYield = stubFunction(SchedYieldName, nil)
+var schedYield = newHostFunc(SchedYieldName, schedYieldFn, nil)
+
+func schedYieldFn(_ context.Context, mod api.Module, _ []uint64) Errno {
+	sysCtx := mod.(*wasm.CallContext).Sys
+	sysCtx.Osyield()
+	return ErrnoSuccess
+}
diff --git a/internal/platform/open_file_windows.go b/internal/platform/open_file_windows.go
index a84513ba..8955366b 100644
--- a/internal/platform/open_file_windows.go
+++ b/internal/platform/open_file_windows.go
@@ -29,6 +29,9 @@ const (
 )
 
 func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
+	isDir := flag&O_DIRECTORY > 0
+	flag &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
+
 	fd, err := open(name, flag|syscall.O_CLOEXEC, uint32(perm))
 	if err == nil {
 		return os.NewFile(uintptr(fd), name), nil
@@ -40,6 +43,25 @@ func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
 			err = syscall.ENOENT
 		} else if errors.Is(err, syscall.ERROR_FILE_EXISTS) {
 			err = syscall.EEXIST
+		} else if notFound := errors.Is(err, syscall.ERROR_FILE_NOT_FOUND); notFound && isDir {
+			// Either symlink or hard link directory not found. We change the returned errno depending on
+			// if it is symlink or not to have consistent behavior across OSes.
+			st, e := os.Lstat(name)
+			if e == nil && st.Mode()&os.ModeSymlink != 0 {
+				// Dangling symlink dir must raise ENOTIDR.
+				err = syscall.ENOTDIR
+			} else {
+				err = syscall.ENOENT
+			}
+		} else if notFound {
+			// Either symlink or hard link file not found. We change the returned errno depending on
+			// if it is symlink or not to have consistent behavior across OSes.
+			st, e := os.Lstat(name)
+			if e == nil && st.Mode()&os.ModeSymlink != 0 {
+				err = syscall.ELOOP
+			} else {
+				err = syscall.ENOENT
+			}
 		}
 	}
 	return f, err
@@ -48,7 +70,6 @@ func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
 // The following is lifted from syscall_windows.go to add support for setting FILE_SHARE_DELETE.
 // https://github.com/golang/go/blob/go1.20/src/syscall/syscall_windows.go#L308-L379
 func open(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
-	mode &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
 	if len(path) == 0 {
 		return syscall.InvalidHandle, syscall.ERROR_FILE_NOT_FOUND
 	}
diff --git a/internal/platform/time.go b/internal/platform/time.go
index 21e419d6..5de23af1 100644
--- a/internal/platform/time.go
+++ b/internal/platform/time.go
@@ -39,6 +39,9 @@ func NewFakeNanotime() *sys.Nanotime {
 // FakeNanosleep implements sys.Nanosleep by returning without sleeping.
 func FakeNanosleep(int64) {}
 
+// FakeOsyield implements sys.Osyield by returning without yielding.
+func FakeOsyield() {}
+
 // Walltime implements sys.Walltime with time.Now.
 //
 // Note: This is only notably less efficient than it could be is reading
diff --git a/internal/sys/sys.go b/internal/sys/sys.go
index a239d8ba..1c86bce9 100644
--- a/internal/sys/sys.go
+++ b/internal/sys/sys.go
@@ -24,6 +24,7 @@ type Context struct {
 	nanotime           *sys.Nanotime
 	nanotimeResolution sys.ClockResolution
 	nanosleep          *sys.Nanosleep
+	osyield            *sys.Osyield
 	randSource         io.Reader
 	fsc                *FSContext
 }
@@ -93,6 +94,11 @@ func (c *Context) Nanosleep(ns int64) {
 	(*(c.nanosleep))(ns)
 }
 
+// Osyield implements sys.Osyield.
+func (c *Context) Osyield() {
+	(*(c.osyield))()
+}
+
 // FS returns the possibly empty (sysfs.UnimplementedFS) file system context.
 func (c *Context) FS() *FSContext {
 	return c.fsc
@@ -115,7 +121,7 @@ func (eofReader) Read([]byte) (int, error) {
 
 // DefaultContext returns Context with no values set except a possible nil fs.FS
 func DefaultContext(fs sysfs.FS) *Context {
-	if sysCtx, err := NewContext(0, nil, nil, nil, nil, nil, nil, nil, 0, nil, 0, nil, fs); err != nil {
+	if sysCtx, err := NewContext(0, nil, nil, nil, nil, nil, nil, nil, 0, nil, 0, nil, nil, fs); err != nil {
 		panic(fmt.Errorf("BUG: DefaultContext should never error: %w", err))
 	} else {
 		return sysCtx
@@ -125,6 +131,7 @@ func DefaultContext(fs sysfs.FS) *Context {
 var (
 	_                = DefaultContext(nil) // Force panic on bug.
 	ns sys.Nanosleep = platform.FakeNanosleep
+	oy sys.Osyield   = platform.FakeOsyield
 )
 
 // NewContext is a factory function which helps avoid needing to know defaults or exporting all fields.
@@ -140,6 +147,7 @@ func NewContext(
 	nanotime *sys.Nanotime,
 	nanotimeResolution sys.ClockResolution,
 	nanosleep *sys.Nanosleep,
+	osyield *sys.Osyield,
 	rootFS sysfs.FS,
 ) (sysCtx *Context, err error) {
 	sysCtx = &Context{args: args, environ: environ}
@@ -186,6 +194,12 @@ func NewContext(
 		sysCtx.nanosleep = &ns
 	}
 
+	if osyield != nil {
+		sysCtx.osyield = osyield
+	} else {
+		sysCtx.osyield = &oy
+	}
+
 	if rootFS != nil {
 		sysCtx.fsc, err = NewFSContext(stdin, stdout, stderr, rootFS)
 	} else {
diff --git a/site/content/specs.md b/site/content/specs.md
index 01952459..d014027c 100644
--- a/site/content/specs.md
+++ b/site/content/specs.md
@@ -124,7 +124,7 @@ Notes:
 | poll_oneoff             |   ✅    | Rust,TinyGo,Zig |
 | proc_exit               |   ✅    | Rust,TinyGo,Zig |
 | proc_raise              |   💀   |                 |
-| sched_yield             |   ❌    |                 |
+| sched_yield             |   ✅    |            Rust |
 | random_get              |   ✅    | Rust,TinyGo,Zig |
 | sock_accept             |   ❌    |                 |
 | sock_recv               |   ❌    |                 |
diff --git a/sys/clock.go b/sys/clock.go
index 74507df1..d0fa2bd5 100644
--- a/sys/clock.go
+++ b/sys/clock.go
@@ -20,3 +20,6 @@ type Nanotime func() int64
 
 // Nanosleep puts the current goroutine to sleep for at least ns nanoseconds.
 type Nanosleep func(ns int64)
+
+// Osyield yields the processor, typically to implement spin-wait loops.
+type Osyield func()


Test patch:
diff --git a/config_test.go b/config_test.go
index 714ce3f3..24aa19ef 100644
--- a/config_test.go
+++ b/config_test.go
@@ -176,6 +176,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,         // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -193,6 +194,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,              // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -210,6 +212,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,             // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -227,6 +230,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,              // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -244,6 +248,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,          // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -261,6 +266,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,         // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -278,6 +284,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,                  // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -295,6 +302,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,                   // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -312,6 +320,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,                  // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -329,6 +338,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,         // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				sysfs.Adapt(testFS),
 			),
 		},
@@ -346,6 +356,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,         // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil,                  // nanosleep
+				nil,                  // osyield
 				sysfs.Adapt(testFS2), // fs
 			),
 		},
@@ -363,6 +374,7 @@ func TestModuleConfig_toSysContext(t *testing.T) {
 				&wt, 1,         // walltime, walltimeResolution
 				&nt, 1, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // fs
 			),
 		},
@@ -522,6 +534,19 @@ func TestModuleConfig_toSysContext_WithNanosleep(t *testing.T) {
 	sysCtx.Nanosleep(2)
 }
 
+// TestModuleConfig_toSysContext_WithOsyield has to test differently because
+// we can't compare function pointers when functions are passed by value.
+func TestModuleConfig_toSysContext_WithOsyield(t *testing.T) {
+	var yielded bool
+	sysCtx, err := NewModuleConfig().
+		WithOsyield(func() {
+			yielded = true
+		}).(*moduleConfig).toSysContext()
+	require.NoError(t, err)
+	sysCtx.Osyield()
+	require.True(t, yielded)
+}
+
 func TestModuleConfig_toSysContext_Errors(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -703,6 +728,7 @@ func requireSysContext(
 	walltime *sys.Walltime, walltimeResolution sys.ClockResolution,
 	nanotime *sys.Nanotime, nanotimeResolution sys.ClockResolution,
 	nanosleep *sys.Nanosleep,
+	osyield *sys.Osyield,
 	fs sysfs.FS,
 ) *internalsys.Context {
 	sysCtx, err := internalsys.NewContext(
@@ -715,7 +741,7 @@ func requireSysContext(
 		randSource,
 		walltime, walltimeResolution,
 		nanotime, nanotimeResolution,
-		nanosleep,
+		nanosleep, osyield,
 		fs,
 	)
 	require.NoError(t, err)
diff --git a/imports/wasi_snapshot_preview1/sched_test.go b/imports/wasi_snapshot_preview1/sched_test.go
index 52d0e3d3..fb3c2256 100644
--- a/imports/wasi_snapshot_preview1/sched_test.go
+++ b/imports/wasi_snapshot_preview1/sched_test.go
@@ -3,15 +3,22 @@ package wasi_snapshot_preview1_test
 import (
 	"testing"
 
+	"github.com/tetratelabs/wazero"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 	. "github.com/tetratelabs/wazero/internal/wasi_snapshot_preview1"
 )
 
-// Test_schedYield only tests it is stubbed for GrainLang per #271
 func Test_schedYield(t *testing.T) {
-	log := requireErrnoNosys(t, SchedYieldName)
+	var yielded bool
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().
+		WithOsyield(func() {
+			yielded = true
+		}))
+	defer r.Close(testCtx)
+	requireErrno(t, ErrnoSuccess, mod, SchedYieldName)
 	require.Equal(t, `
---> wasi_snapshot_preview1.sched_yield()
-<-- errno=ENOSYS
-`, log)
+==> wasi_snapshot_preview1.sched_yield()
+<== errno=ESUCCESS
+`, "\n"+log.String())
+	require.True(t, yielded)
 }
diff --git a/internal/platform/open_file_test.go b/internal/platform/open_file_test.go
index 1f7a4e77..0f0a029c 100644
--- a/internal/platform/open_file_test.go
+++ b/internal/platform/open_file_test.go
@@ -3,6 +3,7 @@ package platform
 import (
 	"os"
 	"path"
+	"path/filepath"
 	"syscall"
 	"testing"
 
@@ -27,4 +28,19 @@ func TestOpenFile_Errors(t *testing.T) {
 		_, err = OpenFile(filepath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
 		require.ErrorIs(t, err, syscall.EEXIST)
 	})
+
+	// This is similar to https://github.com/WebAssembly/wasi-testsuite/blob/dc7f8d27be1030cd4788ebdf07d9b57e5d23441e/tests/rust/src/bin/dangling_symlink.rs
+	t.Run("dangling symlinks", func(t *testing.T) {
+		target := filepath.Join(tmp, "target")
+		symlink := filepath.Join(tmp, "dangling_symlink_symlink.cleanup")
+
+		err := os.Symlink(target, symlink)
+		require.NoError(t, err)
+
+		_, err = OpenFile(symlink, O_DIRECTORY|O_NOFOLLOW, 0o0666)
+		require.ErrorIs(t, err, syscall.ENOTDIR)
+
+		_, err = OpenFile(symlink, O_NOFOLLOW, 0o0666)
+		require.ErrorIs(t, err, syscall.ELOOP)
+	})
 }
diff --git a/internal/sys/sys_test.go b/internal/sys/sys_test.go
index 9cccbe4f..6edb7112 100644
--- a/internal/sys/sys_test.go
+++ b/internal/sys/sys_test.go
@@ -41,6 +41,7 @@ func TestDefaultSysContext(t *testing.T) {
 		nil, 0, // walltime, walltimeResolution
 		nil, 0, // nanotime, nanotimeResolution
 		nil,    // nanosleep
+		nil,    // osyield
 		testFS, // rootFS
 	)
 	require.NoError(t, err)
@@ -126,6 +127,7 @@ func TestNewContext_Args(t *testing.T) {
 				nil, 0,                           // walltime, walltimeResolution
 				nil, 0, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // rootFS
 			)
 			if tc.expectedErr == "" {
@@ -188,6 +190,7 @@ func TestNewContext_Environ(t *testing.T) {
 				nil, 0,                           // walltime, walltimeResolution
 				nil, 0, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // rootFS
 			)
 			if tc.expectedErr == "" {
@@ -236,6 +239,7 @@ func TestNewContext_Walltime(t *testing.T) {
 				tc.time, tc.resolution, // walltime, walltimeResolution
 				nil, 0, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // rootFS
 			)
 			if tc.expectedErr == "" {
@@ -284,6 +288,7 @@ func TestNewContext_Nanotime(t *testing.T) {
 				nil, 0, // nanotime, nanotimeResolution
 				tc.time, tc.resolution, // nanotime, nanotimeResolution
 				nil, // nanosleep
+				nil, // osyield
 				nil, // rootFS
 			)
 			if tc.expectedErr == "" {
@@ -341,8 +346,29 @@ func TestNewContext_Nanosleep(t *testing.T) {
 		nil, 0, // Nanosleep, NanosleepResolution
 		nil, 0, // Nanosleep, NanosleepResolution
 		&aNs, // nanosleep
+		nil,  // osyield
 		nil,  // rootFS
 	)
 	require.Nil(t, err)
 	require.Equal(t, &aNs, sysCtx.nanosleep)
 }
+
+func TestNewContext_Osyield(t *testing.T) {
+	var oy sys.Osyield = func() {}
+	sysCtx, err := NewContext(
+		0,   // max
+		nil, // args
+		nil,
+		nil,    // stdin
+		nil,    // stdout
+		nil,    // stderr
+		nil,    // randSource
+		nil, 0, // Nanosleep, NanosleepResolution
+		nil, 0, // Nanosleep, NanosleepResolution
+		nil, // nanosleep
+		&oy, // osyield
+		nil, // rootFS
+	)
+	require.Nil(t, err)
+	require.Equal(t, &oy, sysCtx.osyield)
+}