Repository: tetratelabs/wazero
Pull request #1431 (2023-05-03 09:27:38)

Adds Path to platform.File and refactors tests
This adds Path which is non-empty when the file is a normal file or directory. This immediately supports the re-open use case, and later may make some things in windows easier.

This also adds a platform.NewFsFile function which makes some call sites better.

Finally, this fixes a bug hidden by how our tests were written, where the wrong errno was returned when a file was used after close in windows.

codefromthecrypt: @evacchi I'm stumped for tonight on how I screwed up TestOpenFile_Errors/writing_to_a_read-only_file_is_EBADF on windows. If you have a minute to check it out or fix, much oblidged. otherwise, I'll swing the axe tomorrow. I tried to split code out of the other PRs and this is the one that's ready except this fail.
evacchi: apparently the code on `main` can't acquire this lock, which trickles down into the original error. In fact the field of `poll.FD.fdmu.state` is 1 for `main` and 0 on this branch.

So, your recent change is now acquiring the lock, and it no longer returns with "use of closed file"

```
// poll/fd_windows.go:
func (fd *FD) Write(buf []byte) (int, error) {
	if err := fd.writeLock(); err != nil {
		return 0, err
	}

// the callee returns the actual error
func (fd *FD) writeLock() error {
	if !fd.fdmu.rwlock(false) {
		return errClosing(fd.isFile)
	}
	return nil
}

```

now I've investigated this further and it looks like the issue is that the test was originally written as:

```
		f := openFsFile(t, path, os.O_RDONLY, 0)
		defer require.Zero(t, f.Close())
```

and apparently the semantics of `defer` are to eagerly evaluate all the arguments to `require.Zero()` and _then_ delay `require.Zero(...)`; in other words `f.Close()` was not really deferred 😨 

in other words, this is also broken on main, we just never noticed 😰 🤣 


evacchi: ...and apparently I have pushed it to the other PR #1430 🤦  
evacchi: feel free to revert the commit from either (or both!) PRs, hope this helps
codefromthecrypt: Thanks for the help @evacchi, super subtle! I didn't mean for you to fix the other PR, just mentioned it (it wasn't it good condition for someone to help on :p).

Anyway, thanks for the help as I was so stumped by I suppose what amounted to assuming the main tests did what they appear. (missing the eager close in the defer that was there before)

great work!

Patch:
diff --git a/internal/platform/sync_linux.go b/internal/platform/datasync_linux.go
similarity index 65%
rename from internal/platform/sync_linux.go
rename to internal/platform/datasync_linux.go
index 41ae44c8..36384063 100644
--- a/internal/platform/sync_linux.go
+++ b/internal/platform/datasync_linux.go
@@ -7,14 +7,11 @@ import (
 	"syscall"
 )
 
-func fdatasync(f fs.File) syscall.Errno {
+func datasync(f fs.File) syscall.Errno {
 	if fd, ok := f.(fdFile); ok {
 		return UnwrapOSError(syscall.Fdatasync(int(fd.Fd())))
 	}
 
 	// Attempt to sync everything, even if we only need to sync the data.
-	if s, ok := f.(syncFile); ok {
-		return UnwrapOSError(s.Sync())
-	}
-	return 0
+	return sync(f)
 }
diff --git a/internal/platform/sync_unsupported.go b/internal/platform/datasync_unsupported.go
similarity index 54%
rename from internal/platform/sync_unsupported.go
rename to internal/platform/datasync_unsupported.go
index 38284aa5..ce752db3 100644
--- a/internal/platform/sync_unsupported.go
+++ b/internal/platform/datasync_unsupported.go
@@ -7,10 +7,7 @@ import (
 	"syscall"
 )
 
-func fdatasync(f fs.File) syscall.Errno {
+func datasync(f fs.File) syscall.Errno {
 	// Attempt to sync everything, even if we only need to sync the data.
-	if s, ok := f.(syncFile); ok {
-		return UnwrapOSError(s.Sync())
-	}
-	return 0
+	return sync(f)
 }
diff --git a/internal/platform/file.go b/internal/platform/file.go
index 0676c600..a60a5bd6 100644
--- a/internal/platform/file.go
+++ b/internal/platform/file.go
@@ -27,13 +27,19 @@ import (
 // A writable filesystem abstraction is not yet implemented as of Go 1.20. See
 // https://github.com/golang/go/issues/45757
 type File interface {
+	// Path returns path used to open the file or empty if not applicable. For
+	// example, a file representing stdout will return empty.
+	//
+	// Note: This can drift on rename.
+	Path() string
+
 	// Stat is similar to syscall.Fstat.
 	//
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
-	//   - syscall.EBADF if the file or directory was closed.
+	//   - syscall.ENOSYS: the implementation does not support this function.
+	//   - syscall.EBADF: the file or directory was closed.
 	//
 	// # Notes
 	//
@@ -49,8 +55,8 @@ type File interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
-	//   - syscall.EBADF if the file or directory was closed.
+	//   - syscall.ENOSYS: the implementation does not support this function.
+	//   - syscall.EBADF: the file or directory was closed.
 	//
 	// # Notes
 	//
@@ -66,8 +72,8 @@ type File interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
-	//   - syscall.EBADF if the file or directory was closed.
+	//   - syscall.ENOSYS: the implementation does not support this function.
+	//   - syscall.EBADF: the file or directory was closed.
 	//
 	// # Notes
 	//
@@ -81,7 +87,7 @@ type File interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.EBADF if the file or directory was closed.
+	//   - syscall.EBADF: the file or directory was closed.
 	//
 	// # Notes
 	//
@@ -96,7 +102,7 @@ type File interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.EBADF if the file or directory was closed.
+	//   - syscall.EBADF: the file or directory was closed.
 	//
 	// # Notes
 	//
@@ -109,7 +115,7 @@ type File interface {
 	// Close closes the underlying file.
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//
 	// # Notes
 	//
@@ -150,13 +156,23 @@ func (UnimplementedFile) Datasync() syscall.Errno {
 	return 0 // not syscall.ENOSYS
 }
 
-type DefaultFile struct {
-	F fs.File
+func NewFsFile(path string, f fs.File) File {
+	return &fsFile{path, f}
+}
+
+type fsFile struct {
+	path string
+	file fs.File
+}
+
+// Path implements File.Path
+func (f *fsFile) Path() string {
+	return f.path
 }
 
 // Stat implements File.Stat
-func (f *DefaultFile) Stat() (Stat_t, syscall.Errno) {
-	st, errno := statFile(f.F)
+func (f *fsFile) Stat() (Stat_t, syscall.Errno) {
+	st, errno := statFile(f.file)
 	if errno == syscall.EIO {
 		errno = syscall.EBADF
 	}
@@ -164,42 +180,42 @@ func (f *DefaultFile) Stat() (Stat_t, syscall.Errno) {
 }
 
 // Chmod implements File.Chmod
-func (f *DefaultFile) Chmod(mode fs.FileMode) syscall.Errno {
-	if f, ok := f.F.(chmodFile); ok {
+func (f *fsFile) Chmod(mode fs.FileMode) syscall.Errno {
+	if f, ok := f.file.(chmodFile); ok {
 		return UnwrapOSError(f.Chmod(mode))
 	}
 	return syscall.ENOSYS
 }
 
 // Chown implements File.Chown
-func (f *DefaultFile) Chown(uid, gid int) syscall.Errno {
-	if f, ok := f.F.(fdFile); ok {
+func (f *fsFile) Chown(uid, gid int) syscall.Errno {
+	if f, ok := f.file.(fdFile); ok {
 		return fchown(f.Fd(), uid, gid)
 	}
 	return syscall.ENOSYS
 }
 
 // Sync implements File.Sync
-func (f *DefaultFile) Sync() syscall.Errno {
-	if f, ok := f.F.(syncFile); ok {
+func (f *fsFile) Sync() syscall.Errno {
+	if f, ok := f.file.(syncFile); ok {
 		return UnwrapOSError(f.Sync())
 	}
 	return 0 // don't error
 }
 
 // Datasync implements File.Datasync
-func (f *DefaultFile) Datasync() syscall.Errno {
-	return fdatasync(f.F)
+func (f *fsFile) Datasync() syscall.Errno {
+	return datasync(f.file)
 }
 
 // Close implements File.Close
-func (f *DefaultFile) Close() syscall.Errno {
-	return UnwrapOSError(f.F.Close())
+func (f *fsFile) Close() syscall.Errno {
+	return UnwrapOSError(f.file.Close())
 }
 
 // File implements File.File
-func (f *DefaultFile) File() fs.File {
-	return f.F
+func (f *fsFile) File() fs.File {
+	return f.file
 }
 
 // ReadFile declares all read interfaces defined on os.File used by wazero.
diff --git a/internal/platform/open_file.go b/internal/platform/open_file.go
index 62950538..6fb3f5b7 100644
--- a/internal/platform/open_file.go
+++ b/internal/platform/open_file.go
@@ -17,7 +17,10 @@ const (
 
 // OpenFile is like os.OpenFile except it returns syscall.Errno. A zero
 // syscall.Errno is success.
-func OpenFile(path string, flag int, perm fs.FileMode) (File, syscall.Errno) {
+func OpenFile(path string, flag int, perm fs.FileMode) (fs.File, syscall.Errno) {
 	f, err := os.OpenFile(path, flag, perm)
-	return &DefaultFile{F: f}, UnwrapOSError(err)
+	// Note: This does not return a platform.File because sysfs.FS that returns
+	// one may want to hide the real OS path. For example, this is needed for
+	// pre-opens.
+	return f, UnwrapOSError(err)
 }
diff --git a/internal/platform/open_file_js.go b/internal/platform/open_file_js.go
index f59663a8..5a45038f 100644
--- a/internal/platform/open_file_js.go
+++ b/internal/platform/open_file_js.go
@@ -12,8 +12,8 @@ const (
 	O_NOFOLLOW  = 1 << 30
 )
 
-func OpenFile(path string, flag int, perm fs.FileMode) (File, syscall.Errno) {
+func OpenFile(path string, flag int, perm fs.FileMode) (fs.File, syscall.Errno) {
 	flag &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
 	f, err := os.OpenFile(path, flag, perm)
-	return &DefaultFile{F: f}, UnwrapOSError(err)
+	return f, UnwrapOSError(err)
 }
diff --git a/internal/platform/open_file_sun.go b/internal/platform/open_file_sun.go
index 5553f68b..791072ed 100644
--- a/internal/platform/open_file_sun.go
+++ b/internal/platform/open_file_sun.go
@@ -14,7 +14,7 @@ const (
 	O_NOFOLLOW  = syscall.O_NOFOLLOW
 )
 
-func OpenFile(path string, flag int, perm fs.FileMode) (File, syscall.Errno) {
+func OpenFile(path string, flag int, perm fs.FileMode) (fs.File, syscall.Errno) {
 	f, err := os.OpenFile(path, flag, perm)
-	return &DefaultFile{F: f}, UnwrapOSError(err)
+	return f, UnwrapOSError(err)
 }
diff --git a/internal/platform/open_file_windows.go b/internal/platform/open_file_windows.go
index 6d9c9d4a..2589647a 100644
--- a/internal/platform/open_file_windows.go
+++ b/internal/platform/open_file_windows.go
@@ -28,12 +28,12 @@ const (
 	O_NOFOLLOW  = 1 << 30
 )
 
-func OpenFile(path string, flag int, perm fs.FileMode) (File, syscall.Errno) {
+func OpenFile(path string, flag int, perm fs.FileMode) (fs.File, syscall.Errno) {
 	if f, errno := openFile(path, flag, perm); errno != 0 {
 		return nil, errno
-	} else {
+	} else { // TODO: revisit windowsWrappedFile once fsFile is complete
 		f := &windowsWrappedFile{WriteFile: f, path: path, flag: flag, perm: perm}
-		return &DefaultFile{F: f}, 0
+		return f, 0
 	}
 }
 
diff --git a/internal/platform/sync.go b/internal/platform/sync.go
new file mode 100644
index 00000000..a847c3c1
--- /dev/null
+++ b/internal/platform/sync.go
@@ -0,0 +1,13 @@
+package platform
+
+import (
+	"io/fs"
+	"syscall"
+)
+
+func sync(f fs.File) syscall.Errno {
+	if s, ok := f.(syncFile); ok {
+		return UnwrapOSError(s.Sync())
+	}
+	return 0
+}
diff --git a/internal/platform/wrap_windows.go b/internal/platform/wrap_windows.go
index a10ed198..82b3a054 100644
--- a/internal/platform/wrap_windows.go
+++ b/internal/platform/wrap_windows.go
@@ -1,6 +1,7 @@
 package platform
 
 import (
+	"errors"
 	"io/fs"
 	"syscall"
 )
@@ -61,7 +62,13 @@ func (w *windowsWrappedFile) Write(p []byte) (n int, err error) {
 		return
 	}
 
-	return w.WriteFile.Write(p)
+	n, err = w.WriteFile.Write(p)
+	// ERROR_ACCESS_DENIED is often returned instead of EBADF
+	// when a file is used after close.
+	if errors.Is(err, ERROR_ACCESS_DENIED) {
+		err = syscall.EBADF
+	}
+	return
 }
 
 // Close implements io.Closer
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index 3e8dda9c..2bbcc8a8 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -160,6 +160,11 @@ type lazyDir struct {
 	f  platform.File
 }
 
+// Path implements the same method as documented on platform.File
+func (r *lazyDir) Path() string {
+	return "."
+}
+
 // Stat implements the same method as documented on platform.File
 func (r *lazyDir) Stat() (platform.Stat_t, syscall.Errno) {
 	if f, ok := r.file(); !ok {
@@ -273,7 +278,6 @@ type FileEntry struct {
 	// was called.
 	ReadDir *ReadDir
 
-	openPath string
 	openFlag int
 	openPerm fs.FileMode
 }
@@ -408,9 +412,7 @@ func stdinReader(r io.Reader) (*FileEntry, error) {
 		freader = NewStdioFileReader(r, s, PollerDefaultStdin)
 	}
 	return &FileEntry{
-		Name: noopStdinStat.Name(), File: &platform.DefaultFile{
-			F: freader,
-		},
+		Name: noopStdinStat.Name(), File: platform.NewFsFile("", freader),
 	}, nil
 }
 
@@ -423,9 +425,7 @@ func stdioWriter(w io.Writer, defaultStat stdioFileInfo) (*FileEntry, error) {
 		return nil, err
 	}
 	return &FileEntry{
-		Name: s.Name(), File: &platform.DefaultFile{
-			F: &stdioFileWriter{w: w, s: s},
-		},
+		Name: s.Name(), File: platform.NewFsFile("", &stdioFileWriter{w: w, s: s}),
 	}, nil
 }
 
@@ -453,7 +453,7 @@ func (c *FSContext) OpenFile(fs sysfs.FS, path string, flag int, perm fs.FileMod
 	if f, errno := fs.OpenFile(path, flag, perm); errno != 0 {
 		return 0, errno
 	} else {
-		fe := &FileEntry{openPath: path, FS: fs, File: f, openFlag: flag, openPerm: perm}
+		fe := &FileEntry{FS: fs, File: f, openFlag: flag, openPerm: perm}
 		if path == "/" || path == "." {
 			fe.Name = ""
 		} else {
@@ -493,7 +493,7 @@ func (c *FSContext) reopen(f *FileEntry) syscall.Errno {
 	}
 
 	// Re-opens with  the same parameters as before.
-	opened, errno := f.FS.OpenFile(f.openPath, f.openFlag, f.openPerm)
+	opened, errno := f.FS.OpenFile(f.File.Path(), f.openFlag, f.openPerm)
 	if errno != 0 {
 		return errno
 	}
diff --git a/internal/sysfs/adapter.go b/internal/sysfs/adapter.go
index 1393f097..762daa66 100644
--- a/internal/sysfs/adapter.go
+++ b/internal/sysfs/adapter.go
@@ -48,7 +48,7 @@ func (a *adapter) Open(name string) (fs.File, error) {
 func (a *adapter) OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) {
 	path = cleanPath(path)
 	f, err := a.fs.Open(path)
-	return &platform.DefaultFile{F: f}, platform.UnwrapOSError(err)
+	return platform.NewFsFile(path, f), platform.UnwrapOSError(err)
 }
 
 // Stat implements FS.Stat
@@ -59,7 +59,7 @@ func (a *adapter) Stat(path string) (platform.Stat_t, syscall.Errno) {
 		return platform.Stat_t{}, platform.UnwrapOSError(err)
 	}
 	defer f.Close()
-	return (&platform.DefaultFile{F: f}).Stat()
+	return platform.NewFsFile(path, f).Stat()
 }
 
 // Lstat implements FS.Lstat
diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go
index 0a698310..bdad24bd 100644
--- a/internal/sysfs/dirfs.go
+++ b/internal/sysfs/dirfs.go
@@ -42,7 +42,11 @@ func (d *dirFS) Open(name string) (fs.File, error) {
 
 // OpenFile implements FS.OpenFile
 func (d *dirFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) {
-	return platform.OpenFile(d.join(path), flag, perm)
+	f, errno := platform.OpenFile(d.join(path), flag, perm)
+	if errno != 0 {
+		return nil, errno
+	}
+	return platform.NewFsFile(path, f), 0
 }
 
 // Lstat implements FS.Lstat
diff --git a/internal/sysfs/readfs.go b/internal/sysfs/readfs.go
index 50d71339..47ed66fd 100644
--- a/internal/sysfs/readfs.go
+++ b/internal/sysfs/readfs.go
@@ -67,7 +67,7 @@ func (r *readFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.Fil
 	if errno != 0 {
 		return nil, errno
 	}
-	return &platform.DefaultFile{F: maskForReads(f.File())}, 0
+	return platform.NewFsFile(path, maskForReads(f.File())), 0
 }
 
 // maskForReads masks the file with read-only interfaces used by wazero.
diff --git a/internal/sysfs/rootfs.go b/internal/sysfs/rootfs.go
index 8fe48018..bd973923 100644
--- a/internal/sysfs/rootfs.go
+++ b/internal/sysfs/rootfs.go
@@ -137,7 +137,7 @@ func (c *CompositeFS) OpenFile(path string, flag int, perm fs.FileMode) (f platf
 		case ".", "/", "":
 			if len(c.rootGuestPaths) > 0 {
 				dir := &openRootDir{c: c, f: f.File().(fs.ReadDirFile)}
-				f = &platform.DefaultFile{F: dir}
+				f = platform.NewFsFile(path, dir)
 			}
 		}
 	}
@@ -483,7 +483,7 @@ type fakeRootFS struct{ UnimplementedFS }
 func (*fakeRootFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) {
 	switch path {
 	case ".", "/", "":
-		return &platform.DefaultFile{F: fakeRootDir{}}, 0
+		return platform.NewFsFile(path, fakeRootDir{}), 0
 	}
 	return nil, syscall.ENOENT
 }
diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go
index 78cfdee6..87294a45 100644
--- a/internal/sysfs/sysfs.go
+++ b/internal/sysfs/sysfs.go
@@ -48,7 +48,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` or `flag` is invalid.
 	//   - syscall.ENOENT: `path` doesn't exist and `flag` doesn't contain
 	//     os.O_CREATE.
@@ -83,7 +83,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.ENOENT: `path` doesn't exist.
 	//
 	// # Notes
@@ -103,7 +103,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.ENOENT: `path` doesn't exist.
 	//
 	// # Notes
@@ -123,7 +123,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.EEXIST: `path` exists and is a directory.
 	//   - syscall.ENOTDIR: `path` exists and is a file.
@@ -144,7 +144,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` does not exist.
 	//
@@ -164,7 +164,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` does not exist.
 	//
@@ -182,7 +182,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` does not exist.
 	//
@@ -200,7 +200,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `from` or `to` is invalid.
 	//   - syscall.ENOENT: `from` or `to` don't exist.
 	//   - syscall.ENOTDIR: `from` is a directory and `to` exists as a file.
@@ -222,7 +222,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` doesn't exist.
 	//   - syscall.ENOTDIR: `path` exists, but isn't a directory.
@@ -242,7 +242,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` doesn't exist.
 	//   - syscall.EISDIR: `path` exists, but is a directory.
@@ -264,7 +264,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EPERM: `oldPath` is invalid.
 	//   - syscall.ENOENT: `oldPath` doesn't exist.
 	//   - syscall.EISDIR: `newPath` exists, but is a directory.
@@ -283,7 +283,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EPERM: `oldPath` or `newPath` is invalid.
 	//   - syscall.EEXIST: `newPath` exists.
 	//
@@ -308,7 +308,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//
 	// # Notes
@@ -327,9 +327,10 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid or size is negative.
-	//   - syscall.ENOENT: `path` doesn't exist
+	//   - syscall.ENOENT: `path` doesn't exist.
+	//   - syscall.EISDIR: `path` is a directory.
 	//   - syscall.EACCES: `path` doesn't have write access.
 	//
 	// # Notes
@@ -356,7 +357,7 @@ type FS interface {
 	// # Errors
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
-	//   - syscall.ENOSYS the implementation does not support this function.
+	//   - syscall.ENOSYS: the implementation does not support this function.
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.EEXIST: `path` exists and is a directory.
 	//   - syscall.ENOTDIR: `path` exists and is a file.


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index d0c92dbf..bb825745 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -3667,11 +3667,8 @@ func Test_pathLink(t *testing.T) {
 			uint64(newFd), uint64(destination), uint64(len(destinationName)))
 		require.Contains(t, log.String(), wasip1.ErrnoName(wasip1.ErrnoSuccess))
 
-		f, errno := platform.OpenFile(destinationRealPath, os.O_RDONLY, 0)
-		require.EqualErrno(t, 0, errno)
-		defer func() {
-			require.Zero(t, f.Close())
-		}()
+		f := openFsFile(t, destinationRealPath, os.O_RDONLY, 0)
+		defer f.Close()
 
 		st, errno := f.Stat()
 		require.EqualErrno(t, 0, errno)
@@ -5028,8 +5025,7 @@ func Test_fdReaddir_opened_file_written(t *testing.T) {
 	require.EqualErrno(t, 0, errno)
 
 	// Then write a file to the directory.
-	f, errno := platform.OpenFile(joinPath(dirPath, "file"), os.O_CREATE, 0)
-	require.EqualErrno(t, 0, errno)
+	f := openFsFile(t, joinPath(dirPath, "file"), os.O_CREATE, 0)
 	defer f.Close()
 
 	// get the real inode of the current directory
@@ -5076,3 +5072,9 @@ func Test_fdReaddir_opened_file_written(t *testing.T) {
 func joinPath(dirName, baseName string) string {
 	return path.Join(dirName, baseName)
 }
+
+func openFsFile(t *testing.T, path string, flag int, perm fs.FileMode) platform.File {
+	f, errno := platform.OpenFile(path, flag, perm)
+	require.EqualErrno(t, 0, errno)
+	return platform.NewFsFile(path, f)
+}
diff --git a/internal/platform/chown_unix_test.go b/internal/platform/chown_unix_test.go
index b9567aa6..32bd0728 100644
--- a/internal/platform/chown_unix_test.go
+++ b/internal/platform/chown_unix_test.go
@@ -18,8 +18,8 @@ func TestChown(t *testing.T) {
 	dir := path.Join(tmpDir, "dir")
 	require.NoError(t, os.Mkdir(dir, 0o0777))
 
-	dirF, errno := OpenFile(dir, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	dirF := openFsFile(t, dir, syscall.O_RDONLY, 0)
+	defer dirF.Close()
 
 	dirStat, err := dirF.File().Stat()
 	require.NoError(t, err)
@@ -66,8 +66,8 @@ func TestDefaultFileChown(t *testing.T) {
 	dir := path.Join(tmpDir, "dir")
 	require.NoError(t, os.Mkdir(dir, 0o0777))
 
-	dirF, errno := OpenFile(dir, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	dirF := openFsFile(t, dir, syscall.O_RDONLY, 0)
+	defer dirF.Close()
 
 	dirStat, err := dirF.File().Stat()
 	require.NoError(t, err)
@@ -116,8 +116,8 @@ func TestLchown(t *testing.T) {
 	dir := path.Join(tmpDir, "dir")
 	require.NoError(t, os.Mkdir(dir, 0o0777))
 
-	dirF, errno := OpenFile(dir, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	dirF := openFsFile(t, dir, syscall.O_RDONLY, 0)
+	defer dirF.Close()
 
 	dirStat, err := dirF.File().Stat()
 	require.NoError(t, err)
@@ -127,8 +127,8 @@ func TestLchown(t *testing.T) {
 	link := path.Join(tmpDir, "link")
 	require.NoError(t, os.Symlink(dir, link))
 
-	linkF, errno := OpenFile(link, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	linkF := openFsFile(t, link, syscall.O_RDONLY, 0)
+	defer linkF.Close()
 
 	linkStat, err := linkF.File().Stat()
 	require.NoError(t, err)
diff --git a/internal/platform/file_test.go b/internal/platform/file_test.go
index 19b7a4f3..2fa5f6eb 100644
--- a/internal/platform/file_test.go
+++ b/internal/platform/file_test.go
@@ -7,6 +7,7 @@ import (
 	"io/fs"
 	"os"
 	"path"
+	"runtime"
 	"syscall"
 	"testing"
 
@@ -21,6 +22,11 @@ type NoopFile struct {
 	UnimplementedFile
 }
 
+// The current design requires the user to implement Path.
+func (NoopFile) Path() string {
+	return "noop"
+}
+
 // The current design requires the user to consciously implement Close.
 // However, we could change UnimplementedFile to return zero.
 func (NoopFile) Close() (errno syscall.Errno) { return }
@@ -40,11 +46,13 @@ func TestFileDatasync_NoError(t *testing.T) {
 }
 
 func testSync_NoError(t *testing.T, sync func(File) syscall.Errno) {
-	ro, err := embedFS.Open("file_test.go")
+	roPath := "file_test.go"
+	ro, err := embedFS.Open(roPath)
 	require.NoError(t, err)
 	defer ro.Close()
 
-	rw, err := os.Create(path.Join(t.TempDir(), "datasync"))
+	rwPath := path.Join(t.TempDir(), "datasync")
+	rw, err := os.Create(rwPath)
 	require.NoError(t, err)
 	defer rw.Close()
 
@@ -58,11 +66,11 @@ func testSync_NoError(t *testing.T, sync func(File) syscall.Errno) {
 		},
 		{
 			name: "File of read-only fs.File",
-			f:    &DefaultFile{F: ro},
+			f:    NewFsFile(roPath, ro),
 		},
 		{
 			name: "File of os.File",
-			f:    &DefaultFile{F: rw},
+			f:    NewFsFile(rwPath, rw),
 		},
 	}
 
@@ -75,11 +83,11 @@ func testSync_NoError(t *testing.T, sync func(File) syscall.Errno) {
 	}
 }
 
-func TestFileSync(t *testing.T) {
+func TestFsFileSync(t *testing.T) {
 	testSync(t, File.Sync)
 }
 
-func TestFileDatasync(t *testing.T) {
+func TestFsFileDatasync(t *testing.T) {
 	testSync(t, File.Datasync)
 }
 
@@ -87,31 +95,60 @@ func TestFileDatasync(t *testing.T) {
 // sync anyway. There is no test in Go for syscall.Fdatasync, but closest is
 // similar to below. Effectively, this only tests that things don't error.
 func testSync(t *testing.T, sync func(File) syscall.Errno) {
-	f, errno := os.CreateTemp("", t.Name())
-	require.NoError(t, errno)
+	dPath := t.TempDir()
+	fPath := path.Join(dPath, t.Name())
+
+	f := openFsFile(t, fPath, os.O_RDWR|os.O_CREATE, 0o600)
 	defer f.Close()
 
 	expected := "hello world!"
 
 	// Write the expected data
-	_, errno = f.Write([]byte(expected))
-	require.NoError(t, errno)
+	_, err := f.File().(io.Writer).Write([]byte(expected))
+	require.NoError(t, err)
 
 	// Sync the data.
-	if errno = sync(&DefaultFile{F: f}); errno == syscall.ENOSYS {
-		return // don't continue if it isn't supported.
-	}
+	errno := sync(f)
 	require.EqualErrno(t, 0, errno)
 
 	// Rewind while the file is still open.
-	_, err := f.Seek(0, io.SeekStart)
+	_, err = f.File().(io.Seeker).Seek(0, io.SeekStart)
 	require.NoError(t, err)
 
 	// Read data from the file
 	var buf bytes.Buffer
-	_, errno = io.Copy(&buf, f)
-	require.NoError(t, errno)
+	_, err = io.Copy(&buf, f.File().(io.Reader))
+	require.NoError(t, err)
 
 	// It may be the case that sync worked.
 	require.Equal(t, expected, buf.String())
+
+	// Windows allows you to sync a closed file
+	if runtime.GOOS != "windows" {
+		testEBADFIfFileClosed(t, sync)
+	}
+}
+
+func testEBADFIfFileClosed(t *testing.T, fn func(File) syscall.Errno) bool {
+	return t.Run("EBADF if file closed", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		f := openForWrite(t, path.Join(tmpDir, "EBADF"), []byte{1, 2, 3, 4})
+
+		// close the file underneath
+		require.Zero(t, f.Close())
+
+		require.EqualErrno(t, syscall.EBADF, fn(f))
+	})
+}
+
+func openForWrite(t *testing.T, path string, content []byte) File {
+	require.NoError(t, os.WriteFile(path, content, 0o0600))
+	return openFsFile(t, path, os.O_RDWR, 0o666)
+}
+
+func openFsFile(t *testing.T, path string, flag int, perm fs.FileMode) File {
+	f, errno := OpenFile(path, flag, perm)
+	require.EqualErrno(t, 0, errno)
+	return NewFsFile(path, f)
 }
diff --git a/internal/platform/futimens_test.go b/internal/platform/futimens_test.go
index 37c8540a..4aca0bc6 100644
--- a/internal/platform/futimens_test.go
+++ b/internal/platform/futimens_test.go
@@ -167,8 +167,7 @@ func testFutimens(t *testing.T, usePath bool) {
 						}
 					}
 
-					f, errno := OpenFile(path, flag, 0)
-					require.EqualErrno(t, 0, errno)
+					f := openFsFile(t, path, flag, 0)
 
 					errno = UtimensFile(f.File(), tc.times)
 					require.Zero(t, f.Close())
@@ -222,11 +221,10 @@ func TestUtimensFile(t *testing.T) {
 		err := os.WriteFile(file, []byte{}, 0o700)
 		require.NoError(t, err)
 
-		fileF, errno := OpenFile(file, syscall.O_RDWR, 0)
-		require.EqualErrno(t, 0, errno)
+		fileF := openFsFile(t, file, syscall.O_RDWR, 0)
 		require.Zero(t, fileF.Close())
 
-		errno = UtimensFile(fileF.File(), nil)
+		errno := UtimensFile(fileF.File(), nil)
 		require.EqualErrno(t, syscall.EBADF, errno)
 	})
 
@@ -235,8 +233,7 @@ func TestUtimensFile(t *testing.T) {
 		err := os.Mkdir(dir, 0o700)
 		require.NoError(t, err)
 
-		dirF, errno := OpenFile(dir, syscall.O_RDONLY, 0)
-		require.EqualErrno(t, 0, errno)
+		dirF := openFsFile(t, dir, syscall.O_RDONLY, 0)
 		require.Zero(t, dirF.Close())
 
 		err = UtimensFile(dirF.File(), nil)
diff --git a/internal/platform/open_file_test.go b/internal/platform/open_file_test.go
index f4204e23..37bef1ae 100644
--- a/internal/platform/open_file_test.go
+++ b/internal/platform/open_file_test.go
@@ -21,7 +21,7 @@ func TestOpenFile(t *testing.T) {
 
 		f, errno := OpenFile(path+"/", os.O_RDONLY, 0)
 		require.EqualErrno(t, 0, errno)
-		require.Zero(t, f.Close())
+		require.NoError(t, f.Close())
 	})
 
 	// from os.TestDirFSPathsValid
@@ -29,7 +29,7 @@ func TestOpenFile(t *testing.T) {
 		t.Run("strange name", func(t *testing.T) {
 			f, errno := OpenFile(path.Join(tmpDir, `e:xperi\ment.txt`), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
 			require.EqualErrno(t, 0, errno)
-			require.Zero(t, f.Close())
+			require.NoError(t, f.Close())
 		})
 	}
 }
@@ -60,19 +60,18 @@ func TestOpenFile_Errors(t *testing.T) {
 		filepath := path.Join(tmpDir, "file.txt")
 		f, errno := OpenFile(filepath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
 		require.EqualErrno(t, 0, errno)
-		defer require.Zero(t, f.Close())
+		defer f.Close()
 
-		_, errno = OpenFile(filepath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
-		require.EqualErrno(t, syscall.EEXIST, errno)
+		_, err := OpenFile(filepath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
+		require.EqualErrno(t, syscall.EEXIST, err)
 	})
 
 	t.Run("writing to a read-only file is EBADF", func(t *testing.T) {
 		path := path.Join(tmpDir, "file")
 		require.NoError(t, os.WriteFile(path, nil, 0o600))
 
-		f, errno := OpenFile(path, os.O_RDONLY, 0)
-		defer require.Zero(t, f.Close())
-		require.EqualErrno(t, 0, errno)
+		f := openFsFile(t, path, os.O_RDONLY, 0)
+		defer f.Close()
 
 		_, err := f.File().(io.Writer).Write([]byte{1, 2, 3, 4})
 		require.EqualErrno(t, syscall.EBADF, UnwrapOSError(err))
@@ -82,9 +81,8 @@ func TestOpenFile_Errors(t *testing.T) {
 		path := path.Join(tmpDir, "diragain")
 		require.NoError(t, os.Mkdir(path, 0o755))
 
-		f, errno := OpenFile(path, os.O_RDONLY, 0)
-		defer require.Zero(t, f.Close())
-		require.EqualErrno(t, 0, errno)
+		f := openFsFile(t, path, os.O_RDONLY, 0)
+		defer f.Close()
 
 		_, err := f.File().(io.Writer).Write([]byte{1, 2, 3, 4})
 		require.EqualErrno(t, syscall.EBADF, UnwrapOSError(err))
diff --git a/internal/platform/stat_test.go b/internal/platform/stat_test.go
index 4942569f..3d9cd217 100644
--- a/internal/platform/stat_test.go
+++ b/internal/platform/stat_test.go
@@ -158,14 +158,11 @@ func TestStat(t *testing.T) {
 func TestStatFile(t *testing.T) {
 	tmpDir := t.TempDir()
 
-	var st Stat_t
-
-	tmpDirF, errno := OpenFile(tmpDir, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	tmpDirF := openFsFile(t, tmpDir, syscall.O_RDONLY, 0)
 	defer tmpDirF.Close()
 
 	t.Run("dir", func(t *testing.T) {
-		st, errno = tmpDirF.Stat()
+		st, errno := tmpDirF.Stat()
 		require.EqualErrno(t, 0, errno)
 
 		require.True(t, st.Mode.IsDir())
@@ -177,19 +174,18 @@ func TestStatFile(t *testing.T) {
 	if runtime.GOOS != "windows" {
 		t.Run("closed dir", func(t *testing.T) {
 			require.Zero(t, tmpDirF.Close())
-			st, errno = tmpDirF.Stat()
+			_, errno := tmpDirF.Stat()
 			require.EqualErrno(t, syscall.EBADF, errno)
 		})
 	}
 
 	file := path.Join(tmpDir, "file")
 	require.NoError(t, os.WriteFile(file, nil, 0o400))
-	fileF, errno := OpenFile(file, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	fileF := openFsFile(t, file, syscall.O_RDONLY, 0)
 	defer fileF.Close()
 
 	t.Run("file", func(t *testing.T) {
-		st, errno = fileF.Stat()
+		st, errno := fileF.Stat()
 		require.EqualErrno(t, 0, errno)
 
 		require.False(t, st.Mode.IsDir())
@@ -198,18 +194,17 @@ func TestStatFile(t *testing.T) {
 
 	t.Run("closed file", func(t *testing.T) {
 		require.Zero(t, fileF.Close())
-		_, errno = fileF.Stat()
+		_, errno := fileF.Stat()
 		require.EqualErrno(t, syscall.EBADF, errno)
 	})
 
 	subdir := path.Join(tmpDir, "sub")
 	require.NoError(t, os.Mkdir(subdir, 0o500))
-	subdirF, errno := OpenFile(subdir, syscall.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	subdirF := openFsFile(t, subdir, syscall.O_RDONLY, 0)
 	defer subdirF.Close()
 
 	t.Run("subdir", func(t *testing.T) {
-		st, errno = subdirF.Stat()
+		st, errno := subdirF.Stat()
 		require.EqualErrno(t, 0, errno)
 
 		require.True(t, st.Mode.IsDir())
@@ -219,7 +214,7 @@ func TestStatFile(t *testing.T) {
 	if runtime.GOOS != "windows" { // windows allows you to stat a closed dir
 		t.Run("closed subdir", func(t *testing.T) {
 			require.Zero(t, subdirF.Close())
-			st, errno = subdirF.Stat()
+			_, errno := subdirF.Stat()
 			require.EqualErrno(t, syscall.EBADF, errno)
 		})
 	}
@@ -264,8 +259,7 @@ func Test_StatFile_times(t *testing.T) {
 			err := os.Chtimes(file, time.UnixMicro(tc.atimeNsec/1e3), time.UnixMicro(tc.mtimeNsec/1e3))
 			require.NoError(t, err)
 
-			f, errno := OpenFile(file, syscall.O_RDONLY, 0)
-			require.EqualErrno(t, 0, errno)
+			f := openFsFile(t, file, syscall.O_RDONLY, 0)
 			defer f.Close()
 
 			st, errno := f.Stat()
@@ -279,25 +273,21 @@ func Test_StatFile_times(t *testing.T) {
 
 func TestStatFile_dev_inode(t *testing.T) {
 	tmpDir := t.TempDir()
-	d, errno := OpenFile(tmpDir, os.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	d := openFsFile(t, tmpDir, os.O_RDONLY, 0)
 	defer d.Close()
 
 	path1 := path.Join(tmpDir, "1")
-	f1, errno := OpenFile(path1, os.O_CREATE, 0o666)
-	require.EqualErrno(t, 0, errno)
+	f1 := openFsFile(t, path1, os.O_CREATE, 0o666)
 	defer f1.Close()
 
 	path2 := path.Join(tmpDir, "2")
-	f2, errno := OpenFile(path2, os.O_CREATE, 0o666)
-	require.EqualErrno(t, 0, errno)
+	f2 := openFsFile(t, path2, os.O_CREATE, 0o666)
 	defer f2.Close()
 
 	pathLink2 := path.Join(tmpDir, "link2")
 	err := os.Symlink(path2, pathLink2)
 	require.NoError(t, err)
-	l2, errno := OpenFile(pathLink2, os.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	l2 := openFsFile(t, pathLink2, os.O_RDONLY, 0)
 	defer l2.Close()
 
 	// First, stat the directory
@@ -334,8 +324,7 @@ func TestStatFile_dev_inode(t *testing.T) {
 
 	// Renaming a file shouldn't change its inodes.
 	require.Zero(t, Rename(path1, path2))
-	f1, errno = OpenFile(path2, os.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	f1 = openFsFile(t, path2, os.O_RDONLY, 0)
 	defer f1.Close()
 
 	st1Again, errno = f1.Stat()
diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go
index b961a06d..147cbb1e 100644
--- a/internal/sys/fs_test.go
+++ b/internal/sys/fs_test.go
@@ -19,9 +19,9 @@ import (
 )
 
 var (
-	noopStdin  = &FileEntry{Name: "stdin", File: &platform.DefaultFile{F: NewStdioFileReader(eofReader{}, noopStdinStat, PollerDefaultStdin)}}
-	noopStdout = &FileEntry{Name: "stdout", File: &platform.DefaultFile{F: &stdioFileWriter{w: io.Discard, s: noopStdoutStat}}}
-	noopStderr = &FileEntry{Name: "stderr", File: &platform.DefaultFile{F: &stdioFileWriter{w: io.Discard, s: noopStderrStat}}}
+	noopStdin  = &FileEntry{Name: "stdin", File: platform.NewFsFile("", NewStdioFileReader(eofReader{}, noopStdinStat, PollerDefaultStdin))}
+	noopStdout = &FileEntry{Name: "stdout", File: platform.NewFsFile("", &stdioFileWriter{w: io.Discard, s: noopStdoutStat})}
+	noopStderr = &FileEntry{Name: "stderr", File: platform.NewFsFile("", &stdioFileWriter{w: io.Discard, s: noopStderrStat})}
 )
 
 //go:embed testdata