Skip to content

Commit

Permalink
fix: fs::remove_dir_all: treat ENOENT as success
Browse files Browse the repository at this point in the history
  • Loading branch information
lolbinarycat committed Aug 2, 2024
1 parent 5367673 commit 6485909
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 24 deletions.
79 changes: 55 additions & 24 deletions library/std/src/sys/pal/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2082,6 +2082,16 @@ mod remove_dir_impl {
}
}

fn is_enoent(result: &io::Result<()>) -> bool {
if let Err(err) = result
&& matches!(err.raw_os_error(), Some(libc::ENOENT))
{
true
} else {
false
}
}

fn remove_dir_all_recursive(parent_fd: Option<RawFd>, path: &CStr) -> io::Result<()> {
// try opening as directory
let fd = match openat_nofollow_dironly(parent_fd, &path) {
Expand All @@ -2097,36 +2107,57 @@ mod remove_dir_impl {
None => Err(err),
};
}
Err(err) if matches!(err.raw_os_error(), Some(libc::ENOENT)) => {
// the file has already been removed by something else,
// do nothing.
return Ok(());
}
result => result?,
};

// open the directory passing ownership of the fd
let (dir, fd) = fdreaddir(fd)?;
for child in dir {
let child = child?;
let child_name = child.name_cstr();
match is_dir(&child) {
Some(true) => {
remove_dir_all_recursive(Some(fd), child_name)?;
}
Some(false) => {
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
}
None => {
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
// if the process has the appropriate privileges. This however can causing orphaned
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
// into it first instead of trying to unlink() it.
remove_dir_all_recursive(Some(fd), child_name)?;
let result: io::Result<()> = try {
// open the directory passing ownership of the fd
let (dir, fd) = fdreaddir(fd)?;
for child in dir {
let child = child?;
let child_name = child.name_cstr();
// we need an inner try block, because if one of these
// directories has already been deleted, then we need to
// continue the loop, not return ok.
let result: io::Result<()> = try {
match is_dir(&child) {
Some(true) => {
remove_dir_all_recursive(Some(fd), child_name)?;
}
Some(false) => {
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
}
None => {
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
// if the process has the appropriate privileges. This however can causing orphaned
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
// into it first instead of trying to unlink() it.
remove_dir_all_recursive(Some(fd), child_name)?;
}
}
};
if result.is_err() && !is_enoent(&result) {
return result;
}
}
}

// unlink the directory after removing its contents
cvt(unsafe {
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
})?;
Ok(())
// unlink the directory after removing its contents
cvt(unsafe {
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
})?;
};
if is_enoent(&result) {
// the file has already been removed by something else,
// do nothing.
Ok(())
} else {
result
}
}

fn remove_dir_all_modern(p: &Path) -> io::Result<()> {
Expand Down
26 changes: 26 additions & 0 deletions tests/run-make/remove-dir-all-race/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use run_make_support::{
fs_wrapper::{create_dir, remove_dir_all, write},
run_in_tmpdir,
};
use std::thread;
use std::time::Duration;

fn main() {
run_in_tmpdir(|| {
for i in 0..10 {
create_dir("outer");
create_dir("outer/inner");
write("outer/inner.txt", b"sometext");

let t1 = thread::spawn(move || {
thread::sleep(Duration::from_millis(i * 10));
remove_dir_all("outer")
});

let t2 = thread::spawn(move || remove_dir_all("outer"));

t1.join().unwrap();
t2.join().unwrap();
}
})
}

0 comments on commit 6485909

Please sign in to comment.