diff --git a/crates/test-programs/src/bin/p1_renumber.rs b/crates/test-programs/src/bin/p1_renumber.rs index 6782fc709b63..855ecee7b9c6 100644 --- a/crates/test-programs/src/bin/p1_renumber.rs +++ b/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) } } diff --git a/crates/wasi/src/p0.rs b/crates/wasi/src/p0.rs index 5d610f0ae557..f9d3f904d6ef 100644 --- a/crates/wasi/src/p0.rs +++ b/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 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(()) } diff --git a/crates/wasi/src/p1.rs b/crates/wasi/src/p1.rs index 66ecd168e300..2b52cc4a08a2 100644 --- a/crates/wasi/src/p1.rs +++ b/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(()) } diff --git a/tests/wasi.rs b/tests/wasi.rs index 148d3fde9eea..64b94d2d1061 100644 --- a/tests/wasi.rs +++ b/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",