Repository: tetratelabs/wazero
Pull request #1548 (2023-06-30 03:23:16)

Simplifies filesystem implementations
This performs a number of simplifying changes to files, particularly around directories. Some of them against my earlier advice to @evacchi!

* use POSIX errors only for readdir.
* stop using a separate subtype for windows osFile as the code is simpler with an if statement.
* stop using a separate table for directory stream descriptors. We aren't yet testing this works, so it is distracting. It was my bad advice to start doing this.

codefromthecrypt: made a mistake. I will debug on my windows box and mark undraft after
codefromthecrypt: oops zig test failed. I'll backfill one so we don't have to wait so long

```
1105/2509 test.Dir.Iterator but dir is deleted during iteration... reached unreachable codeerror instantiating wasm binary: module[] function[_start] failed: wasm error: unreachable
wasm stack trace:
	.os.abort()
		0x48ee: /home/runner/zig-source/lib/std/os.zig:611:32
	.builtin.default_panic(i32,i32,i32,i32)
		0x44c: /home/runner/zig-source/lib/std/builtin.zig:894:25
	.fs.IterableDir.Iterator__struct_94278.nextWasi(i32,i32)
		0xfc93ce: /home/runner/zig-source/lib/std/fs.zig:825:39
	.fs.IterableDir.Iterator__struct_94278.next(i32,i32)
		0xfc8a06: /home/runner/zig-source/lib/std/fs.zig:797:37
	.fs.test.test.Dir.Iterator but dir is deleted during iteration() i32
		0xfcd3f7: /home/runner/zig-source/lib/std/fs/test.zig:310:36
	.test_runner.mainTerminal()
		0x3e04: /home/runner/zig-source/lib/test_runner.zig:175:28
	.test_runner.main()
		0x396: /home/runner/zig-source/lib/test_runner.zig:35:28
	._start()
		0x1d: /home/runner/zig-source/lib/std/start.zig:599:22 (inlined)
		      /home/runner/zig-source/lib/std/start.zig:232:42
```
codefromthecrypt: self-note: the test I need to add instead of wait for zig to fail is readdir after close, like this..

https://github.com/ziglang/zig/blob/master/lib/std/fs/test.zig#L290-L317

```
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=1045884,buf_len=1024,cookie=0)
<== (bufused=,errno=EINVAL)
```

Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index c87d7699..4f5f3fb6 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -858,16 +858,12 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) syscall.Err
 		return syscall.EINVAL
 	}
 
-	// Validate the FD is a directory
-	f, errno := openedDir(fsc, fd)
-	if errno != 0 {
-		return errno
-	}
-	// Discard the bool value because we validated the fd already.
-	dir, errno := fsc.LookupReaddir(fd, f)
+	// Get or open the directory
+	dir, errno := openDir(fsc, fd)
 	if errno != 0 {
 		return errno
 	}
+
 	// Validate the cookie and possibly sync the internal state to the one the cookie represents.
 	if errno = dir.Rewind(cookie); errno != 0 {
 		return errno
@@ -1010,22 +1006,12 @@ func writeDirent(buf []byte, dNext uint64, ino uint64, dNamlen uint32, dType fs.
 	le.PutUint32(buf[20:], uint32(filetype)) //  d_type
 }
 
-// openedDir returns the sys.FileEntry for the directory and 0 if the fd points to a readable directory.
-func openedDir(fsc *sys.FSContext, fd int32) (*sys.FileEntry, syscall.Errno) {
+// openDir lazy opens a sys.Readdir for the directory or returns an error.
+func openDir(fsc *sys.FSContext, fd int32) (*sys.Readdir, syscall.Errno) {
 	if f, ok := fsc.LookupFile(fd); !ok {
 		return nil, syscall.EBADF
-	} else if isDir, errno := f.File.IsDir(); errno != 0 {
-		return nil, errno
-	} else if !isDir {
-		// fd_readdir docs don't indicate whether to return syscall.ENOTDIR or
-		// syscall.EBADF. It has been noticed that rust will crash on syscall.ENOTDIR,
-		// and POSIX C ref seems to not return this, so we don't either.
-		//
-		// See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#fd_readdir
-		// and https://en.wikibooks.org/wiki/C_Programming/POSIX_Reference/dirent.h
-		return nil, syscall.EBADF
 	} else {
-		return f, 0
+		return f.OpenDir(true)
 	}
 }
 
diff --git a/internal/fsapi/dir.go b/internal/fsapi/dir.go
index c28783b4..3ad75b94 100644
--- a/internal/fsapi/dir.go
+++ b/internal/fsapi/dir.go
@@ -9,20 +9,22 @@ import (
 
 // Dirent is an entry read from a directory.
 //
-// This is a portable variant of syscall.Dirent containing fields needed for
-// WebAssembly ABI including WASI snapshot-01 and wasi-filesystem. Unlike
-// fs.DirEntry, this may include the Ino.
+// # Notes
+//
+//   - This extends `dirent` defined in POSIX with some fields defined by
+//     Linux. See https://man7.org/linux/man-pages/man3/readdir.3.html and
+//     https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html
 type Dirent struct {
-	// ^^ Dirent name matches syscall.Dirent
-
-	// Name is the base name of the directory entry.
-	Name string
-
 	// Ino is the file serial number, or zero if not available.
 	Ino uint64
 
+	// Name is the base name of the directory entry. Empty is invalid.
+	Name string
+
 	// Type is fs.FileMode masked on fs.ModeType. For example, zero is a
 	// regular file, fs.ModeDir is a directory and fs.ModeIrregular is unknown.
+	//
+	// Note: This is defined by Linux, not POSIX.
 	Type fs.FileMode
 }
 
diff --git a/internal/fsapi/file.go b/internal/fsapi/file.go
index 2d6ad037..ba454acd 100644
--- a/internal/fsapi/file.go
+++ b/internal/fsapi/file.go
@@ -24,8 +24,11 @@ import (
 //
 // # Notes
 //
-// A writable filesystem abstraction is not yet implemented as of Go 1.20. See
-// https://github.com/golang/go/issues/45757
+//   - You must call Close to avoid file resource conflicts. For example,
+//     Windows cannot delete the underlying directory while a handle to it
+//     remains open.
+//   - A writable filesystem abstraction is not yet implemented as of Go 1.20.
+//     See https://github.com/golang/go/issues/45757
 type File interface {
 	// Ino returns the inode (Stat_t.Ino) of this file, zero if unknown or an
 	// error there was an error retrieving it.
@@ -216,15 +219,13 @@ type File interface {
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
 	//   - syscall.ENOSYS: the implementation does not support this function.
-	//   - syscall.ENOTDIR: the file was not a directory
+	//   - syscall.EBADF: the file was closed or not a directory.
+	//   - syscall.ENOENT: the directory could not be read (e.g. deleted).
 	//
 	// # Notes
 	//
 	//   - This is like `Readdir` on os.File, but unlike `readdir` in POSIX.
 	//     See https://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir.html
-	//   - For portability reasons, no error is returned at the end of the
-	//     directory, when the file is closed or removed while open.
-	//     See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
 	Readdir(n int) (dirents []Dirent, errno syscall.Errno)
 	// ^-- TODO: consider being more like POSIX, for example, returning a
 	// closeable Dirent object that can iterate on demand. This would
@@ -371,8 +372,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.
+	// A zero syscall.Errno is returned if unimplemented or success.
 	//
 	// # Notes
 	//
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index 760de604..1f1856bb 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/tetratelabs/wazero/internal/descriptor"
 	"github.com/tetratelabs/wazero/internal/fsapi"
-	"github.com/tetratelabs/wazero/internal/platform"
 	socketapi "github.com/tetratelabs/wazero/internal/sock"
 	"github.com/tetratelabs/wazero/internal/sysfs"
 )
@@ -53,6 +52,35 @@ type FileEntry struct {
 
 	// File is always non-nil.
 	File fsapi.File
+
+	// openDir is nil until OpenDir was called.
+	openDir *Readdir
+}
+
+// OpenDir lazy creates a directory stream for this file.
+//
+// # Parameters
+//
+// When `addDotEntries` is true, navigational entries "." and ".." precede
+// any other entries in the directory. Otherwise, they will be absent.
+//
+// # Errors
+//
+// # This returns the same errors as fsapi.File Readdir
+//
+// Notes:
+//   - In the future, this may be refactored to allow multiple open directory
+//     streams per file (likely in wasip>1).
+func (f *FileEntry) OpenDir(addDotEntries bool) (dir *Readdir, errno syscall.Errno) {
+	if dir = f.openDir; dir != nil {
+		return dir, 0
+	} else if dir, errno = newReaddirFromFileEntry(f, addDotEntries); errno != 0 {
+		// not a directory or error reading it.
+		return nil, errno
+	} else {
+		f.openDir = dir
+		return dir, 0
+	}
 }
 
 const direntBufSize = 16
@@ -134,11 +162,14 @@ func (d *Readdir) init() syscall.Errno {
 }
 
 // newReaddirFromFileEntry is a constructor for Readdir that takes a FileEntry to initialize.
-func newReaddirFromFileEntry(f *FileEntry) (*Readdir, syscall.Errno) {
-	// Generate the dotEntries only once and return it many times in the dirInit closure.
-	dotEntries, errno := synthesizeDotEntries(f)
-	if errno != 0 {
-		return nil, errno
+func newReaddirFromFileEntry(f *FileEntry, addDotEntries bool) (*Readdir, syscall.Errno) {
+	var dotEntries []fsapi.Dirent
+	if addDotEntries {
+		// Generate the dotEntries only once and return it many times in the dirInit closure.
+		var errno syscall.Errno
+		if dotEntries, errno = synthesizeDotEntries(f); errno != 0 {
+			return nil, errno
+		}
 	}
 	dirInit := func() ([]fsapi.Dirent, syscall.Errno) {
 		// Ensure we always rewind to the beginning when we re-init.
@@ -259,21 +290,12 @@ type FSContext struct {
 	// (or directories) and defaults to empty.
 	// TODO: This is unguarded, so not goroutine-safe!
 	openedFiles FileTable
-
-	// readdirs is a map of numeric identifiers to Readdir structs
-	// and defaults to empty.
-	// TODO: This is unguarded, so not goroutine-safe!
-	readdirs ReaddirTable
 }
 
 // FileTable is a specialization of the descriptor.Table type used to map file
 // descriptors to file entries.
 type FileTable = descriptor.Table[int32, *FileEntry]
 
-// ReaddirTable is a specialization of the descriptor.Table type used to map
-// file descriptors to Readdir structs.
-type ReaddirTable = descriptor.Table[int32, *Readdir]
-
 // RootFS returns a possibly unimplemented root filesystem. Any files that
 // should be added to the table should be inserted via InsertFile.
 //
@@ -287,6 +309,11 @@ func (c *FSContext) RootFS() fsapi.FS {
 	}
 }
 
+// LookupFile returns a file if it is in the table.
+func (c *FSContext) LookupFile(fd int32) (*FileEntry, bool) {
+	return c.openedFiles.Lookup(fd)
+}
+
 // OpenFile opens the file into the table and returns its file descriptor.
 // The result must be closed by CloseFile or Close.
 func (c *FSContext) OpenFile(fs fsapi.FS, path string, flag int, perm fs.FileMode) (int32, syscall.Errno) {
@@ -307,6 +334,34 @@ func (c *FSContext) OpenFile(fs fsapi.FS, path string, flag int, perm fs.FileMod
 	}
 }
 
+// Renumber assigns the file pointed by the descriptor `from` to `to`.
+func (c *FSContext) Renumber(from, to int32) syscall.Errno {
+	fromFile, ok := c.openedFiles.Lookup(from)
+	if !ok || to < 0 {
+		return syscall.EBADF
+	} else if fromFile.IsPreopen {
+		return syscall.ENOTSUP
+	}
+
+	// If toFile is already open, we close it to prevent windows lock issues.
+	//
+	// The doc is unclear and other implementations do nothing for already-opened To FDs.
+	// https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-fd_renumberfd-fd-to-fd---errno
+	// https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasi-common/src/snapshots/preview_1.rs#L531-L546
+	if toFile, ok := c.openedFiles.Lookup(to); ok {
+		if toFile.IsPreopen {
+			return syscall.ENOTSUP
+		}
+		_ = toFile.File.Close()
+	}
+
+	c.openedFiles.Delete(from)
+	if !c.openedFiles.InsertAt(fromFile, to) {
+		return syscall.EBADF
+	}
+	return 0
+}
+
 // SockAccept accepts a socketapi.TCPConn into the file table and returns
 // its file descriptor.
 func (c *FSContext) SockAccept(sockFD int32, nonblock bool) (int32, syscall.Errno) {
@@ -336,77 +391,17 @@ func (c *FSContext) SockAccept(sockFD int32, nonblock bool) (int32, syscall.Errn
 	}
 }
 
-// LookupFile returns a file if it is in the table.
-func (c *FSContext) LookupFile(fd int32) (*FileEntry, bool) {
-	return c.openedFiles.Lookup(fd)
-}
-
-// LookupReaddir returns a Readdir struct or creates an empty one if it was not present.
-//
-// Note: this currently assumes that idx == fd, where fd is the file descriptor of the directory.
-// CloseFile will delete this idx from the internal store. In the future, idx may be independent
-// of a file fd, and the idx may have to be disposed with an explicit CloseReaddir.
-func (c *FSContext) LookupReaddir(idx int32, f *FileEntry) (*Readdir, syscall.Errno) {
-	if item, _ := c.readdirs.Lookup(idx); item != nil {
-		return item, 0
-	} else {
-		item, err := newReaddirFromFileEntry(f)
-		if err != 0 {
-			return nil, err
-		}
-		ok := c.readdirs.InsertAt(item, idx)
-		if !ok {
-			return nil, syscall.EINVAL
-		}
-		return item, 0
-	}
-}
-
-// CloseReaddir delete the Readdir struct at the given index
-//
-// Note: Currently only necessary in tests. In the future, the idx will have to be disposed explicitly,
-// unless we maintain a map fd -> []idx, and we let CloseFile close all the idx in []idx.
-func (c *FSContext) CloseReaddir(idx int32) {
-	c.readdirs.Delete(idx)
-}
-
-// Renumber assigns the file pointed by the descriptor `from` to `to`.
-func (c *FSContext) Renumber(from, to int32) syscall.Errno {
-	fromFile, ok := c.openedFiles.Lookup(from)
-	if !ok || to < 0 {
-		return syscall.EBADF
-	} else if fromFile.IsPreopen {
-		return syscall.ENOTSUP
-	}
-
-	// If toFile is already open, we close it to prevent windows lock issues.
-	//
-	// The doc is unclear and other implementations do nothing for already-opened To FDs.
-	// https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-fd_renumberfd-fd-to-fd---errno
-	// https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasi-common/src/snapshots/preview_1.rs#L531-L546
-	if toFile, ok := c.openedFiles.Lookup(to); ok {
-		if toFile.IsPreopen {
-			return syscall.ENOTSUP
-		}
-		_ = toFile.File.Close()
-	}
-
-	c.openedFiles.Delete(from)
-	if !c.openedFiles.InsertAt(fromFile, to) {
-		return syscall.EBADF
-	}
-	return 0
-}
-
 // CloseFile returns any error closing the existing file.
-func (c *FSContext) CloseFile(fd int32) syscall.Errno {
+func (c *FSContext) CloseFile(fd int32) (errno syscall.Errno) {
 	f, ok := c.openedFiles.Lookup(fd)
 	if !ok {
 		return syscall.EBADF
 	}
+	if errno = f.File.Close(); errno != 0 {
+		return errno
+	}
 	c.openedFiles.Delete(fd)
-	c.readdirs.Delete(fd)
-	return platform.UnwrapOSError(f.File.Close())
+	return errno
 }
 
 // Close implements io.Closer
@@ -418,10 +413,8 @@ func (c *FSContext) Close() (err error) {
 		}
 		return true
 	})
-	// A closed FSContext cannot be reused so clear the state instead of
-	// using Reset.
+	// A closed FSContext cannot be reused so clear the state.
 	c.openedFiles = FileTable{}
-	c.readdirs = ReaddirTable{}
 	return
 }
 
diff --git a/internal/sysfs/dir.go b/internal/sysfs/dir.go
index 8462e846..c3256f9c 100644
--- a/internal/sysfs/dir.go
+++ b/internal/sysfs/dir.go
@@ -13,12 +13,12 @@ func adjustReaddirErr(f fsapi.File, isClosed bool, err error) syscall.Errno {
 		return 0 // e.g. Readdir on darwin returns io.EOF, but linux doesn't.
 	} else if errno := platform.UnwrapOSError(err); errno != 0 {
 		errno = dirError(f, isClosed, errno)
-		// Ignore errors when the file was closed or removed.
+		// Comply with errors allowed on fsapi.File Readdir
 		switch errno {
-		case syscall.EIO, syscall.EBADF: // closed while open
-			return 0
-		case syscall.ENOENT: // Linux error when removed while open
-			return 0
+		case syscall.EINVAL: // os.File Readdir can return this
+			return syscall.EBADF
+		case syscall.ENOTDIR: // dirError can return this
+			return syscall.EBADF
 		}
 		return errno
 	}
diff --git a/internal/sysfs/file.go b/internal/sysfs/file.go
index 913b9fd1..d6f975e6 100644
--- a/internal/sysfs/file.go
+++ b/internal/sysfs/file.go
@@ -137,7 +137,7 @@ func (f *fsFile) cachedStat() (fileType fs.FileMode, ino uint64, errno syscall.E
 	return f.cachedSt.fileType, f.cachedSt.ino, 0
 }
 
-// Ino implements File.Ino
+// Ino implements the same method as documented on fsapi.File
 func (f *fsFile) Ino() (uint64, syscall.Errno) {
 	if _, ino, errno := f.cachedStat(); errno != 0 {
 		return 0, errno
@@ -146,17 +146,17 @@ func (f *fsFile) Ino() (uint64, syscall.Errno) {
 	}
 }
 
-// IsAppend implements File.IsAppend
+// IsAppend implements the same method as documented on fsapi.File
 func (f *fsFile) IsAppend() bool {
 	return false
 }
 
-// SetAppend implements File.SetAppend
+// SetAppend implements the same method as documented on fsapi.File
 func (f *fsFile) SetAppend(bool) (errno syscall.Errno) {
 	return fileError(f, f.closed, syscall.ENOSYS)
 }
 
-// IsDir implements File.IsDir
+// IsDir implements the same method as documented on fsapi.File
 func (f *fsFile) IsDir() (bool, syscall.Errno) {
 	if ft, _, errno := f.cachedStat(); errno != 0 {
 		return false, errno
@@ -166,7 +166,7 @@ func (f *fsFile) IsDir() (bool, syscall.Errno) {
 	return false, 0
 }
 
-// Stat implements File.Stat
+// Stat implements the same method as documented on fsapi.File
 func (f *fsFile) Stat() (st fsapi.Stat_t, errno syscall.Errno) {
 	if f.closed {
 		errno = syscall.EBADF
@@ -195,7 +195,7 @@ func (f *fsFile) cacheStat(st fsapi.Stat_t) (fsapi.Stat_t, syscall.Errno) {
 	return st, 0
 }
 
-// Read implements File.Read
+// Read implements the same method as documented on fsapi.File
 func (f *fsFile) Read(buf []byte) (n int, errno syscall.Errno) {
 	if n, errno = read(f.file, buf); errno != 0 {
 		// Defer validation overhead until we've already had an error.
@@ -204,7 +204,7 @@ func (f *fsFile) Read(buf []byte) (n int, errno syscall.Errno) {
 	return
 }
 
-// Pread implements File.Pread
+// Pread implements the same method as documented on fsapi.File
 func (f *fsFile) Pread(buf []byte, off int64) (n int, errno syscall.Errno) {
 	if ra, ok := f.file.(io.ReaderAt); ok {
 		if n, errno = pread(ra, buf, off); errno != 0 {
@@ -243,7 +243,7 @@ func (f *fsFile) Pread(buf []byte, off int64) (n int, errno syscall.Errno) {
 	return
 }
 
-// Seek implements File.Seek.
+// Seek implements the same method as documented on fsapi.File
 func (f *fsFile) Seek(offset int64, whence int) (newOffset int64, errno syscall.Errno) {
 	// If this is a directory, and we're attempting to seek to position zero,
 	// we have to re-open the file to ensure the directory state is reset.
@@ -267,16 +267,17 @@ func (f *fsFile) Seek(offset int64, whence int) (newOffset int64, errno syscall.
 	return
 }
 
-func (f *fsFile) reopen() syscall.Errno {
-	_ = f.close()
-	var err error
-	f.file, err = f.fs.Open(f.name)
-	return platform.UnwrapOSError(err)
-}
-
 // Readdir implements File.Readdir. Notably, this uses fs.ReadDirFile if
 // available.
 func (f *fsFile) Readdir(n int) (dirents []fsapi.Dirent, errno syscall.Errno) {
+	// Windows lets you Readdir after close, fs.File also may not implement
+	// close in a meaningful way. read our closed field to return consistent
+	// results.
+	if f.closed {
+		errno = syscall.EBADF
+		return
+	}
+
 	if of, ok := f.file.(*os.File); ok {
 		// We can't use f.name here because it is the path up to the fsapi.FS,
 		// not necessarily the real path. For this reason, Windows may not be
@@ -300,12 +301,12 @@ func (f *fsFile) Readdir(n int) (dirents []fsapi.Dirent, errno syscall.Errno) {
 			dirents = append(dirents, fsapi.Dirent{Name: e.Name(), Type: e.Type()})
 		}
 	} else {
-		errno = syscall.ENOTDIR
+		errno = syscall.EBADF // not a directory
 	}
 	return
 }
 
-// Write implements File.Write
+// Write implements the same method as documented on fsapi.File.
 func (f *fsFile) Write(buf []byte) (n int, errno syscall.Errno) {
 	if w, ok := f.file.(io.Writer); ok {
 		if n, errno = write(w, buf); errno != 0 {
@@ -318,7 +319,7 @@ func (f *fsFile) Write(buf []byte) (n int, errno syscall.Errno) {
 	return
 }
 
-// Pwrite implements File.Pwrite
+// Pwrite implements the same method as documented on fsapi.File.
 func (f *fsFile) Pwrite(buf []byte, off int64) (n int, errno syscall.Errno) {
 	if wa, ok := f.file.(io.WriterAt); ok {
 		if n, errno = pwrite(wa, buf, off); errno != 0 {
@@ -331,7 +332,7 @@ func (f *fsFile) Pwrite(buf []byte, off int64) (n int, errno syscall.Errno) {
 	return
 }
 
-// Close implements File.Close
+// Close implements the same method as documented on fsapi.File.
 func (f *fsFile) Close() syscall.Errno {
 	if f.closed {
 		return 0
@@ -406,6 +407,21 @@ func seek(s io.Seeker, offset int64, whence int) (int64, syscall.Errno) {
 	return newOffset, platform.UnwrapOSError(err)
 }
 
+// reopenFile allows re-opening a file for reasons such as applying flags or
+// directory iteration.
+type reopenFile func() syscall.Errno
+
+// compile-time check to ensure fsFile.reopen implements reopenFile.
+var _ reopenFile = (*fsFile)(nil).reopen
+
+// reopen implements the same method as documented on reopenFile.
+func (f *fsFile) reopen() syscall.Errno {
+	_ = f.close()
+	var err error
+	f.file, err = f.fs.Open(f.name)
+	return platform.UnwrapOSError(err)
+}
+
 func readdir(f *os.File, path string, n int) (dirents []fsapi.Dirent, errno syscall.Errno) {
 	fis, e := f.Readdir(n)
 	if errno = platform.UnwrapOSError(e); errno != 0 {
diff --git a/internal/sysfs/open_file.go b/internal/sysfs/open_file.go
index 473f8ca6..ebe9e3d5 100644
--- a/internal/sysfs/open_file.go
+++ b/internal/sysfs/open_file.go
@@ -7,14 +7,9 @@ import (
 	"os"
 	"syscall"
 
-	"github.com/tetratelabs/wazero/internal/fsapi"
 	"github.com/tetratelabs/wazero/internal/platform"
 )
 
-func newOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File {
-	return newDefaultOsFile(openPath, openFlag, openPerm, f)
-}
-
 // 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) (*os.File, syscall.Errno) {
diff --git a/internal/sysfs/open_file_js.go b/internal/sysfs/open_file_js.go
index e473acbe..d84e2a37 100644
--- a/internal/sysfs/open_file_js.go
+++ b/internal/sysfs/open_file_js.go
@@ -8,10 +8,6 @@ import (
 	"github.com/tetratelabs/wazero/internal/platform"
 )
 
-func newOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) File {
-	return newDefaultOsFile(openPath, openFlag, openPerm, f)
-}
-
 func openFile(path string, flag int, perm fs.FileMode) (*os.File, syscall.Errno) {
 	flag &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
 	f, err := os.OpenFile(path, flag, perm)
diff --git a/internal/sysfs/open_file_sun.go b/internal/sysfs/open_file_sun.go
index e23b7185..576c8085 100644
--- a/internal/sysfs/open_file_sun.go
+++ b/internal/sysfs/open_file_sun.go
@@ -7,14 +7,9 @@ import (
 	"os"
 	"syscall"
 
-	"github.com/tetratelabs/wazero/internal/fsapi"
 	"github.com/tetratelabs/wazero/internal/platform"
 )
 
-func newOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File {
-	return newDefaultOsFile(openPath, openFlag, openPerm, f)
-}
-
 func openFile(path string, flag int, perm fs.FileMode) (*os.File, syscall.Errno) {
 	f, err := os.OpenFile(path, flag, perm)
 	return f, platform.UnwrapOSError(err)
diff --git a/internal/sysfs/open_file_windows.go b/internal/sysfs/open_file_windows.go
index d9297d7e..5000ae85 100644
--- a/internal/sysfs/open_file_windows.go
+++ b/internal/sysfs/open_file_windows.go
@@ -11,12 +11,6 @@ import (
 	"github.com/tetratelabs/wazero/internal/platform"
 )
 
-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, fd: f.Fd()},
-	}
-}
-
 func openFile(path string, flag int, perm fs.FileMode) (*os.File, syscall.Errno) {
 	isDir := flag&fsapi.O_DIRECTORY > 0
 	flag &= ^(fsapi.O_DIRECTORY | fsapi.O_NOFOLLOW) // erase placeholders
@@ -154,46 +148,3 @@ func open(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
 	h, e := syscall.CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0)
 	return h, e
 }
-
-// windowsOsFile overrides osFile to special case directory handling in Windows.
-type windowsOsFile struct {
-	osFile
-
-	dirInitialized bool
-}
-
-// Readdir implements File.Readdir
-func (f *windowsOsFile) Readdir(n int) (dirents []fsapi.Dirent, errno syscall.Errno) {
-	if errno = f.maybeInitDir(); errno != 0 {
-		return
-	}
-
-	return f.osFile.Readdir(n)
-}
-
-func (f *windowsOsFile) maybeInitDir() syscall.Errno {
-	if f.dirInitialized {
-		return 0
-	}
-
-	if isDir, errno := f.IsDir(); errno != 0 {
-		return errno
-	} else if !isDir {
-		return syscall.ENOTDIR
-	}
-
-	// On Windows, once the directory is opened, changes to the directory are
-	// not visible on ReadDir on that already-opened file handle.
-	//
-	// To provide consistent behavior with other platforms, we re-open it.
-	if errno := f.osFile.Close(); errno != 0 {
-		return errno
-	}
-	newW, errno := openFile(f.path, f.flag, f.perm)
-	if errno != 0 {
-		return errno
-	}
-	f.osFile.file = newW
-	f.dirInitialized = true
-	return 0
-}
diff --git a/internal/sysfs/osfile.go b/internal/sysfs/osfile.go
index 95798f48..74436e41 100644
--- a/internal/sysfs/osfile.go
+++ b/internal/sysfs/osfile.go
@@ -4,6 +4,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"runtime"
 	"syscall"
 	"time"
 
@@ -11,8 +12,12 @@ import (
 	"github.com/tetratelabs/wazero/internal/platform"
 )
 
-func newDefaultOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File {
-	return &osFile{path: openPath, flag: openFlag, perm: openPerm, file: f, fd: f.Fd()}
+func newOsFile(openPath string, openFlag int, openPerm fs.FileMode, f *os.File) fsapi.File {
+	// Windows cannot read files written to a directory after it was opened.
+	// This was noticed in #1087 in zig tests. Use a flag instead of a
+	// different type.
+	reopenDir := runtime.GOOS == "windows"
+	return &osFile{path: openPath, flag: openFlag, perm: openPerm, reopenDir: reopenDir, file: f, fd: f.Fd()}
 }
 
 // osFile is a file opened with this package, and uses os.File or syscalls to
@@ -24,6 +29,9 @@ type osFile struct {
 	file *os.File
 	fd   uintptr
 
+	// reopenDir is true if reopen should be called before Readdir.
+	reopenDir bool
+
 	// closed is true when closed was called. This ensures proper syscall.EBADF
 	closed bool
 
@@ -72,6 +80,9 @@ func (f *osFile) SetAppend(enable bool) (errno syscall.Errno) {
 	return fileError(f, f.closed, f.reopen())
 }
 
+// compile-time check to ensure osFile.reopen implements reopenFile.
+var _ reopenFile = (*fsFile)(nil).reopen
+
 func (f *osFile) reopen() (errno syscall.Errno) {
 	// Clear any create flag, as we are re-opening, not re-creating.
 	f.flag &= ^syscall.O_CREAT
@@ -183,6 +194,13 @@ func (f *osFile) PollRead(timeout *time.Duration) (ready bool, errno syscall.Err
 // Readdir implements File.Readdir. Notably, this uses "Readdir", not
 // "ReadDir", from os.File.
 func (f *osFile) Readdir(n int) (dirents []fsapi.Dirent, errno syscall.Errno) {
+	if f.reopenDir { // Lazy re-open the directory if needed.
+		f.reopenDir = false
+		if errno = adjustReaddirErr(f, f.closed, f.reopen()); errno != 0 {
+			return
+		}
+	}
+
 	if dirents, errno = readdir(f.file, f.path, n); errno != 0 {
 		errno = adjustReaddirErr(f, f.closed, errno)
 	}


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index c9d7c548..c6496c6f 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -2013,9 +2013,7 @@ func Test_fdReaddir(t *testing.T) {
 
 	fsc := mod.(*wasm.ModuleInstance).Sys.FS()
 	preopen := fsc.RootFS()
-
-	fd, errno := fsc.OpenFile(preopen, "dir", os.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	fd := sys.FdPreopen + 1
 
 	tests := []struct {
 		name            string
@@ -2069,7 +2067,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				_ = rdd.Advance()
 			},
 
@@ -2084,7 +2082,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				_ = rdd.Advance()
 			},
 			bufLen:          30, // length is longer than the second entry, but not long enough for a header.
@@ -2099,7 +2097,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				_ = rdd.Advance()
 			},
 			bufLen:          50, // length is longer than the second entry + enough for the header of third.
@@ -2113,7 +2111,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				_ = rdd.Advance()
 			},
 			bufLen:          53, // length is long enough for second and third.
@@ -2127,7 +2125,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				rdd.Skip(2)
 			},
 			bufLen:          27, // length is long enough for exactly third.
@@ -2141,7 +2139,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				rdd.Skip(2)
 			},
 			bufLen:          300, // length is long enough for third and more
@@ -2155,7 +2153,7 @@ func Test_fdReaddir(t *testing.T) {
 			initialDir: "dir",
 			dir: func() {
 				f, _ := fsc.LookupFile(fd)
-				rdd, _ := fsc.LookupReaddir(fd, f)
+				rdd, _ := f.OpenDir(true)
 				rdd.Skip(5)
 			},
 			bufLen:          300, // length is long enough for third and more
@@ -2169,12 +2167,12 @@ func Test_fdReaddir(t *testing.T) {
 		tc := tt
 		t.Run(tc.name, func(t *testing.T) {
 			defer log.Reset()
-			defer fsc.CloseReaddir(fd)
 
-			dir, errno := preopen.OpenFile(tc.initialDir, os.O_RDONLY, 0)
+			fd, errno := fsc.OpenFile(preopen, tc.initialDir, os.O_RDONLY, 0)
 			require.EqualErrno(t, 0, errno)
+			defer fsc.CloseFile(fd) // nolint
+
 			file, _ := fsc.LookupFile(fd)
-			file.File = dir
 
 			if tc.dir != nil {
 				tc.dir()
@@ -2202,7 +2200,7 @@ func Test_fdReaddir(t *testing.T) {
 				require.Equal(t, tc.expectedMem, mem[:tc.expectedMemSize])
 			}
 
-			rdd, errno := fsc.LookupReaddir(fd, file)
+			rdd, errno := file.OpenDir(true)
 			require.Equal(t, syscall.Errno(0), errno)
 			require.Equal(t, tc.expectedCookie, rdd.Cookie())
 		})
@@ -2282,8 +2280,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
 	fileFD, errno := fsc.OpenFile(preopen, "animals.txt", os.O_RDONLY, 0)
 	require.EqualErrno(t, 0, errno)
 
-	dirFD, errno := fsc.OpenFile(preopen, "dir", os.O_RDONLY, 0)
-	require.EqualErrno(t, 0, errno)
+	// Directories are stateful, so we open them during the test.
+	dirFD := fileFD + 1
 
 	tests := []struct {
 		name                       string
@@ -2380,13 +2378,10 @@ func Test_fdReaddir_Errors(t *testing.T) {
 			defer log.Reset()
 
 			// Reset the directory so that tests don't taint each other.
-			if file, ok := fsc.LookupFile(tc.fd); ok && tc.fd == dirFD {
-				dir, errno := preopen.OpenFile("dir", os.O_RDONLY, 0)
+			if tc.fd == dirFD {
+				dirFD, errno = fsc.OpenFile(preopen, "dir", os.O_RDONLY, 0)
 				require.EqualErrno(t, 0, errno)
-				defer dir.Close()
-
-				file.File = dir
-				fsc.CloseReaddir(tc.fd)
+				defer fsc.CloseFile(dirFD) // nolint
 			}
 
 			requireErrnoResult(t, tc.expectedErrno, mod, wasip1.FdReaddirName,
diff --git a/imports/wasi_snapshot_preview1/wasi_bench_test.go b/imports/wasi_snapshot_preview1/wasi_bench_test.go
index 8d735659..9302b9d8 100644
--- a/imports/wasi_snapshot_preview1/wasi_bench_test.go
+++ b/imports/wasi_snapshot_preview1/wasi_bench_test.go
@@ -219,7 +219,6 @@ func Benchmark_fdReaddir(b *testing.B) {
 				if f.File, errno = fsc.RootFS().OpenFile(".", os.O_RDONLY, 0); errno != 0 {
 					b.Fatal(errno)
 				}
-				fsc.CloseReaddir(fd)
 
 				// Make an initial call to build the state of an unread directory
 				if bc.continued {
diff --git a/internal/sysfs/dir_test.go b/internal/sysfs/dir_test.go
index 937afac3..dfa8fe1a 100644
--- a/internal/sysfs/dir_test.go
+++ b/internal/sysfs/dir_test.go
@@ -4,7 +4,6 @@ import (
 	"io"
 	"io/fs"
 	"os"
-	"path"
 	"runtime"
 	"sort"
 	"syscall"
@@ -57,11 +56,12 @@ func TestReaddir(t *testing.T) {
 				testReaddirAll(t, dotF, tc.expectIno)
 			})
 
-			// Don't err if something else closed the directory while reading.
+			// Err if the caller closed the directory while reading. This is
+			// different from something else deleting it.
 			t.Run("closed dir", func(t *testing.T) {
 				require.EqualErrno(t, 0, dotF.Close())
 				_, errno := dotF.Readdir(-1)
-				require.EqualErrno(t, 0, errno)
+				require.EqualErrno(t, syscall.EBADF, errno)
 			})
 
 			fileF, errno := sysfs.OpenFSFile(tc.fs, "empty.txt", syscall.O_RDONLY, 0)
@@ -70,7 +70,7 @@ func TestReaddir(t *testing.T) {
 
 			t.Run("file", func(t *testing.T) {
 				_, errno := fileF.Readdir(-1)
-				require.EqualErrno(t, syscall.ENOTDIR, errno)
+				require.EqualErrno(t, syscall.EBADF, errno)
 			})
 
 			dirF, errno := sysfs.OpenFSFile(tc.fs, "dir", syscall.O_RDONLY, 0)
@@ -127,30 +127,6 @@ func TestReaddir(t *testing.T) {
 			})
 		})
 	}
-
-	// Don't err if something else removed the directory while reading.
-	t.Run("removed while open", func(t *testing.T) {
-		dirF, errno := sysfs.OpenFSFile(dirFS, "dir", syscall.O_RDONLY, 0)
-		require.EqualErrno(t, 0, errno)
-		defer dirF.Close()
-
-		dirents, errno := dirF.Readdir(1)
-		require.EqualErrno(t, 0, errno)
-		require.Equal(t, 1, len(dirents))
-
-		// Speculatively try to remove even if it won't likely work
-		// on windows.
-		err := os.RemoveAll(path.Join(tmpDir, "dir"))
-		if err != nil && runtime.GOOS == "windows" {
-			t.Skip()
-		} else {
-			require.NoError(t, err)
-		}
-
-		_, errno = dirF.Readdir(1)
-		require.EqualErrno(t, 0, errno)
-		// don't validate the contents as due to caching it might be present.
-	})
 }
 
 func testReaddirAll(t *testing.T, dotF fsapi.File, expectIno bool) {
diff --git a/internal/sysfs/dirfs_test.go b/internal/sysfs/dirfs_test.go
index 5e052ead..705ac0dc 100644
--- a/internal/sysfs/dirfs_test.go
+++ b/internal/sysfs/dirfs_test.go
@@ -36,7 +36,7 @@ func TestNewDirFS(t *testing.T) {
 		d, errno := testFS.OpenFile(".", os.O_RDONLY, 0)
 		require.EqualErrno(t, 0, errno)
 		_, errno = d.Readdir(-1)
-		require.EqualErrno(t, syscall.ENOTDIR, errno)
+		require.EqualErrno(t, syscall.EBADF, errno)
 	})
 }
 
@@ -802,10 +802,7 @@ func TestDirFS_Truncate(t *testing.T) {
 	})
 }
 
-// Test_fdReaddir_opened_file_written ensures that writing files to the already-opened directory
-// is visible. This is significant on Windows.
-// https://github.com/ziglang/zig/blob/2ccff5115454bab4898bae3de88f5619310bc5c1/lib/std/fs/test.zig#L156-L184
-func Test_fdReaddir_opened_file_written(t *testing.T) {
+func TestDirFS_Readdir(t *testing.T) {
 	root := t.TempDir()
 	testFS := NewDirFS(root)
 
@@ -813,21 +810,46 @@ func Test_fdReaddir_opened_file_written(t *testing.T) {
 	errno := testFS.Mkdir(readDirTarget, 0o700)
 	require.EqualErrno(t, 0, errno)
 
-	// Open the directory, before writing files!
+	// Open the empty directory
 	dirFile, errno := testFS.OpenFile(readDirTarget, os.O_RDONLY, 0)
 	require.EqualErrno(t, 0, errno)
 	defer dirFile.Close()
 
-	// Then write a file to the directory.
-	f, err := os.Create(path.Join(root, readDirTarget, "my-file"))
-	require.NoError(t, err)
-	defer f.Close()
+	// Write files to the directory after it is open.
+	require.NoError(t, os.WriteFile(path.Join(root, readDirTarget, "1"), nil, 0o444))
+	require.NoError(t, os.WriteFile(path.Join(root, readDirTarget, "2"), nil, 0o444))
 
-	dirents, errno := dirFile.Readdir(-1)
-	require.EqualErrno(t, 0, errno)
+	// Test files are visible. This fails in windows unless the implementation
+	// re-opens the underlying file.
+	// https://github.com/ziglang/zig/blob/e3736baddb8ecff90f0594be9f604c7484ce9aa2/lib/std/fs/test.zig#L290-L317
+	t.Run("Sees files written after open", func(t *testing.T) {
+		dirents, errno := dirFile.Readdir(1)
+		require.EqualErrno(t, 0, errno)
+
+		require.Equal(t, 1, len(dirents))
+		n := dirents[0].Name
+		switch n {
+		case "1", "2": // order is inconsistent on scratch images.
+		default:
+			require.Equal(t, "1", n)
+		}
+	})
+
+	// Test there is no error reading the directory if it was deleted while
+	// iterating. See docs on Readdir for why in general, but specifically Zig
+	// tests enforce this. This test is Windows sensitive as well.
+	//
+	// https://github.com/ziglang/zig/blob/e3736baddb8ecff90f0594be9f604c7484ce9aa2/lib/std/fs/test.zig#L311C1-L311C1
+	t.Run("syscall.ENOENT or no error, deleted while reading", func(t *testing.T) {
+		require.NoError(t, os.RemoveAll(path.Join(root, readDirTarget)))
 
-	require.Equal(t, 1, len(dirents))
-	require.Equal(t, "my-file", dirents[0].Name)
+		dirents, errno := dirFile.Readdir(-1)
+		if errno != 0 {
+			require.EqualErrno(t, syscall.ENOENT, errno)
+		}
+
+		require.Equal(t, 0, len(dirents))
+	})
 }
 
 func TestDirFS_Link(t *testing.T) {