Allow preopen file descriptors to be closed. (#96)

* Allow preopen file descriptors to be closed.

This ports the test updates from bytecodealliance/wasmtime#5828 to the
preview2-prototype and implements the ability to close preopened file
descriptors.

* Fix compilation on Windows.

host/src/filesystem.rs

@@ -213,7 +213,13 @@ impl wasi_filesystem::WasiFilesystem for WasiCtx {
     async fn get_preopens(
         &mut self,
     ) -> Result<Vec<(wasi_filesystem::Descriptor, String)>, anyhow::Error> {
-        Ok(self.preopens.clone())
+        // Create new handles to the preopens.
+        let mut results = Vec::new();
+        for (handle, name) in &self.preopens {
+            let desc = self.table.push(Box::new(handle.dup()))?;
+            results.push((desc, name.clone()));
+        }
+        Ok(results)
     }
 
     async fn fadvise(
@@ -610,8 +616,6 @@ impl wasi_filesystem::WasiFilesystem for WasiCtx {
 
     async fn drop_descriptor(&mut self, fd: wasi_filesystem::Descriptor) -> anyhow::Result<()> {
         let table = self.table_mut();
-        // TODO: `WasiCtx` no longer keeps track of which directories are preopens, so we currently have no way
-        // of preventing them from being closed.  Is that a problem?
         if !(table.delete::<Box<dyn WasiFile>>(fd).is_ok()
             || table.delete::<Box<dyn WasiDir>>(fd).is_ok())
         {

host/tests/runtime.rs

@@ -360,7 +360,11 @@ async fn run_clock_time_get(store: Store<WasiCtx>, wasi: WasiCommand) -> Result<
 }
 
 async fn run_close_preopen(store: Store<WasiCtx>, wasi: WasiCommand) -> Result<()> {
-    expect_fail(run_with_temp_dir(store, wasi).await)
+    run_with_temp_dir(store, wasi).await
+}
+
+async fn run_overwrite_preopen(store: Store<WasiCtx>, wasi: WasiCommand) -> Result<()> {
+    run_with_temp_dir(store, wasi).await
 }
 
 async fn run_dangling_fd(store: Store<WasiCtx>, wasi: WasiCommand) -> Result<()> {

test-programs/wasi-tests/src/bin/close_preopen.rs

@@ -7,17 +7,7 @@ unsafe fn test_close_preopen(dir_fd: wasi::Fd) {
     assert!(dir_fd > pre_fd, "dir_fd number");
 
     // Try to close a preopened directory handle.
-    assert_errno!(
-        wasi::fd_close(pre_fd).expect_err("closing a preopened file descriptor"),
-        wasi::ERRNO_NOTSUP
-    );
-
-    // Try to renumber over a preopened directory handle.
-    assert_errno!(
-        wasi::fd_renumber(dir_fd, pre_fd)
-            .expect_err("renumbering over a preopened file descriptor"),
-        wasi::ERRNO_NOTSUP
-    );
+    wasi::fd_close(pre_fd).expect("closing a preopened file descriptor");
 
     // Ensure that dir_fd is still open.
     let dir_fdstat = wasi::fd_fdstat_get(dir_fd).expect("failed fd_fdstat_get");
@@ -27,19 +17,10 @@ unsafe fn test_close_preopen(dir_fd: wasi::Fd) {
         "expected the scratch directory to be a directory",
     );
 
-    // Try to renumber a preopened directory handle.
+    // Ensure that pre_fd is closed.
     assert_errno!(
-        wasi::fd_renumber(pre_fd, dir_fd)
-            .expect_err("renumbering over a preopened file descriptor"),
-        wasi::ERRNO_NOTSUP
-    );
-
-    // Ensure that dir_fd is still open.
-    let dir_fdstat = wasi::fd_fdstat_get(dir_fd).expect("failed fd_fdstat_get");
-    assert_eq!(
-        dir_fdstat.fs_filetype,
-        wasi::FILETYPE_DIRECTORY,
-        "expected the scratch directory to be a directory",
+        wasi::fd_fdstat_get(pre_fd).expect_err("failed fd_fdstat_get"),
+        wasi::ERRNO_BADF
     );
 }
 

test-programs/wasi-tests/src/bin/overwrite_preopen.rs

@@ -0,0 +1,49 @@
+use std::{env, process};
+use wasi_tests::{assert_errno, open_scratch_directory};
+
+unsafe fn test_overwrite_preopen(dir_fd: wasi::Fd) {
+    let pre_fd: wasi::Fd = (libc::STDERR_FILENO + 1) as wasi::Fd;
+
+    assert!(dir_fd > pre_fd, "dir_fd number");
+
+    let old_dir_filestat = wasi::fd_filestat_get(dir_fd).expect("failed fd_filestat_get");
+
+    // Try to renumber over a preopened directory handle.
+    wasi::fd_renumber(dir_fd, pre_fd).expect("renumbering over a preopened file descriptor");
+
+    // Ensure that pre_fd is still open.
+    let new_dir_filestat = wasi::fd_filestat_get(pre_fd).expect("failed fd_filestat_get");
+
+    // Ensure that we renumbered.
+    assert_eq!(old_dir_filestat.dev, new_dir_filestat.dev);
+    assert_eq!(old_dir_filestat.ino, new_dir_filestat.ino);
+
+    // Ensure that dir_fd is closed.
+    assert_errno!(
+        wasi::fd_fdstat_get(dir_fd).expect_err("failed fd_fdstat_get"),
+        wasi::ERRNO_BADF
+    );
+}
+
+fn main() {
+    let mut args = env::args();
+    let prog = args.next().unwrap();
+    let arg = if let Some(arg) = args.next() {
+        arg
+    } else {
+        eprintln!("usage: {} <scratch directory>", prog);
+        process::exit(1);
+    };
+
+    // Open scratch directory
+    let dir_fd = match open_scratch_directory(&arg) {
+        Ok(dir_fd) => dir_fd,
+        Err(err) => {
+            eprintln!("{}", err);
+            process::exit(1)
+        }
+    };
+
+    // Run the tests.
+    unsafe { test_overwrite_preopen(dir_fd) }
+}

wasi-common/cap-std-sync/src/dir.rs

@@ -2,17 +2,22 @@ use crate::file::{filetype_from, get_fd_flags, File};
 use cap_fs_ext::{DirEntryExt, DirExt, MetadataExt, SystemTimeSpec};
 use std::any::Any;
 use std::path::{Path, PathBuf};
+use std::sync::Arc;
 use wasi_common::{
     dir::{ReaddirCursor, ReaddirEntity, WasiDir},
     file::{FdFlags, Filestat, OFlags, WasiFile},
     Error, ErrorExt,
 };
 
-pub struct Dir(cap_std::fs::Dir);
+/// A directory handle.
+///
+/// We hold an `Arc` so that preopens can be regular handles which can
+/// be closed, without closing the underlying file descriptor.
+pub struct Dir(Arc<cap_std::fs::Dir>);
 
 impl Dir {
     pub fn from_cap_std(dir: cap_std::fs::Dir) -> Self {
-        Dir(dir)
+        Dir(Arc::new(dir))
     }
 
     pub fn open_file_(
@@ -159,7 +164,7 @@ impl WasiDir for Dir {
     }
 
     async fn get_fdflags(&self) -> Result<FdFlags, Error> {
-        let fdflags = get_fd_flags(&self.0)?;
+        let fdflags = get_fd_flags(&*self.0)?;
         Ok(fdflags)
     }
 
@@ -334,6 +339,10 @@ impl WasiDir for Dir {
         }
         Ok(())
     }
+
+    fn dup(&self) -> Box<dyn WasiDir> {
+        Box::new(Dir(Arc::clone(&self.0)))
+    }
 }
 
 fn convert_systimespec(t: Option<wasi_common::SystemTimeSpec>) -> Option<SystemTimeSpec> {

wasi-common/src/ctx.rs

@@ -14,7 +14,7 @@ pub struct WasiCtx {
     pub sched: Box<dyn WasiSched>,
     pub table: Table,
     pub env: Vec<(String, String)>,
-    pub preopens: Vec<(u32, String)>,
+    pub preopens: Vec<(Box<dyn WasiDir>, String)>,
 }
 
 impl WasiCtx {
@@ -91,8 +91,7 @@ impl WasiCtx {
     }
 
     pub fn push_preopened_dir(&mut self, dir: Box<dyn WasiDir>, path: &str) -> anyhow::Result<()> {
-        let fd = self.push_dir(dir)?;
-        self.preopens.push((fd, path.to_owned()));
+        self.preopens.push((dir, path.to_owned()));
         Ok(())
     }
 }

wasi-common/src/dir.rs

@@ -99,6 +99,8 @@ pub trait WasiDir: Send + Sync {
     ) -> Result<(), Error> {
         Err(Error::not_supported())
     }
+
+    fn dup(&self) -> Box<dyn WasiDir>;
 }
 
 pub trait TableDirExt {