Skip to content

Commit a696f34

Browse files
committed
fix: fs::remove_dir_all: treat ENOENT as success
fixes #127576
1 parent c92a8e4 commit a696f34

File tree

2 files changed

+84
-27
lines changed
  • library/std/src/sys/pal/unix
  • tests/run-make/remove-dir-all-race

2 files changed

+84
-27
lines changed

Diff for: library/std/src/sys/pal/unix/fs.rs

+59-27
Original file line numberDiff line numberDiff line change
@@ -2113,6 +2113,16 @@ mod remove_dir_impl {
21132113
}
21142114
}
21152115

2116+
fn is_enoent(result: &io::Result<()>) -> bool {
2117+
if let Err(err) = result &&
2118+
matches!(err.raw_os_error(), Some(libc::ENOENT))
2119+
{
2120+
true
2121+
} else {
2122+
false
2123+
}
2124+
}
2125+
21162126
fn remove_dir_all_recursive(parent_fd: Option<RawFd>, path: &CStr) -> io::Result<()> {
21172127
// try opening as directory
21182128
let fd = match openat_nofollow_dironly(parent_fd, &path) {
@@ -2128,36 +2138,58 @@ mod remove_dir_impl {
21282138
None => Err(err),
21292139
};
21302140
}
2141+
Err(err) if matches!(err.raw_os_error(), Some(libc::ENOENT)) => {
2142+
// the file has already been removed by something else,
2143+
// do nothing.
2144+
return Ok(());
2145+
}
21312146
result => result?,
21322147
};
21332148

2134-
// open the directory passing ownership of the fd
2135-
let (dir, fd) = fdreaddir(fd)?;
2136-
for child in dir {
2137-
let child = child?;
2138-
let child_name = child.name_cstr();
2139-
match is_dir(&child) {
2140-
Some(true) => {
2141-
remove_dir_all_recursive(Some(fd), child_name)?;
2142-
}
2143-
Some(false) => {
2144-
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
2145-
}
2146-
None => {
2147-
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
2148-
// if the process has the appropriate privileges. This however can causing orphaned
2149-
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
2150-
// into it first instead of trying to unlink() it.
2151-
remove_dir_all_recursive(Some(fd), child_name)?;
2152-
}
2153-
}
2154-
}
2155-
2156-
// unlink the directory after removing its contents
2157-
cvt(unsafe {
2158-
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
2159-
})?;
2160-
Ok(())
2149+
let result: io::Result<()> = try {
2150+
// open the directory passing ownership of the fd
2151+
let (dir, fd) = fdreaddir(fd)?;
2152+
for child in dir {
2153+
let child = child?;
2154+
let child_name = child.name_cstr();
2155+
// we need an inner try block, because if one of these
2156+
// directories has already been deleted, then we need to
2157+
// continue the loop, not return ok.
2158+
let result: io::Result<()> = try {
2159+
match is_dir(&child) {
2160+
Some(true) => {
2161+
remove_dir_all_recursive(Some(fd), child_name)?;
2162+
}
2163+
Some(false) => {
2164+
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
2165+
}
2166+
None => {
2167+
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
2168+
// if the process has the appropriate privileges. This however can causing orphaned
2169+
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
2170+
// into it first instead of trying to unlink() it.
2171+
remove_dir_all_recursive(Some(fd), child_name)?;
2172+
}
2173+
}
2174+
};
2175+
if result.is_err() && !is_enoent(&result) {
2176+
return result;
2177+
}
2178+
}
2179+
2180+
// unlink the directory after removing its contents
2181+
cvt(unsafe {
2182+
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
2183+
})?;
2184+
};
2185+
if is_enoent(&result)
2186+
{
2187+
// the file has already been removed by something else,
2188+
// do nothing.
2189+
Ok(())
2190+
} else {
2191+
result
2192+
}
21612193
}
21622194

21632195
fn remove_dir_all_modern(p: &Path) -> io::Result<()> {

Diff for: tests/run-make/remove-dir-all-race/rmake.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use run_make_support::{run_in_tmpdir, fs_wrapper::{create_dir, write, remove_dir_all}};
2+
use std::thread;
3+
use std::time::Duration;
4+
5+
fn main() {
6+
run_in_tmpdir(|| {
7+
for i in 0..10 {
8+
create_dir("outer");
9+
create_dir("outer/inner");
10+
write("outer/inner.txt", b"sometext");
11+
12+
let t1 = thread::spawn(move || {
13+
thread::sleep(Duration::from_millis(i * 10));
14+
remove_dir_all("outer")
15+
});
16+
17+
let t2 = thread::spawn(move || {
18+
remove_dir_all("outer")
19+
});
20+
21+
t1.join().unwrap();
22+
t2.join().unwrap();
23+
}
24+
})
25+
}

0 commit comments

Comments
 (0)