Skip to content

Commit

Permalink
Rollup merge of #129800 - ChrisDenton:remove-dir-all2, r=Amanieu
Browse files Browse the repository at this point in the history 
Move the Windows remove_dir_all impl into a module and make it more race resistant

This attempts to make the Windows implementation of `remove_dir_all` easier to understand and work with by separating out different concerns into their own functions. The code is mostly the same as before just moved around. There are some changes to make it more robust against races (e.g. two calls to `remove_dir_all` running concurrently). The module level comment explains the issue.

try-job: x86_64-msvc
try-job: i686-msvc
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Sep 3, 2024
2 parents 44187d6 + bb9d5c4 commit 72cc383
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 169 deletions.
5 changes: 4 additions & 1 deletion library/std/src/sys/pal/windows/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ pub struct WinError {
pub code: u32,
}
impl WinError {
const fn new(code: u32) -> Self {
pub const fn new(code: u32) -> Self {
Self { code }
}
}
Expand All @@ -272,8 +272,11 @@ impl WinError {
// tidy-alphabetical-start
pub const ACCESS_DENIED: Self = Self::new(c::ERROR_ACCESS_DENIED);
pub const ALREADY_EXISTS: Self = Self::new(c::ERROR_ALREADY_EXISTS);
pub const BAD_NET_NAME: Self = Self::new(c::ERROR_BAD_NET_NAME);
pub const BAD_NETPATH: Self = Self::new(c::ERROR_BAD_NETPATH);
pub const CANT_ACCESS_FILE: Self = Self::new(c::ERROR_CANT_ACCESS_FILE);
pub const DELETE_PENDING: Self = Self::new(c::ERROR_DELETE_PENDING);
pub const DIR_NOT_EMPTY: Self = Self::new(c::ERROR_DIR_NOT_EMPTY);
pub const DIRECTORY: Self = Self::new(c::ERROR_DIRECTORY);
pub const FILE_NOT_FOUND: Self = Self::new(c::ERROR_FILE_NOT_FOUND);
pub const INSUFFICIENT_BUFFER: Self = Self::new(c::ERROR_INSUFFICIENT_BUFFER);
Expand Down
5 changes: 5 additions & 0 deletions library/std/src/sys/pal/windows/c/bindings.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Windows.Wdk.Storage.FileSystem.FILE_WRITE_THROUGH
Windows.Wdk.Storage.FileSystem.NtCreateFile
Windows.Wdk.Storage.FileSystem.NTCREATEFILE_CREATE_DISPOSITION
Windows.Wdk.Storage.FileSystem.NTCREATEFILE_CREATE_OPTIONS
Windows.Wdk.Storage.FileSystem.NtOpenFile
Windows.Wdk.Storage.FileSystem.NtReadFile
Windows.Wdk.Storage.FileSystem.NtWriteFile
Windows.Wdk.Storage.FileSystem.SYMLINK_FLAG_RELATIVE
Expand Down Expand Up @@ -1931,10 +1932,14 @@ Windows.Win32.Foundation.RtlNtStatusToDosError
Windows.Win32.Foundation.SetHandleInformation
Windows.Win32.Foundation.SetLastError
Windows.Win32.Foundation.STATUS_DELETE_PENDING
Windows.Win32.Foundation.STATUS_DIRECTORY_NOT_EMPTY
Windows.Win32.Foundation.STATUS_END_OF_FILE
Windows.Win32.Foundation.STATUS_FILE_DELETED
Windows.Win32.Foundation.STATUS_INVALID_HANDLE
Windows.Win32.Foundation.STATUS_INVALID_PARAMETER
Windows.Win32.Foundation.STATUS_NOT_IMPLEMENTED
Windows.Win32.Foundation.STATUS_PENDING
Windows.Win32.Foundation.STATUS_SHARING_VIOLATION
Windows.Win32.Foundation.STATUS_SUCCESS
Windows.Win32.Foundation.TRUE
Windows.Win32.Foundation.UNICODE_STRING
Expand Down
5 changes: 5 additions & 0 deletions library/std/src/sys/pal/windows/c/windows_sys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ windows_targets::link!("kernel32.dll" "system" fn WideCharToMultiByte(codepage :
windows_targets::link!("kernel32.dll" "system" fn WriteConsoleW(hconsoleoutput : HANDLE, lpbuffer : PCWSTR, nnumberofcharstowrite : u32, lpnumberofcharswritten : *mut u32, lpreserved : *const core::ffi::c_void) -> BOOL);
windows_targets::link!("kernel32.dll" "system" fn WriteFileEx(hfile : HANDLE, lpbuffer : *const u8, nnumberofbytestowrite : u32, lpoverlapped : *mut OVERLAPPED, lpcompletionroutine : LPOVERLAPPED_COMPLETION_ROUTINE) -> BOOL);
windows_targets::link!("ntdll.dll" "system" fn NtCreateFile(filehandle : *mut HANDLE, desiredaccess : FILE_ACCESS_RIGHTS, objectattributes : *const OBJECT_ATTRIBUTES, iostatusblock : *mut IO_STATUS_BLOCK, allocationsize : *const i64, fileattributes : FILE_FLAGS_AND_ATTRIBUTES, shareaccess : FILE_SHARE_MODE, createdisposition : NTCREATEFILE_CREATE_DISPOSITION, createoptions : NTCREATEFILE_CREATE_OPTIONS, eabuffer : *const core::ffi::c_void, ealength : u32) -> NTSTATUS);
windows_targets::link!("ntdll.dll" "system" fn NtOpenFile(filehandle : *mut HANDLE, desiredaccess : u32, objectattributes : *const OBJECT_ATTRIBUTES, iostatusblock : *mut IO_STATUS_BLOCK, shareaccess : u32, openoptions : u32) -> NTSTATUS);
windows_targets::link!("ntdll.dll" "system" fn NtReadFile(filehandle : HANDLE, event : HANDLE, apcroutine : PIO_APC_ROUTINE, apccontext : *const core::ffi::c_void, iostatusblock : *mut IO_STATUS_BLOCK, buffer : *mut core::ffi::c_void, length : u32, byteoffset : *const i64, key : *const u32) -> NTSTATUS);
windows_targets::link!("ntdll.dll" "system" fn NtWriteFile(filehandle : HANDLE, event : HANDLE, apcroutine : PIO_APC_ROUTINE, apccontext : *const core::ffi::c_void, iostatusblock : *mut IO_STATUS_BLOCK, buffer : *const core::ffi::c_void, length : u32, byteoffset : *const i64, key : *const u32) -> NTSTATUS);
windows_targets::link!("ntdll.dll" "system" fn RtlNtStatusToDosError(status : NTSTATUS) -> u32);
Expand Down Expand Up @@ -2982,10 +2983,14 @@ pub struct STARTUPINFOW {
}
pub type STARTUPINFOW_FLAGS = u32;
pub const STATUS_DELETE_PENDING: NTSTATUS = 0xC0000056_u32 as _;
pub const STATUS_DIRECTORY_NOT_EMPTY: NTSTATUS = 0xC0000101_u32 as _;
pub const STATUS_END_OF_FILE: NTSTATUS = 0xC0000011_u32 as _;
pub const STATUS_FILE_DELETED: NTSTATUS = 0xC0000123_u32 as _;
pub const STATUS_INVALID_HANDLE: NTSTATUS = 0xC0000008_u32 as _;
pub const STATUS_INVALID_PARAMETER: NTSTATUS = 0xC000000D_u32 as _;
pub const STATUS_NOT_IMPLEMENTED: NTSTATUS = 0xC0000002_u32 as _;
pub const STATUS_PENDING: NTSTATUS = 0x103_u32 as _;
pub const STATUS_SHARING_VIOLATION: NTSTATUS = 0xC0000043_u32 as _;
pub const STATUS_SUCCESS: NTSTATUS = 0x0_u32 as _;
pub const STD_ERROR_HANDLE: STD_HANDLE = 4294967284u32;
pub type STD_HANDLE = u32;
Expand Down
211 changes: 43 additions & 168 deletions library/std/src/sys/pal/windows/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ 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::{ignore_notfound, AsInner, FromInner, IntoInner};
use crate::{fmt, ptr, slice, thread};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::{fmt, ptr, slice};

mod remove_dir_all;
use remove_dir_all::remove_dir_all_iterative;

pub struct File {
handle: Handle,
Expand Down Expand Up @@ -646,6 +649,22 @@ impl File {
Ok(info)
}
}

/// Deletes the file, consuming the file handle to ensure the delete occurs
/// as immediately as possible.
/// This attempts to use `posix_delete` but falls back to `win32_delete`
/// if that is not supported by the filesystem.
#[allow(unused)]
fn delete(self) -> Result<(), WinError> {
// If POSIX delete is not supported for this filesystem then fallback to win32 delete.
match self.posix_delete() {
Err(WinError::INVALID_PARAMETER)
| Err(WinError::NOT_SUPPORTED)
| Err(WinError::INVALID_FUNCTION) => self.win32_delete(),
result => result,
}
}

/// Delete using POSIX semantics.
///
/// Files will be deleted as soon as the handle is closed. This is supported
Expand All @@ -654,21 +673,23 @@ impl File {
///
/// If the operation is not supported for this filesystem or OS version
/// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`.
fn posix_delete(&self) -> io::Result<()> {
#[allow(unused)]
fn posix_delete(&self) -> Result<(), WinError> {
let info = c::FILE_DISPOSITION_INFO_EX {
Flags: c::FILE_DISPOSITION_FLAG_DELETE
| c::FILE_DISPOSITION_FLAG_POSIX_SEMANTICS
| c::FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE,
};
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info)
}

/// Delete a file using win32 semantics. The file won't actually be deleted
/// until all file handles are closed. However, marking a file for deletion
/// will prevent anyone from opening a new handle to the file.
fn win32_delete(&self) -> io::Result<()> {
#[allow(unused)]
fn win32_delete(&self) -> Result<(), WinError> {
let info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ };
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info)
}

/// Fill the given buffer with as many directory entries as will fit.
Expand All @@ -684,21 +705,23 @@ impl File {
/// A symlink directory is simply an empty directory with some "reparse" metadata attached.
/// So if you open a link (not its target) and iterate the directory,
/// you will always iterate an empty directory regardless of the target.
fn fill_dir_buff(&self, buffer: &mut DirBuff, restart: bool) -> io::Result<bool> {
#[allow(unused)]
fn fill_dir_buff(&self, buffer: &mut DirBuff, restart: bool) -> Result<bool, WinError> {
let class =
if restart { c::FileIdBothDirectoryRestartInfo } else { c::FileIdBothDirectoryInfo };

unsafe {
let result = cvt(c::GetFileInformationByHandleEx(
self.handle.as_raw_handle(),
let result = c::GetFileInformationByHandleEx(
self.as_raw_handle(),
class,
buffer.as_mut_ptr().cast(),
buffer.capacity() as _,
));
match result {
Ok(_) => Ok(true),
Err(e) if e.raw_os_error() == Some(c::ERROR_NO_MORE_FILES as _) => Ok(false),
Err(e) => Err(e),
);
if result == 0 {
let err = api::get_last_error();
if err.code == c::ERROR_NO_MORE_FILES { Ok(false) } else { Err(err) }
} else {
Ok(true)
}
}
}
Expand Down Expand Up @@ -804,62 +827,6 @@ unsafe fn from_maybe_unaligned<'a>(p: *const u16, len: usize) -> Cow<'a, [u16]>
}
}

/// Open a link relative to the parent directory, ensure no symlinks are followed.
fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result<File> {
// This is implemented using the lower level `NtCreateFile` function as
// unfortunately opening a file relative to a parent is not supported by
// win32 functions. It is however a fundamental feature of the NT kernel.
//
// See https://docs.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntcreatefile
unsafe {
let mut handle = ptr::null_mut();
let mut io_status = c::IO_STATUS_BLOCK::PENDING;
let mut name_str = c::UNICODE_STRING::from_ref(name);
use crate::sync::atomic::{AtomicU32, Ordering};
// The `OBJ_DONT_REPARSE` attribute ensures that we haven't been
// tricked into following a symlink. However, it may not be available in
// earlier versions of Windows.
static ATTRIBUTES: AtomicU32 = AtomicU32::new(c::OBJ_DONT_REPARSE);
let object = c::OBJECT_ATTRIBUTES {
ObjectName: &mut name_str,
RootDirectory: parent.as_raw_handle(),
Attributes: ATTRIBUTES.load(Ordering::Relaxed),
..c::OBJECT_ATTRIBUTES::default()
};
let status = c::NtCreateFile(
&mut handle,
access,
&object,
&mut io_status,
crate::ptr::null_mut(),
0,
c::FILE_SHARE_DELETE | c::FILE_SHARE_READ | c::FILE_SHARE_WRITE,
c::FILE_OPEN,
// If `name` is a symlink then open the link rather than the target.
c::FILE_OPEN_REPARSE_POINT,
crate::ptr::null_mut(),
0,
);
// Convert an NTSTATUS to the more familiar Win32 error codes (aka "DosError")
if c::nt_success(status) {
Ok(File::from_raw_handle(handle))
} else if status == c::STATUS_DELETE_PENDING {
// We make a special exception for `STATUS_DELETE_PENDING` because
// otherwise this will be mapped to `ERROR_ACCESS_DENIED` which is
// very unhelpful.
Err(io::Error::from_raw_os_error(c::ERROR_DELETE_PENDING as i32))
} else if status == c::STATUS_INVALID_PARAMETER
&& ATTRIBUTES.load(Ordering::Relaxed) == c::OBJ_DONT_REPARSE
{
// Try without `OBJ_DONT_REPARSE`. See above.
ATTRIBUTES.store(0, Ordering::Relaxed);
open_link_no_reparse(parent, name, access)
} else {
Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as _))
}
}
}

impl AsInner<Handle> for File {
#[inline]
fn as_inner(&self) -> &Handle {
Expand Down Expand Up @@ -1142,114 +1109,22 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
Ok(())
}

/// Open a file or directory without following symlinks.
fn open_link(path: &Path, access_mode: u32) -> io::Result<File> {
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
// Open a file or directory without following symlinks.
let mut opts = OpenOptions::new();
opts.access_mode(access_mode);
opts.access_mode(c::FILE_LIST_DIRECTORY);
// `FILE_FLAG_BACKUP_SEMANTICS` allows opening directories.
// `FILE_FLAG_OPEN_REPARSE_POINT` opens a link instead of its target.
opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | c::FILE_FLAG_OPEN_REPARSE_POINT);
File::open(path, &opts)
}

pub fn remove_dir_all(path: &Path) -> io::Result<()> {
let file = open_link(path, c::DELETE | c::FILE_LIST_DIRECTORY)?;
let file = File::open(path, &opts)?;

// Test if the file is not a directory or a symlink to a directory.
if (file.basic_info()?.FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) == 0 {
return Err(io::Error::from_raw_os_error(c::ERROR_DIRECTORY as _));
}

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 {
// If POSIX delete is not supported for this filesystem then fallback to win32 delete.
c::ERROR_NOT_SUPPORTED
| c::ERROR_INVALID_FUNCTION
| c::ERROR_INVALID_PARAMETER => {
remove_dir_all_iterative(&file, File::win32_delete)
}
_ => Err(e),
}
} else {
Err(e)
}
}
ok => ok,
}
}

fn remove_dir_all_iterative(f: &File, delete: fn(&File) -> io::Result<()>) -> io::Result<()> {
// When deleting files we may loop this many times when certain error conditions occur.
// This allows remove_dir_all to succeed when the error is temporary.
const MAX_RETRIES: u32 = 10;

let mut buffer = DirBuff::new();
let mut dirlist = vec![f.duplicate()?];

// FIXME: This is a hack so we can push to the dirlist vec after borrowing from it.
fn copy_handle(f: &File) -> mem::ManuallyDrop<File> {
unsafe { mem::ManuallyDrop::new(File::from_raw_handle(f.as_raw_handle())) }
}

let mut restart = true;
while let Some(dir) = dirlist.last() {
let dir = copy_handle(dir);

// Fill the buffer and iterate the entries.
let more_data = dir.fill_dir_buff(&mut buffer, restart)?;
restart = false;
for (name, is_directory) in buffer.iter() {
if is_directory {
let child_dir = open_link_no_reparse(
&dir,
&name,
c::SYNCHRONIZE | c::DELETE | c::FILE_LIST_DIRECTORY,
);
// On success, add the handle to the queue.
// If opening the directory fails we treat it the same as a file
if let Ok(child_dir) = child_dir {
dirlist.push(child_dir);
continue;
}
}
for i in 1..=MAX_RETRIES {
let result = open_link_no_reparse(&dir, &name, c::SYNCHRONIZE | c::DELETE);
match result {
Ok(f) => delete(&f)?,
// Already deleted, so skip.
Err(e) if e.kind() == io::ErrorKind::NotFound => break,
// Retry a few times if the file is locked or a delete is already in progress.
Err(e)
if i < MAX_RETRIES
&& (e.raw_os_error() == Some(c::ERROR_DELETE_PENDING as _)
|| e.raw_os_error() == Some(c::ERROR_SHARING_VIOLATION as _)) => {}
// Otherwise return the error.
Err(e) => return Err(e),
}
thread::yield_now();
}
}
// If there were no more files then delete the directory.
if !more_data {
if let Some(dir) = dirlist.pop() {
// Retry deleting a few times in case we need to wait for a file to be deleted.
for i in 1..=MAX_RETRIES {
let result = delete(&dir);
if let Err(e) = result {
if i == MAX_RETRIES || e.kind() != io::ErrorKind::DirectoryNotEmpty {
return Err(e);
}
thread::yield_now();
} else {
break;
}
}
}
}
}
Ok(())
// Remove the directory and all its contents.
remove_dir_all_iterative(file).io_result()
}

pub fn readlink(path: &Path) -> io::Result<PathBuf> {
Expand Down
Loading

0 comments on commit 72cc383

Please sign in to comment.