Skip to content

Commit 6485909

Browse files
committed
fix: fs::remove_dir_all: treat ENOENT as success
fixes rust-lang#127576
1 parent 5367673 commit 6485909

File tree

2 files changed

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

2 files changed

+81
-24
lines changed

library/std/src/sys/pal/unix/fs.rs

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2082,6 +2082,16 @@ mod remove_dir_impl {
20822082
}
20832083
}
20842084

2085+
fn is_enoent(result: &io::Result<()>) -> bool {
2086+
if let Err(err) = result
2087+
&& matches!(err.raw_os_error(), Some(libc::ENOENT))
2088+
{
2089+
true
2090+
} else {
2091+
false
2092+
}
2093+
}
2094+
20852095
fn remove_dir_all_recursive(parent_fd: Option<RawFd>, path: &CStr) -> io::Result<()> {
20862096
// try opening as directory
20872097
let fd = match openat_nofollow_dironly(parent_fd, &path) {
@@ -2097,36 +2107,57 @@ mod remove_dir_impl {
20972107
None => Err(err),
20982108
};
20992109
}
2110+
Err(err) if matches!(err.raw_os_error(), Some(libc::ENOENT)) => {
2111+
// the file has already been removed by something else,
2112+
// do nothing.
2113+
return Ok(());
2114+
}
21002115
result => result?,
21012116
};
21022117

2103-
// open the directory passing ownership of the fd
2104-
let (dir, fd) = fdreaddir(fd)?;
2105-
for child in dir {
2106-
let child = child?;
2107-
let child_name = child.name_cstr();
2108-
match is_dir(&child) {
2109-
Some(true) => {
2110-
remove_dir_all_recursive(Some(fd), child_name)?;
2111-
}
2112-
Some(false) => {
2113-
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
2114-
}
2115-
None => {
2116-
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
2117-
// if the process has the appropriate privileges. This however can causing orphaned
2118-
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
2119-
// into it first instead of trying to unlink() it.
2120-
remove_dir_all_recursive(Some(fd), child_name)?;
2118+
let result: io::Result<()> = try {
2119+
// open the directory passing ownership of the fd
2120+
let (dir, fd) = fdreaddir(fd)?;
2121+
for child in dir {
2122+
let child = child?;
2123+
let child_name = child.name_cstr();
2124+
// we need an inner try block, because if one of these
2125+
// directories has already been deleted, then we need to
2126+
// continue the loop, not return ok.
2127+
let result: io::Result<()> = try {
2128+
match is_dir(&child) {
2129+
Some(true) => {
2130+
remove_dir_all_recursive(Some(fd), child_name)?;
2131+
}
2132+
Some(false) => {
2133+
cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?;
2134+
}
2135+
None => {
2136+
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
2137+
// if the process has the appropriate privileges. This however can causing orphaned
2138+
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
2139+
// into it first instead of trying to unlink() it.
2140+
remove_dir_all_recursive(Some(fd), child_name)?;
2141+
}
2142+
}
2143+
};
2144+
if result.is_err() && !is_enoent(&result) {
2145+
return result;
21212146
}
21222147
}
2123-
}
21242148

2125-
// unlink the directory after removing its contents
2126-
cvt(unsafe {
2127-
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
2128-
})?;
2129-
Ok(())
2149+
// unlink the directory after removing its contents
2150+
cvt(unsafe {
2151+
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
2152+
})?;
2153+
};
2154+
if is_enoent(&result) {
2155+
// the file has already been removed by something else,
2156+
// do nothing.
2157+
Ok(())
2158+
} else {
2159+
result
2160+
}
21302161
}
21312162

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

0 commit comments

Comments
 (0)