Rollup merge of #127623 - lolbinarycat:fix_remove_dir_all, r=Amanieu

fix: fs::remove_dir_all: treat internal ENOENT as success

fixes #127576

try-job: test-various
This commit is contained in:
Matthias Krüger
2024-08-23 06:26:51 +02:00
committed by GitHub
8 changed files with 153 additions and 36 deletions

View File

@@ -10,6 +10,7 @@ use crate::sync::Arc;
use crate::sys::time::SystemTime;
use crate::sys::unsupported;
pub use crate::sys_common::fs::exists;
use crate::sys_common::ignore_notfound;
/// A file descriptor.
#[derive(Clone, Copy)]
@@ -527,15 +528,23 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
for child in readdir(path)? {
let child = child?;
let child_type = child.file_type()?;
if child_type.is_dir() {
remove_dir_all(&child.path())?;
} else {
unlink(&child.path())?;
let result: io::Result<()> = try {
let child = child?;
let child_type = child.file_type()?;
if child_type.is_dir() {
remove_dir_all(&child.path())?;
} else {
unlink(&child.path())?;
}
};
// ignore internal NotFound errors
if let Err(err) = result
&& err.kind() != io::ErrorKind::NotFound
{
return result;
}
}
rmdir(path)
ignore_notfound(rmdir(path))
}
pub fn readlink(p: &Path) -> io::Result<PathBuf> {

View File

@@ -2002,6 +2002,7 @@ mod remove_dir_impl {
use crate::path::{Path, PathBuf};
use crate::sys::common::small_c_string::run_path_with_cstr;
use crate::sys::{cvt, cvt_r};
use crate::sys_common::ignore_notfound;
pub fn openat_nofollow_dironly(parent_fd: Option<RawFd>, p: &CStr) -> io::Result<OwnedFd> {
let fd = cvt_r(|| unsafe {
@@ -2055,6 +2056,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) {
@@ -2078,27 +2089,35 @@ mod remove_dir_impl {
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)?;
// 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 {
ignore_notfound(cvt(unsafe {
unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), path.as_ptr(), libc::AT_REMOVEDIR)
})?;
}))?;
Ok(())
}

View File

@@ -13,7 +13,7 @@ use crate::sys::common::small_c_string::run_path_with_cstr;
use crate::sys::time::SystemTime;
use crate::sys::unsupported;
pub use crate::sys_common::fs::exists;
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::sys_common::{ignore_notfound, AsInner, FromInner, IntoInner};
use crate::{fmt, iter, ptr};
pub struct File {
@@ -794,14 +794,22 @@ fn remove_dir_all_recursive(parent: &WasiFd, path: &Path) -> io::Result<()> {
io::const_io_error!(io::ErrorKind::Uncategorized, "invalid utf-8 file name found")
})?;
if entry.file_type()?.is_dir() {
remove_dir_all_recursive(&entry.inner.dir.fd, path.as_ref())?;
} else {
entry.inner.dir.fd.unlink_file(path)?;
let result: io::Result<()> = try {
if entry.file_type()?.is_dir() {
remove_dir_all_recursive(&entry.inner.dir.fd, path.as_ref())?;
} else {
entry.inner.dir.fd.unlink_file(path)?;
}
};
// ignore internal NotFound errors
if let Err(err) = &result
&& err.kind() != io::ErrorKind::NotFound
{
return result;
}
}
// Once all this directory's contents are deleted it should be safe to
// delete the directory tiself.
parent.remove_directory(osstr2str(path.as_ref())?)
ignore_notfound(parent.remove_directory(osstr2str(path.as_ref())?))
}

View File

@@ -14,7 +14,7 @@ use crate::sys::handle::Handle;
use crate::sys::path::maybe_verbatim;
use crate::sys::time::SystemTime;
use crate::sys::{c, cvt, Align8};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::sys_common::{ignore_notfound, AsInner, FromInner, IntoInner};
use crate::{fmt, ptr, slice, thread};
pub struct File {
@@ -1160,7 +1160,7 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> {
return Err(io::Error::from_raw_os_error(c::ERROR_DIRECTORY as _));
}
match remove_dir_all_iterative(&file, File::posix_delete) {
match ignore_notfound(remove_dir_all_iterative(&file, File::posix_delete)) {
Err(e) => {
if let Some(code) = e.raw_os_error() {
match code as u32 {