Skip to content

Windows: std.fs.Dir.deleteDir() will silently fail on non-empty directories #5537

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

Closed
squeek502 opened this issue Jun 4, 2020 · 3 comments · Fixed by #15316
Closed

Windows: std.fs.Dir.deleteDir() will silently fail on non-empty directories #5537

squeek502 opened this issue Jun 4, 2020 · 3 comments · Fixed by #15316
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

When ./dir is non-empty, the following code will succeed but the directory will not be deleted. I expect error.DirNotEmpty to be returned in this case.

const std = @import("std");

pub fn main() !void {
    return std.fs.cwd().deleteDir("dir");
}

Both NtCreateFile and NtClose seem to be returning SUCCESS here:

zig/lib/std/os.zig

Lines 1768 to 1791 in fd067fb

var rc = w.ntdll.NtCreateFile(
&tmp_handle,
w.SYNCHRONIZE | w.DELETE,
&attr,
&io,
null,
0,
w.FILE_SHARE_READ | w.FILE_SHARE_WRITE | w.FILE_SHARE_DELETE,
w.FILE_OPEN,
create_options_flags,
null,
0,
);
if (rc == .SUCCESS) {
rc = w.ntdll.NtClose(tmp_handle);
}
switch (rc) {
.SUCCESS => return,
.OBJECT_NAME_INVALID => unreachable,
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.INVALID_PARAMETER => unreachable,
.FILE_IS_A_DIRECTORY => return error.IsDir,
else => return w.unexpectedStatus(rc),
}

@squeek502
Copy link
Collaborator Author

squeek502 commented Jun 5, 2020

From https://go.microsoft.com/fwlink/?LinkId=140636 section 4.3.1 (page 33):

The delete on close flag can be specified when creating or opening a link or stream. While the handle remains open, the handle is in the delete-on-close state. This means that additional handles can be opened to the file or stream provided they specify FILE_SHARE_DELETE access. After receiving the cleanup IRP for a handle that specified DELETE_ON_CLOSE, the handle’s corresponding link or stream enters the delete-pending state. While in this state any attempt to open the link or stream will fail with STATUS_DELETE_PENDING.

Note that if the DELETE_ON_CLOSE flag is specified on a directory with child files or directories, the create operation will succeed, but the delete on close flag will be silently ignored when processing the cleanup IRP and the directory will not be deleted.

Seems like FileDispositionInformation might be better for this use case? See the linked PDF above.

EDIT:
The relevant FileDispositionInformation info from section 4.3.2 (page 33):

4.3.2 Set Delete-on-closeusing FileDispositionInformationInformation Class (IRP_MJ_SET_INFORMATION)

The FileDispositionInformation information class can be used to mark a link or stream for deletion and transition it to the delete-pending state. In this state attempts to open the link or stream will fail with STATUS_DELETE_PENDING.

This operation can fail for any of the following reasons:

  • The handle wasopened by ID –STATUS_INVALID_PARAMTER
  • The file is an internal file system metadata file –STATUS_CANNOT_DELETE
  • The file is marked read-only –STATUS_CANNOT_DELETE
  • The volume is marked read-only _-STATUS_MEDIA_WRITE_PROTECTED or STATUS_ACCESS_DENIED (on CDFS & UDF file systems when the file system is read-only)
  • The handle is to a directory that still has files or directories in it –STATUS_DIRECTORY_NOT_EMTPY

@Vexu Vexu added bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library. labels Jun 7, 2020
@Vexu Vexu added this to the 0.7.0 milestone Jun 7, 2020
@kubkon
Copy link
Member

kubkon commented Sep 27, 2020

Should be fixed with #6397 so closing. If the issue persists, please feel free to reopen!

@squeek502
Copy link
Collaborator Author

Should be re-opened as #6397 was reverted in #6462

(for some reason I can't re-open it even though I started this issue?)

@andrewrk andrewrk reopened this Sep 30, 2020
squeek502 added a commit to squeek502/zig that referenced this issue Sep 30, 2020
Re-adds the test that was added and then reverted in ziglang#6397, but with the test for ziglang#5537 skipped for now since that issue is no longer fixed.
andrewrk pushed a commit that referenced this issue Sep 30, 2020
Re-adds the test that was added and then reverted in #6397, but with the test for #5537 skipped for now since that issue is no longer fixed.
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 30, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.8.1 Jun 4, 2021
@andrewrk andrewrk modified the milestones: 0.8.1, 0.9.1 Sep 1, 2021
@andrewrk andrewrk modified the milestones: 0.9.1, 0.9.0, 0.10.0 Nov 20, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
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

Successfully merging a pull request may close this issue.

4 participants