Fix leak in fd_renumber

crates/test-programs/src/bin/p1_renumber.rs

@@ -113,6 +113,34 @@ unsafe fn test_renumber(dir_fd: wasip1::Fd) {
     );
 }
 
+unsafe fn test_renumber_loop(dir_fd: wasip1::Fd) {
+    let mut cur = wasip1::path_open(
+        dir_fd,
+        0,
+        "file1",
+        wasip1::OFLAGS_CREAT,
+        wasip1::RIGHTS_FD_READ | wasip1::RIGHTS_FD_WRITE,
+        0,
+        0,
+    )
+    .expect("opening a file");
+
+    // Historically `fd_renumber` didn't properly close the "to" file descriptor
+    // which meant that the resources would be leaked. The test harness here
+    // allows 1000 entries in the resource table, and then most unix systems
+    // allow ~1000 fds open by default. Round that up and do this 2000 times to
+    // try to exercise any bugs should they show up. Historically this would
+    // exhaust host file descriptors before `fd_renumber` was fixed.
+    for _ in 0..2000 {
+        let next = wasip1::path_open(dir_fd, 0, "file1", 0, wasip1::RIGHTS_FD_READ, 0, 0)
+            .expect("opening a file");
+        wasip1::fd_renumber(cur, next).unwrap();
+        cur = next;
+    }
+
+    wasip1::fd_close(cur).unwrap();
+}
+
 fn main() {
     let mut args = env::args();
     let prog = args.next().unwrap();
@@ -134,4 +162,5 @@ fn main() {
 
     // Run the tests.
     unsafe { test_renumber(dir_fd) }
+    unsafe { test_renumber_loop(dir_fd) }
 }

crates/wasi/src/p0.rs

@@ -31,7 +31,7 @@ wiggle::from_witx!({
             fd_filestat_set_times, fd_read, fd_pread, fd_seek, fd_sync, fd_readdir, fd_write,
             fd_pwrite, poll_oneoff, path_create_directory, path_filestat_get,
             path_filestat_set_times, path_link, path_open, path_readlink, path_remove_directory,
-            path_rename, path_symlink, path_unlink_file
+            path_rename, path_symlink, path_unlink_file, fd_renumber
         }
     },
     errors: { errno => trappable Error },
@@ -50,7 +50,7 @@ mod sync {
                 fd_filestat_set_times, fd_read, fd_pread, fd_seek, fd_sync, fd_readdir, fd_write,
                 fd_pwrite, poll_oneoff, path_create_directory, path_filestat_get,
                 path_filestat_set_times, path_link, path_open, path_readlink, path_remove_directory,
-                path_rename, path_symlink, path_unlink_file
+                path_rename, path_symlink, path_unlink_file, fd_renumber
             }
         },
         errors: { errno => trappable Error },
@@ -300,13 +300,13 @@ impl<T: Snapshot1 + Send> wasi_unstable::WasiUnstable for T {
         Ok(())
     }
 
-    fn fd_renumber(
+    async fn fd_renumber(
         &mut self,
         memory: &mut GuestMemory<'_>,
         from: types::Fd,
         to: types::Fd,
     ) -> Result<(), Error> {
-        Snapshot1::fd_renumber(self, memory, from.into(), to.into())?;
+        Snapshot1::fd_renumber(self, memory, from.into(), to.into()).await?;
         Ok(())
     }
 

crates/wasi/src/p1.rs

@@ -863,7 +863,7 @@ wiggle::from_witx!({
             fd_filestat_set_times, fd_read, fd_pread, fd_seek, fd_sync, fd_readdir, fd_write,
             fd_pwrite, poll_oneoff, path_create_directory, path_filestat_get,
             path_filestat_set_times, path_link, path_open, path_readlink, path_remove_directory,
-            path_rename, path_symlink, path_unlink_file
+            path_rename, path_symlink, path_unlink_file, fd_renumber
         }
     },
     errors: { errno => trappable Error },
@@ -882,7 +882,7 @@ pub(crate) mod sync {
                 fd_filestat_set_times, fd_read, fd_pread, fd_seek, fd_sync, fd_readdir, fd_write,
                 fd_pwrite, poll_oneoff, path_create_directory, path_filestat_get,
                 path_filestat_set_times, path_link, path_open, path_readlink, path_remove_directory,
-                path_rename, path_symlink, path_unlink_file
+                path_rename, path_symlink, path_unlink_file, fd_renumber
             }
         },
         errors: { errno => trappable Error },
@@ -1854,28 +1854,33 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
     }
 
     /// Atomically replace a file descriptor by renumbering another file descriptor.
-    #[instrument(skip(self, _memory))]
-    fn fd_renumber(
+    #[instrument(skip(self, memory))]
+    async fn fd_renumber(
         &mut self,
-        _memory: &mut GuestMemory<'_>,
-        from: types::Fd,
-        to: types::Fd,
+        memory: &mut GuestMemory<'_>,
+        from_fd: types::Fd,
+        to_fd: types::Fd,
     ) -> Result<(), types::Error> {
-        let mut st = self.transact()?;
-        let from = from.into();
-        let to = to.into();
-        if !st.descriptors.used.contains_key(&to) {
-            return Err(types::Errno::Badf.into());
+        let from = from_fd.into();
+        let to = to_fd.into();
+        {
+            let st = self.transact()?;
+            if !st.descriptors.used.contains_key(&to) || !st.descriptors.used.contains_key(&from) {
+                return Err(types::Errno::Badf.into());
+            }
+            if from == to {
+                return Ok(());
+            }
         }
+        self.fd_close(memory, to_fd).await?;
+        let mut st = self.transact()?;
         let btree_map::Entry::Occupied(desc) = st.descriptors.used.entry(from) else {
             return Err(types::Errno::Badf.into());
         };
-        if from != to {
-            let desc = desc.remove();
-            st.descriptors.free.insert(from);
-            st.descriptors.free.remove(&to);
-            st.descriptors.used.insert(to, desc);
-        }
+        let desc = desc.remove();
+        st.descriptors.free.insert(from);
+        st.descriptors.free.remove(&to);
+        st.descriptors.used.insert(to, desc);
         Ok(())
     }
 

tests/wasi.rs

@@ -18,10 +18,6 @@ use wasmtime::error::Context;
 use wasmtime::{Result, format_err};
 
 const KNOWN_FAILURES: &[&str] = &[
-    #[cfg(windows)]
-    "renumber",
-    #[cfg(windows)]
-    "stdio",
     // Waiting for the upstream tests to update to use the 0.3.0 official WITs.
     "multi-clock-wait",
     "filesystem-unlink-errors",