From 0f0f005e927be5e9d813e40c8c6c6580fd4f5697 Mon Sep 17 00:00:00 2001 From: Martin Wickham Date: Sat, 4 Jun 2022 17:34:21 -0500 Subject: [PATCH 1/3] std.windows: use posix semantics to delete files, if available Justification: When a file is deleted on Windows, it may not be immediately removed from the directory. This can cause problems with future scans of that directory, which will see the partially deleted file. Under some workloads and system configurations, Windows files may appear to be deleted immediately. This is the PR with requested fixup. Thanks to @SpexGuy for the original PR. --- lib/std/os/windows.zig | 75 +++++++++++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 22ffc850e6..37ec7f09a3 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -937,25 +937,50 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil .DELETE_PENDING => return, else => return unexpectedStatus(rc), } - var file_dispo = FILE_DISPOSITION_INFORMATION{ - .DeleteFile = TRUE, - }; - rc = ntdll.NtSetInformationFile( - tmp_handle, - &io, - &file_dispo, - @sizeOf(FILE_DISPOSITION_INFORMATION), - .FileDispositionInformation, - ); - CloseHandle(tmp_handle); - switch (rc) { - .SUCCESS => return, - .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty, - .INVALID_PARAMETER => unreachable, - .CANNOT_DELETE => return error.AccessDenied, - .MEDIA_WRITE_PROTECTED => return error.AccessDenied, - .ACCESS_DENIED => return error.AccessDenied, - else => return unexpectedStatus(rc), + defer CloseHandle(tmp_handle); + if (comptime builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) { + // Deletion with posix semantics. + var info = FILE_DISPOSITION_INFORMATION_EX{ + .Flags = FILE_DISPOSITION_DELETE | + FILE_DISPOSITION_POSIX_SEMANTICS | + FILE_DISPOSITION_ON_CLOSE | + FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE, + }; + + rc = ntdll.NtSetInformationFile( + tmp_handle, + &io, + &info, + @sizeOf(FILE_DISPOSITION_INFORMATION_EX), + .FileDispositionInformationEx, + ); + switch (rc) { + .SUCCESS => {}, + .CANNOT_DELETE => return error.FileBusy, // file is currently mapped + else => return unexpectedStatus(rc), + } + } else { + // Deletion with file pending semantics, which requires waiting or moving + // files to get them removed (from here). + var file_dispo = FILE_DISPOSITION_INFORMATION{ + .DeleteFile = TRUE, + }; + rc = ntdll.NtSetInformationFile( + tmp_handle, + &io, + &file_dispo, + @sizeOf(FILE_DISPOSITION_INFORMATION), + .FileDispositionInformation, + ); + switch (rc) { + .SUCCESS => {}, + .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty, + .INVALID_PARAMETER => unreachable, + .CANNOT_DELETE => return error.AccessDenied, + .MEDIA_WRITE_PROTECTED => return error.AccessDenied, + .ACCESS_DENIED => return error.AccessDenied, + else => return unexpectedStatus(rc), + } } } @@ -2397,6 +2422,18 @@ pub const FILE_NAME_INFORMATION = extern struct { FileName: [1]WCHAR, }; +pub const FILE_DISPOSITION_INFORMATION_EX = extern struct { + /// combination of FILE_DISPOSITION_* flags + Flags: ULONG, +}; + +const FILE_DISPOSITION_DO_NOT_DELETE: ULONG = 0x00000000; +const FILE_DISPOSITION_DELETE: ULONG = 0x00000001; +const FILE_DISPOSITION_POSIX_SEMANTICS: ULONG = 0x00000002; +const FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK: ULONG = 0x00000004; +const FILE_DISPOSITION_ON_CLOSE: ULONG = 0x00000008; +const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: ULONG = 0x00000010; + pub const FILE_RENAME_INFORMATION = extern struct { ReplaceIfExists: BOOLEAN, RootDirectory: ?HANDLE, From be50dbf1ce03f5dc5deef86882ab3505363b683a Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 1 May 2023 12:45:41 +0200 Subject: [PATCH 2/3] apply suggestion by user @xEgoist FILE_DISPOSITION_ON_CLOSE is used to set/clear the FILE_DELETE_ON_CLOSE, but we do not use that anymore and FILE_DISPOSITION_POSIX_SEMANTICS already implies unmapping of the handle and removal fo it on close. --- lib/std/os/windows.zig | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 37ec7f09a3..e6ce95713c 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -943,7 +943,6 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil var info = FILE_DISPOSITION_INFORMATION_EX{ .Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS | - FILE_DISPOSITION_ON_CLOSE | FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE, }; From 7594d2c0977497c81db0c394f775832689bad492 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 1 May 2023 18:22:28 +0200 Subject: [PATCH 3/3] address review by user @squeek502 --- lib/std/fs/test.zig | 35 +++++++++++++++++++++++++++-------- lib/std/os/windows.zig | 25 +++++++++++-------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 15c8307f58..660ec0c381 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -1416,23 +1416,42 @@ test "File.PermissionsUnix" { try testing.expect(!permissions_unix.unixHas(.other, .execute)); } -test "delete a read-only file on windows" { - if (builtin.os.tag != .windows) return error.SkipZigTest; +test "delete a read-only file on windows with file pending semantics" { + if (builtin.os.tag != .windows or builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) + return error.SkipZigTest; + + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + { + const file = try tmp.dir.createFile("test_file", .{ .read = true }); + defer file.close(); + // Create a file and make it read-only + const metadata = try file.metadata(); + var permissions = metadata.permissions(); + permissions.setReadOnly(true); + try file.setPermissions(permissions); + try testing.expectError(error.AccessDenied, tmp.dir.deleteFile("test_file")); + // Now make the file not read-only + permissions.setReadOnly(false); + try file.setPermissions(permissions); + } + try tmp.dir.deleteFile("test_file"); +} + +test "delete a read-only file on windows with posix semantis" { + if (builtin.os.tag != .windows or !builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) + return error.SkipZigTest; var tmp = tmpDir(.{}); defer tmp.cleanup(); const file = try tmp.dir.createFile("test_file", .{ .read = true }); + defer file.close(); // Create a file and make it read-only const metadata = try file.metadata(); var permissions = metadata.permissions(); permissions.setReadOnly(true); try file.setPermissions(permissions); - try testing.expectError(error.AccessDenied, tmp.dir.deleteFile("test_file")); - // Now make the file not read-only - permissions.setReadOnly(false); - try file.setPermissions(permissions); - file.close(); - try tmp.dir.deleteFile("test_file"); + try tmp.dir.deleteFile("test_file"); // file is unmapped and deleted once last handle closed } test "delete a setAsCwd directory on Windows" { diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index e6ce95713c..ef2643792e 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -938,6 +938,7 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil else => return unexpectedStatus(rc), } defer CloseHandle(tmp_handle); + if (comptime builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) { // Deletion with posix semantics. var info = FILE_DISPOSITION_INFORMATION_EX{ @@ -953,17 +954,13 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil @sizeOf(FILE_DISPOSITION_INFORMATION_EX), .FileDispositionInformationEx, ); - switch (rc) { - .SUCCESS => {}, - .CANNOT_DELETE => return error.FileBusy, // file is currently mapped - else => return unexpectedStatus(rc), - } } else { // Deletion with file pending semantics, which requires waiting or moving // files to get them removed (from here). var file_dispo = FILE_DISPOSITION_INFORMATION{ .DeleteFile = TRUE, }; + rc = ntdll.NtSetInformationFile( tmp_handle, &io, @@ -971,15 +968,15 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil @sizeOf(FILE_DISPOSITION_INFORMATION), .FileDispositionInformation, ); - switch (rc) { - .SUCCESS => {}, - .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty, - .INVALID_PARAMETER => unreachable, - .CANNOT_DELETE => return error.AccessDenied, - .MEDIA_WRITE_PROTECTED => return error.AccessDenied, - .ACCESS_DENIED => return error.AccessDenied, - else => return unexpectedStatus(rc), - } + } + switch (rc) { + .SUCCESS => {}, + .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty, + .INVALID_PARAMETER => unreachable, + .CANNOT_DELETE => return error.AccessDenied, + .MEDIA_WRITE_PROTECTED => return error.AccessDenied, + .ACCESS_DENIED => return error.AccessDenied, + else => return unexpectedStatus(rc), } }