Repository: tetratelabs/wazero
Pull request #1428 (2023-05-02 08:23:34)

Adds Truncate to platform.File
See #1417

codefromthecrypt: looks like there's disagreement in the syscall.Errno returned when ftruncate acts on a directory not a file. I'll sort it out and mark draft meanwhile
codefromthecrypt: meh fix one another breaks (windows). I'll mark draft again
evacchi: apparently the code on `main` can't acquire this lock, which trickles down into the original error. Your recent change is now acquiring the lock, so it returns with "use of closed file"; code on this branch succeeds in acquiring the lock

```
// 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 the question is whether the behavior on main is actually correct 
codefromthecrypt: will rework this after #1430

Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 127f0049..203da61e 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -20,12 +20,6 @@ import (
 	"github.com/tetratelabs/wazero/internal/wasm"
 )
 
-// The following interfaces are used until we finalize our own FD-scoped file.
-type (
-	// truncateFile is implemented by os.File in file_posix.go
-	truncateFile interface{ Truncate(size int64) error }
-)
-
 // fdAdvise is the WASI function named FdAdviseName which provides file
 // advisory information on a file descriptor.
 //
@@ -101,16 +95,10 @@ func fdAllocateFn(_ context.Context, mod api.Module, params []uint64) syscall.Er
 	}
 
 	if st.Size >= tail {
-		// We already have enough space.
-		return 0
-	}
-
-	osf, ok := f.File.File().(truncateFile)
-	if !ok {
-		return syscall.EBADF
+		return 0 // We already have enough space.
 	}
 
-	return platform.UnwrapOSError(osf.Truncate(tail))
+	return f.File.Truncate(tail)
 }
 
 // fdClose is the WASI function named FdCloseName which closes a file
@@ -436,12 +424,9 @@ func fdFilestatSetSizeFn(_ context.Context, mod api.Module, params []uint64) sys
 	// Check to see if the file descriptor is available
 	if f, ok := fsc.LookupFile(fd); !ok {
 		return syscall.EBADF
-	} else if truncateFile, ok := f.File.File().(truncateFile); !ok {
-		return syscall.EBADF // possibly a fake file
-	} else if err := truncateFile.Truncate(int64(size)); err != nil {
-		return platform.UnwrapOSError(err)
+	} else {
+		return f.File.Truncate(int64(size))
 	}
-	return 0
 }
 
 // fdFilestatSetTimes is the WASI function named functionFdFilestatSetTimes
diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go
index 10fe8b46..ee3ee4bb 100644
--- a/internal/gojs/fs.go
+++ b/internal/gojs/fs.go
@@ -48,12 +48,6 @@ var (
 	oEXCL = float64(os.O_EXCL)
 )
 
-// The following interfaces are used until we finalize our own FD-scoped file.
-type (
-	// truncateFile is implemented by os.File in file_posix.go
-	truncateFile interface{ Truncate(size int64) error }
-)
-
 // jsfs = js.Global().Get("fs") // fs_js.go init
 //
 // js.fsCall conventions:
@@ -594,10 +588,8 @@ func (jsfsFtruncate) invoke(ctx context.Context, mod api.Module, args ...interfa
 	var errno syscall.Errno
 	if f, ok := fsc.LookupFile(fd); !ok {
 		errno = syscall.EBADF
-	} else if truncateFile, ok := f.File.File().(truncateFile); !ok {
-		errno = syscall.EBADF // possibly a fake file
 	} else {
-		errno = platform.UnwrapOSError(truncateFile.Truncate(length))
+		errno = f.File.Truncate(length)
 	}
 
 	return jsfsInvoke(ctx, mod, callback, errno)
diff --git a/internal/platform/file.go b/internal/platform/file.go
index 8bf7fe0c..45711c16 100644
--- a/internal/platform/file.go
+++ b/internal/platform/file.go
@@ -95,6 +95,7 @@ type File interface {
 	//     https://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html
 	//   - This returns with no error instead of syscall.ENOSYS when
 	//     unimplemented. This prevents fake filesystems from erring.
+	//   - Windows does not error when calling Sync on a closed file.
 	Sync() syscall.Errno
 
 	// Datasync synchronizes the data of a file.
@@ -110,8 +111,26 @@ type File interface {
 	//     https://pubs.opengroup.org/onlinepubs/9699919799/functions/fdatasync.html
 	//   - This returns with no error instead of syscall.ENOSYS when
 	//     unimplemented. This prevents fake filesystems from erring.
+	//   - As this is commonly missing, some implementations dispatch to Sync.
 	Datasync() syscall.Errno
 
+	// Truncate truncates a file to a specified length.
+	//
+	// # Errors
+	//
+	// A zero syscall.Errno is success. The below are expected otherwise:
+	//   - syscall.ENOSYS: the implementation does not support this function.
+	//   - syscall.EBADF: the file or directory was closed.
+	//   - syscall.EINVAL: the `size` is negative.
+	//   - syscall.EISDIR: the file was a directory.
+	//
+	// # Notes
+	//
+	//   - This is like syscall.Ftruncate and `ftruncate` in POSIX. See
+	//     https://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html
+	//   - Windows does not error when calling Truncate on a closed file.
+	Truncate(size int64) syscall.Errno
+
 	// Close closes the underlying file.
 	//
 	// A zero syscall.Errno is success. The below are expected otherwise:
@@ -156,6 +175,11 @@ func (UnimplementedFile) Datasync() syscall.Errno {
 	return 0 // not syscall.ENOSYS
 }
 
+// Truncate implements File.Truncate
+func (UnimplementedFile) Truncate(int64) syscall.Errno {
+	return syscall.ENOSYS
+}
+
 func NewFsFile(path string, f fs.File) File {
 	return &fsFile{path, f}
 }
@@ -205,6 +229,24 @@ func (f *fsFile) Datasync() syscall.Errno {
 	return datasync(f.file)
 }
 
+// Truncate implements File.Truncate
+func (f *fsFile) Truncate(size int64) syscall.Errno {
+	if tf, ok := f.file.(truncateFile); ok {
+		errno := UnwrapOSError(tf.Truncate(size))
+		if errno == 0 {
+			return 0
+		}
+
+		// Operating systems return different syscall.Errno instead of EISDIR
+		// double-check on any err until we can assure this per OS.
+		if isOpenDir(f) {
+			return syscall.EISDIR
+		}
+		return errno
+	}
+	return syscall.ENOSYS
+}
+
 // Close implements File.Close
 func (f *fsFile) Close() syscall.Errno {
 	return UnwrapOSError(f.file.Close())
@@ -258,3 +300,10 @@ type (
 	// truncateFile is implemented by os.File in file_posix.go
 	truncateFile interface{ Truncate(size int64) error }
 )
+
+func isOpenDir(f File) bool {
+	if st, statErrno := f.Stat(); statErrno == 0 && st.Mode.IsDir() {
+		return true
+	}
+	return false
+}
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index 2bbcc8a8..8e29e034 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -210,6 +210,11 @@ func (r *lazyDir) Datasync() syscall.Errno {
 	}
 }
 
+// Truncate implements the same method as documented on platform.File
+func (r *lazyDir) Truncate(int64) syscall.Errno {
+	return syscall.EISDIR
+}
+
 // File implements the same method as documented on platform.File
 func (r *lazyDir) File() fs.File {
 	if f, ok := r.file(); !ok {


Test patch:
diff --git a/internal/platform/file_test.go b/internal/platform/file_test.go
index 7520ae75..075401a9 100644
--- a/internal/platform/file_test.go
+++ b/internal/platform/file_test.go
@@ -137,6 +137,77 @@ func testSync(t *testing.T, sync func(File) syscall.Errno) {
 	}
 }
 
+func TestFsFileTruncate(t *testing.T) {
+	content := []byte("123456")
+
+	tests := []struct {
+		name            string
+		size            int64
+		expectedContent []byte
+		expectedErr     error
+	}{
+		{
+			name:            "one less",
+			size:            5,
+			expectedContent: []byte("12345"),
+		},
+		{
+			name:            "same",
+			size:            6,
+			expectedContent: content,
+		},
+		{
+			name:            "zero",
+			size:            0,
+			expectedContent: []byte(""),
+		},
+		{
+			name:            "larger",
+			size:            106,
+			expectedContent: append(content, make([]byte, 100)...),
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			tmpDir := t.TempDir()
+
+			f := openForWrite(t, path.Join(tmpDir, tc.name), content)
+			defer f.Close()
+
+			errno := f.Truncate(tc.size)
+			require.EqualErrno(t, 0, errno)
+
+			actual, err := os.ReadFile(f.Path())
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedContent, actual)
+		})
+	}
+
+	truncateToZero := func(f File) syscall.Errno {
+		return f.Truncate(0)
+	}
+
+	if runtime.GOOS != "windows" {
+		// TODO: os.Truncate on windows can create the file even when it
+		// doesn't exist.
+		testEBADFIfFileClosed(t, truncateToZero)
+	}
+
+	testEISDIR(t, truncateToZero)
+
+	t.Run("negative", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		f := openForWrite(t, path.Join(tmpDir, "truncate"), content)
+		defer f.Close()
+
+		errno := f.Truncate(-1)
+		require.EqualErrno(t, syscall.EINVAL, errno)
+	})
+}
+
 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()
@@ -150,6 +221,15 @@ func testEBADFIfFileClosed(t *testing.T, fn func(File) syscall.Errno) bool {
 	})
 }
 
+func testEISDIR(t *testing.T, fn func(File) syscall.Errno) bool {
+	return t.Run("EISDIR if directory", func(t *testing.T) {
+		f := openFsFile(t, os.TempDir(), os.O_RDONLY|O_DIRECTORY, 0o666)
+		defer f.Close()
+
+		require.EqualErrno(t, syscall.EISDIR, 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)
diff --git a/internal/platform/stat_test.go b/internal/platform/stat_test.go
index 3d9cd217..6d22a264 100644
--- a/internal/platform/stat_test.go
+++ b/internal/platform/stat_test.go
@@ -192,7 +192,7 @@ func TestStatFile(t *testing.T) {
 		require.NotEqual(t, uint64(0), st.Ino)
 	})
 
-	t.Run("closed file", func(t *testing.T) {
+	t.Run("closed fsFile", func(t *testing.T) {
 		require.Zero(t, fileF.Close())
 		_, errno := fileF.Stat()
 		require.EqualErrno(t, syscall.EBADF, errno)