Skip to content

Commit 881c117

Browse files
authored
Unrolled build for #146691
Rollup merge of #146691 - alexcrichton:wasip1-remove-dir-all-buffer, r=juntyr std: Fix WASI implementation of `remove_dir_all` This commit is a change to the WASI-specific implementation of the `std::fs::remove_dir_all` function. Specifically it changes how directory entries are read of a directory-being-deleted to specifically buffer them all into a `Vec` before actually proceeding to delete anything. This is necessary to fix an interaction with how the WASIp1 `fd_readdir` API works to have everything work out in the face of mutations while reading a directory. The basic problem is that `fd_readdir`, the WASIp1 API for reading directories, is not a stateful read of a directory but instead a "seekable" read of a directory. Its `cookie` argument enables seeking anywhere within the directory at any time to read further entries. Native host implementations do not have this ability, however, which means that this seeking property must be achieved by re-reading the directory. The problem with this is that WASIp1 has under-specified semantics around what should happen if a directory is mutated between two calls to `fd_readdir`. In essence there's not really any possible implementation in hosts except to read the entire directory and support seeking through the already-read list. This implementation is not possible in the WASIp1-to-WASIp2 adapter that is primarily used to create components for the `wasm32-wasip2` target where it has constrained memory requirements and can't buffer up arbitrarily sized directories. There's some more detailed discussion at bytecodealliance/wasmtime#11701 (comment) as well. The WASIp1 API definitions are effectively "dead" now at the standards level meaning that `fd_readdir` won't be changing nor will a replacement be coming. For the `wasm32-wasip2` target this will get fixed once filesystem APIs are updated to use WASIp2 directly instead of WASIp1, making this buffering unnecessary. In essence while this is a hack it's sort of the least invasive thing that works everywhere for now. I don't think this is viable to fix in hosts so guests compiled to wasm are going to have to work around it by not relying on any guarantees about what happens to a directory if it's mutated between reads.
2 parents e10aa88 + f900758 commit 881c117

File tree

1 file changed

+8
-1
lines changed

1 file changed

+8
-1
lines changed

library/std/src/sys/fs/wasi.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,14 @@ fn remove_dir_all_recursive(parent: &WasiFd, path: &Path) -> io::Result<()> {
848848

849849
// Iterate over all the entries in this directory, and travel recursively if
850850
// necessary
851-
for entry in ReadDir::new(fd, dummy_root) {
851+
//
852+
// Note that all directory entries for this directory are read first before
853+
// any removal is done. This works around the fact that the WASIp1 API for
854+
// reading directories is not well-designed for handling mutations between
855+
// invocations of reading a directory. By reading all the entries at once
856+
// this ensures that, at least without concurrent modifications, it should
857+
// be possible to delete everything.
858+
for entry in ReadDir::new(fd, dummy_root).collect::<Vec<_>>() {
852859
let entry = entry?;
853860
let path = crate::str::from_utf8(&entry.name).map_err(|_| {
854861
io::const_error!(io::ErrorKind::Uncategorized, "invalid utf-8 file name found")

0 commit comments

Comments
 (0)