Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: remove readonly files #134679

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+25 −5
Open

Windows: remove readonly files #134679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bd962b
Windows: remove readonly files
ChrisDenton Dec 23, 2024
6ffc9ca
Update platform information for remove_file
ChrisDenton Dec 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions library/std/src/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2284,8 +2284,8 @@ impl AsInner<fs_imp::DirEntry> for DirEntry {
///
/// # Platform-specific behavior
///
/// This function currently corresponds to the `unlink` function on Unix
/// and the `DeleteFile` function on Windows.
/// This function currently corresponds to the `unlink` function on Unix.
/// On Windows, `DeleteFile` is used or `CreateFileW` and `SetInformationByHandle` for readonly files.
/// Note that, this [may change in the future][changes].
///
/// [changes]: io#platform-specific-behavior
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/fs/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@ fn file_try_clone() {
}

#[test]
#[cfg(not(windows))]
#[cfg(not(target_vendor = "win7"))]
fn unlink_readonly() {
let tmpdir = tmpdir();
let path = tmpdir.join("file");
Expand Down
24 changes: 22 additions & 2 deletions library/std/src/sys/pal/windows/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,10 @@ impl OpenOptions {
impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
let path = maybe_verbatim(path)?;
Self::open_native(&path, opts)
}

fn open_native(path: &[u16], opts: &OpenOptions) -> io::Result<File> {
let creation = opts.get_creation_mode()?;
let handle = unsafe {
c::CreateFileW(
Expand Down Expand Up @@ -1217,8 +1221,24 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {

pub fn unlink(p: &Path) -> io::Result<()> {
let p_u16s = maybe_verbatim(p)?;
cvt(unsafe { c::DeleteFileW(p_u16s.as_ptr()) })?;
Ok(())
if unsafe { c::DeleteFileW(p_u16s.as_ptr()) } == 0 {
let err = api::get_last_error();
// if `DeleteFileW` fails with ERROR_ACCESS_DENIED then try to remove
// the file while ignoring the readonly attribute.
// This is accomplished by calling the `posix_delete` function on an open file handle.
if err == WinError::ACCESS_DENIED {
let mut opts = OpenOptions::new();
opts.access_mode(c::DELETE);
opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT);
if File::open_native(&p_u16s, &opts).map(|f| f.posix_delete()).is_ok() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this cause concurrent deletes of the same file to fail due to having an open handle (iirc windows doesn't let you delete files with open handles?)? I guess before it would also fail with the same error, so not sure that matters even if true...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeleteFile does also open the file in a similar way to we're doing so there's actually no change there.

iirc windows doesn't let you delete files with open handles

That's not quite right. When opening files there's a "sharing mode" that lets you allow others to open the same file. One of those modes is FILE_SHARE_DELETE. If someone opens a file without that share mode then it will prevent any deletions no matter what we do.

Deletes can also race with each other because there's a time between marking a file for deletion and closing the handle where the file is in a kind of limbo state. It can't be opened but it still exists on the filesystem.

But either case is the same whether we use DeleteFile or open the file ourselves.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, sounds good. Thanks for clarifying! :)

return Ok(());
}
}
// return the original error if any of the above fails.
Err(io::Error::from_raw_os_error(err.code as i32))
} else {
Ok(())
}
}

pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
Expand Down
Loading