Skip to content

Windows: Files marked as read-only fail to get deleted #13084

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.

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

Open
squeek502 opened this issue Oct 6, 2022 · 2 comments
Open

Windows: Files marked as read-only fail to get deleted #13084

squeek502 opened this issue Oct 6, 2022 · 2 comments
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@squeek502
Copy link
Collaborator

squeek502 commented Oct 6, 2022

Zig Version

0.10.0-dev.4247+3234e8de3

Steps to Reproduce

The std.fs permissions tests take special steps to undo the read-only flag before allowing the tmp_dir to be cleaned up because otherwise the file (and tmp dir) would fail to get cleaned up (due to CANNOT_DELETE returned from DeleteFile):

zig/lib/std/fs/test.zig

Lines 1243 to 1245 in 3234e8d

// Must be set to non-read-only to delete
permissions.setReadOnly(false);
try file.setPermissions(permissions);

This is required; from https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-deletefile:

To delete a read-only file, first you must remove the read-only attribute.

CANNOT_DELETE being translated to error.AccessDenied was done by me in #10495 but I'm second guessing whether or not this is the correct way to go, as it means that deleteFile will fail for read-only files and deleteTree will fail for any tree with a read-only file inside it, the latter of which is probably unexpected behavior.

An alternate implementation could be to try to recover from CANNOT_DELETE by trying to unset the read-only flag and then retrying the delete. Where exactly this handling of deletion of read-only files should be handled I'm unsure about (at the deleteFile level, or only for deleteTree?).

Expected Behavior

deleteTree and possibily deleteFile successfully delete read-only files on Windows

Actual Behavior

deleteFile fails on read-only files with error.AccessDenied, and in turn deleteTree fails and returns error.AccessDenied whenever it hits a read-only file.

@squeek502 squeek502 added the bug Observed behavior contradicts documented or intended behavior label Oct 6, 2022
@Vexu Vexu added os-windows standard library This issue involves writing Zig code for the standard library. labels Oct 6, 2022
@Vexu Vexu added this to the 0.11.0 milestone Oct 6, 2022
@matu3ba
Copy link
Contributor

matu3ba commented Oct 11, 2022

deleteFile will fail for read-only files and deleteTree will fail for any tree with a read-only file inside it

Read-only files can not be deleted atomically, which has semantic implications as can be seen in this PR comment #11793 (comment) and the one after that.

The latter strategy is also done within boost with I think 3 or 4 runtime fallback strategies for Windows backwards compatibility.

deleteTree and possibily deleteFile successfully delete read-only files on Windows

Bear in mind, that we may be unable to change permissions, if the filesystem is mounted as read-only. Is there a way to check this without trying to delete to prevent the developer needing to think about handling read-only filesystems as usability footgun?

@squeek502
Copy link
Collaborator Author

squeek502 commented Jun 17, 2023

I believe this has been partially mitigated by #15501 (and potentially fully mitigated in Windows >= .win10_rs1, but I'm not fully sure exactly how it affects this particular issue).

EDIT: I think if a >= .win10_rs1 test was added that:

  • creates a directory tree with read-only files/directories in it
  • calls deleteTree on the directory
  • checks that the tree was fully deleted

then this issue can be closed.

@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jul 20, 2023
@andrewrk andrewrk modified the milestones: 0.14.0, 0.16.0 Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

4 participants