Skip to content

std.fs: support Lock.none on Windows #21306

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
wants to merge 7 commits into
base: master
Choose a base branch
from
34 changes: 28 additions & 6 deletions lib/std/fs/File.zig
Original file line number Diff line number Diff line change
@@ -1617,14 +1617,23 @@ pub const LockError = error{
/// A process may hold only one type of lock (shared or exclusive) on
/// a file. When a process terminates in any way, the lock is released.
///
/// Assumes the file is unlocked.
/// Assumes the file is unlocked, or in the case of Lock.none, locked.
///
/// TODO: integrate with async I/O
pub fn lock(file: File, l: Lock) LockError!void {
if (is_windows) {
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
const exclusive = switch (l) {
.none => return,
.none => return windows.UnlockFile(
file.handle,
&io_status_block,
&range_off,
&range_len,
null,
) catch |err| switch (err) {
error.RangeNotLocked => unreachable, // The file is assumed to be locked.
error.Unexpected => return error.Unexpected,
},
.shared => false,
.exclusive => true,
};
@@ -1666,7 +1675,7 @@ pub fn unlock(file: File) void {
&range_len,
null,
) catch |err| switch (err) {
error.RangeNotLocked => unreachable, // Function assumes unlocked.
error.RangeNotLocked => unreachable, // Function assumes locked.
error.Unexpected => unreachable, // Resource deallocation must succeed.
};
} else {
@@ -1684,14 +1693,27 @@ pub fn unlock(file: File) void {
/// A process may hold only one type of lock (shared or exclusive) on
/// a file. When a process terminates in any way, the lock is released.
///
/// Assumes the file is unlocked.
/// Assumes the file is unlocked, or in the case of Lock.none, locked.
///
/// TODO: integrate with async I/O
pub fn tryLock(file: File, l: Lock) LockError!bool {
if (is_windows) {
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
const exclusive = switch (l) {
.none => return,
.none => {
windows.UnlockFile(
file.handle,
&io_status_block,
&range_off,
&range_len,
null,
) catch |err| switch (err) {
error.RangeNotLocked => unreachable, // The file is assumed to be locked.
error.Unexpected => return error.Unexpected,
};

return true;
},
.shared => false,
.exclusive => true,
};
@@ -1758,7 +1780,7 @@ pub fn downgradeLock(file: File) LockError!void {
null,
) catch |err| switch (err) {
error.RangeNotLocked => unreachable, // File was not locked.
error.Unexpected => unreachable, // Resource deallocation must succeed.
error.Unexpected => return error.Unexpected,
Comment on lines -1761 to +1783
Copy link
Member

Choose a reason for hiding this comment

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

Should this change also be made in unlock()?

Copy link
Collaborator

Choose a reason for hiding this comment

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

unlock can't return an error.

Copy link
Member

Choose a reason for hiding this comment

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

Then it seems questionable to me that we're doing it here. If we're worried that the OS might become able to return an error here in the future, the same concern surely applies to unlock()? Are we just crossing our fingers and hoping that such an error will be safe to ignore in unlock()?

Copy link
Collaborator

@squeek502 squeek502 Mar 26, 2025

Choose a reason for hiding this comment

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

Are we just crossing our fingers and hoping that such an error will be safe to ignore in unlock()?

I think so; same sort of thing with File.close. I agree that it seems strange for unlock, not sure that should really count as "resource deallocation."

FWIW, there's exactly one instance of File.unlock in a defer that I could find:

const locked = if (key_log_file.lock(.exclusive)) |_| true else |_| false;
defer if (locked) key_log_file.unlock();

};
} else {
return posix.flock(file.handle, posix.LOCK.SH | posix.LOCK.NB) catch |err| switch (err) {
25 changes: 25 additions & 0 deletions lib/std/fs/test.zig
Original file line number Diff line number Diff line change
@@ -1923,6 +1923,31 @@ test "File.PermissionsUnix" {
try testing.expect(!permissions_unix.unixHas(.other, .execute));
}

test "use Lock.none to unlock files" {
if (native_os == .wasi) return error.SkipZigTest;

var tmp = tmpDir(.{});
defer tmp.cleanup();

// Create a locked file.
const test_file = try tmp.dir.createFile("test_file", .{ .lock = .exclusive, .lock_nonblocking = true });
defer test_file.close();

// Attempt to unlock the file via fs.lock with Lock.none.
try File.lock(test_file, .none);

// Attempt to open the file now that it should be unlocked.
const test_file2 = try tmp.dir.openFile("test_file", .{ .lock = .exclusive, .lock_nonblocking = true });
defer test_file2.close();

// Make sure Lock.none works with tryLock as well.
try testing.expect(try File.tryLock(test_file2, .none));

// Attempt to open the file since it should be unlocked again.
const test_file3 = try tmp.dir.openFile("test_file", .{});
test_file3.close();
}

test "delete a read-only file on windows" {
if (native_os != .windows)
return error.SkipZigTest;
4 changes: 2 additions & 2 deletions lib/std/os/windows.zig
Original file line number Diff line number Diff line change
@@ -2054,7 +2054,7 @@ pub fn LockFile(
Key: ?*ULONG,
FailImmediately: BOOLEAN,
ExclusiveLock: BOOLEAN,
) !void {
) LockFileError!void {
const rc = ntdll.NtLockFile(
FileHandle,
Event,
@@ -2086,7 +2086,7 @@ pub fn UnlockFile(
ByteOffset: *const LARGE_INTEGER,
Length: *const LARGE_INTEGER,
Key: ?*ULONG,
) !void {
) UnlockFileError!void {
const rc = ntdll.NtUnlockFile(FileHandle, IoStatusBlock, ByteOffset, Length, Key);
switch (rc) {
.SUCCESS => return,