diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index f9d6b5fbc86a4..7bf2e1bca62e1 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -2113,6 +2113,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, path: &CStr) -> io::Result<()> { // try opening as directory let fd = match openat_nofollow_dironly(parent_fd, &path) { @@ -2128,36 +2138,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<()> { diff --git a/tests/run-make/remove-dir-all-race/rmake.rs b/tests/run-make/remove-dir-all-race/rmake.rs new file mode 100644 index 0000000000000..787be39de8e76 --- /dev/null +++ b/tests/run-make/remove-dir-all-race/rmake.rs @@ -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(); + } + }) +}