From 3110060351f98ea8de65809379e1bcd9607fb1cb Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sun, 8 Mar 2020 16:28:49 -0600 Subject: [PATCH 01/39] Add `lock` to fs.File.OpenFlags --- lib/std/fs/file.zig | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index c4991a81ff..e892fc3b8f 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -41,6 +41,15 @@ pub const File = struct { read: bool = true, write: bool = false, + /// Open the file with exclusive access. If `write` is true, then the file is opened with an + /// exclusive lock, meaning that no other processes can read or write to the file. Otherwise + /// the file is opened with a shared lock, allowing the other processes to read from the + /// file, but not to write to the file. + /// + /// Note that the lock is only advisory on Linux. This means that a process that does not + /// respect the locking API can still read and write to the file, despite the lock. + lock: bool = false, + /// This prevents `O_NONBLOCK` from being passed even if `std.io.is_async`. /// It allows the use of `noasync` when calling functions related to opening /// the file, reading, and writing. From 9af0590a282e3cf1aa209f4de35402a50effe7a8 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sun, 8 Mar 2020 20:57:47 -0600 Subject: [PATCH 02/39] Add fnctlFlock system call, use it to lock files --- lib/std/fs.zig | 14 ++++++++++++++ lib/std/os.zig | 8 ++++++++ lib/std/os/bits/linux/i386.zig | 8 ++++++++ lib/std/os/bits/linux/x86_64.zig | 12 ++++++++++++ lib/std/os/linux.zig | 4 ++++ 5 files changed, 46 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index de6be91f71..edb12e0f01 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -677,6 +677,20 @@ pub const Dir = struct { try std.event.Loop.instance.?.openatZ(self.fd, sub_path, os_flags, 0) else try os.openatC(self.fd, sub_path, os_flags, 0); + + var locked = false; + if (flags.lock) { + // TODO: integrate async I/O + try os.fcntlFlockBlocking(fd, &os.flock{ + .lock_type = if (flags.write) os.F_WRLCK else os.F_RDLCK, + .whence = os.SEEK_SET, + .start = 0, + .len = 0, + .pid = 0, + }); + locked = true; + } + return File{ .handle = fd, .io_mode = .blocking, diff --git a/lib/std/os.zig b/lib/std/os.zig index ed99c48021..ca007ecb44 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1140,6 +1140,14 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) allocator.free(envp_buf); } +/// Attempts to get lock the file, blocking if the file is locked. +pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *flock) OpenError!void { + const rc = system.fcntlFlock(fd, F_SETLKW, flock_p); + if (rc < 0) { + std.debug.panic("fcntl error: {}\n", .{rc}); + } +} + /// Get an environment variable. /// See also `getenvZ`. pub fn getenv(key: []const u8) ?[]const u8 { diff --git a/lib/std/os/bits/linux/i386.zig b/lib/std/os/bits/linux/i386.zig index 2585785b13..e075f1826e 100644 --- a/lib/std/os/bits/linux/i386.zig +++ b/lib/std/os/bits/linux/i386.zig @@ -488,6 +488,14 @@ pub const MMAP2_UNIT = 4096; pub const VDSO_CGT_SYM = "__vdso_clock_gettime"; pub const VDSO_CGT_VER = "LINUX_2.6"; +pub const flock = extern struct { + lock_type: i16, + whence: i16, + start: off_t, + len: off_t, + pid: pid_t, +}; + pub const msghdr = extern struct { msg_name: ?*sockaddr, msg_namelen: socklen_t, diff --git a/lib/std/os/bits/linux/x86_64.zig b/lib/std/os/bits/linux/x86_64.zig index e92591d94e..97905e4a3e 100644 --- a/lib/std/os/bits/linux/x86_64.zig +++ b/lib/std/os/bits/linux/x86_64.zig @@ -456,6 +456,18 @@ pub const REG_TRAPNO = 20; pub const REG_OLDMASK = 21; pub const REG_CR2 = 22; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + +pub const flock = extern struct { + lock_type: i16, + whence: i16, + start: off_t, + len: off_t, + pid: pid_t, +}; + pub const msghdr = extern struct { msg_name: ?*sockaddr, msg_namelen: socklen_t, diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index ba7356d62c..389f110d0a 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -219,6 +219,10 @@ pub fn mmap(address: ?[*]u8, length: usize, prot: usize, flags: u32, fd: i32, of } } +pub fn fcntlFlock(fd: fd_t, cmd: i32, flock_p: *flock) usize { + return syscall3(SYS_fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), @ptrToInt(flock_p)); +} + pub fn mprotect(address: [*]const u8, length: usize, protection: usize) usize { return syscall3(SYS_mprotect, @ptrToInt(address), length, protection); } From e1868029e9c861fa6b1382ba52052264fc5f9c31 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 9 Mar 2020 20:48:00 -0600 Subject: [PATCH 03/39] Implement blocking file locking API for windows --- lib/std/fs.zig | 132 +++++++++++++++++++-------------- lib/std/os.zig | 4 +- lib/std/os/bits/linux/i386.zig | 1 + lib/std/os/linux.zig | 2 +- 4 files changed, 80 insertions(+), 59 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index edb12e0f01..4d00babb65 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -708,7 +708,12 @@ pub const Dir = struct { const access_mask = w.SYNCHRONIZE | (if (flags.read) @as(u32, w.GENERIC_READ) else 0) | (if (flags.write) @as(u32, w.GENERIC_WRITE) else 0); - return self.openFileWindows(sub_path_w, access_mask, w.FILE_OPEN); + const share_access = if (flags.lock) + w.FILE_SHARE_DELETE | + (if (flags.write) @as(os.windows.ULONG, 0) else w.FILE_SHARE_READ) + else + null; + return self.openFileWindows(sub_path_w, access_mask, share_access, w.FILE_OPEN); } /// Creates, opens, or overwrites a file with write access. @@ -753,7 +758,7 @@ pub const Dir = struct { @as(u32, w.FILE_OVERWRITE_IF) else @as(u32, w.FILE_OPEN_IF); - return self.openFileWindows(sub_path_w, access_mask, creation); + return self.openFileWindows(sub_path_w, access_mask, null, creation); } /// Deprecated; call `openFile` directly. @@ -775,65 +780,80 @@ pub const Dir = struct { self: Dir, sub_path_w: [*:0]const u16, access_mask: os.windows.ACCESS_MASK, + share_access_opt: ?os.windows.ULONG, creation: os.windows.ULONG, ) File.OpenError!File { - const w = os.windows; + var delay: usize = 1; + while (true) { + const w = os.windows; - if (sub_path_w[0] == '.' and sub_path_w[1] == 0) { - return error.IsDir; - } - if (sub_path_w[0] == '.' and sub_path_w[1] == '.' and sub_path_w[2] == 0) { - return error.IsDir; - } + if (sub_path_w[0] == '.' and sub_path_w[1] == 0) { + return error.IsDir; + } + if (sub_path_w[0] == '.' and sub_path_w[1] == '.' and sub_path_w[2] == 0) { + return error.IsDir; + } - var result = File{ - .handle = undefined, - .io_mode = .blocking, - }; + var result = File{ + .handle = undefined, + .io_mode = .blocking, + }; - const path_len_bytes = math.cast(u16, mem.toSliceConst(u16, sub_path_w).len * 2) catch |err| switch (err) { - error.Overflow => return error.NameTooLong, - }; - var nt_name = w.UNICODE_STRING{ - .Length = path_len_bytes, - .MaximumLength = path_len_bytes, - .Buffer = @intToPtr([*]u16, @ptrToInt(sub_path_w)), - }; - var attr = w.OBJECT_ATTRIBUTES{ - .Length = @sizeOf(w.OBJECT_ATTRIBUTES), - .RootDirectory = if (path.isAbsoluteWindowsW(sub_path_w)) null else self.fd, - .Attributes = 0, // Note we do not use OBJ_CASE_INSENSITIVE here. - .ObjectName = &nt_name, - .SecurityDescriptor = null, - .SecurityQualityOfService = null, - }; - var io: w.IO_STATUS_BLOCK = undefined; - const rc = w.ntdll.NtCreateFile( - &result.handle, - access_mask, - &attr, - &io, - null, - w.FILE_ATTRIBUTE_NORMAL, - w.FILE_SHARE_WRITE | w.FILE_SHARE_READ | w.FILE_SHARE_DELETE, - creation, - w.FILE_NON_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT, - null, - 0, - ); - switch (rc) { - .SUCCESS => return result, - .OBJECT_NAME_INVALID => unreachable, - .OBJECT_NAME_NOT_FOUND => return error.FileNotFound, - .OBJECT_PATH_NOT_FOUND => return error.FileNotFound, - .NO_MEDIA_IN_DEVICE => return error.NoDevice, - .INVALID_PARAMETER => unreachable, - .SHARING_VIOLATION => return error.SharingViolation, - .ACCESS_DENIED => return error.AccessDenied, - .PIPE_BUSY => return error.PipeBusy, - .OBJECT_PATH_SYNTAX_BAD => unreachable, - .OBJECT_NAME_COLLISION => return error.PathAlreadyExists, - else => return w.unexpectedStatus(rc), + const path_len_bytes = math.cast(u16, mem.toSliceConst(u16, sub_path_w).len * 2) catch |err| switch (err) { + error.Overflow => return error.NameTooLong, + }; + var nt_name = w.UNICODE_STRING{ + .Length = path_len_bytes, + .MaximumLength = path_len_bytes, + .Buffer = @intToPtr([*]u16, @ptrToInt(sub_path_w)), + }; + var attr = w.OBJECT_ATTRIBUTES{ + .Length = @sizeOf(w.OBJECT_ATTRIBUTES), + .RootDirectory = if (path.isAbsoluteWindowsW(sub_path_w)) null else self.fd, + .Attributes = 0, // Note we do not use OBJ_CASE_INSENSITIVE here. + .ObjectName = &nt_name, + .SecurityDescriptor = null, + .SecurityQualityOfService = null, + }; + var io: w.IO_STATUS_BLOCK = undefined; + const share_access = share_access_opt orelse w.FILE_SHARE_WRITE | w.FILE_SHARE_READ | w.FILE_SHARE_DELETE; + const rc = w.ntdll.NtCreateFile( + &result.handle, + access_mask, + &attr, + &io, + null, + w.FILE_ATTRIBUTE_NORMAL, + share_access, + creation, + w.FILE_NON_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT, + null, + 0, + ); + switch (rc) { + .SUCCESS => return result, + .OBJECT_NAME_INVALID => unreachable, + .OBJECT_NAME_NOT_FOUND => return error.FileNotFound, + .OBJECT_PATH_NOT_FOUND => return error.FileNotFound, + .NO_MEDIA_IN_DEVICE => return error.NoDevice, + .INVALID_PARAMETER => unreachable, + .SHARING_VIOLATION => { + // TODO: check if async or blocking + //return error.SharingViolation + // Sleep so we don't consume a ton of CPU waiting to get lock on file + std.time.sleep(delay); + // Increase sleep time as long as it is less than 5 seconds + if (delay < 5 * std.time.ns_per_s) { + delay *= 2; + } + continue; + }, + .ACCESS_DENIED => return error.AccessDenied, + .PIPE_BUSY => return error.PipeBusy, + .OBJECT_PATH_SYNTAX_BAD => unreachable, + .OBJECT_NAME_COLLISION => return error.PathAlreadyExists, + else => return w.unexpectedStatus(rc), + } } } diff --git a/lib/std/os.zig b/lib/std/os.zig index ca007ecb44..f645f73e26 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1141,8 +1141,8 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) } /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *flock) OpenError!void { - const rc = system.fcntlFlock(fd, F_SETLKW, flock_p); +pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *const flock) OpenError!void { + const rc = system.fcntl(fd, F_SETLKW, flock_p); if (rc < 0) { std.debug.panic("fcntl error: {}\n", .{rc}); } diff --git a/lib/std/os/bits/linux/i386.zig b/lib/std/os/bits/linux/i386.zig index e075f1826e..0043dd24e9 100644 --- a/lib/std/os/bits/linux/i386.zig +++ b/lib/std/os/bits/linux/i386.zig @@ -8,6 +8,7 @@ const iovec = linux.iovec; const iovec_const = linux.iovec_const; const uid_t = linux.uid_t; const gid_t = linux.gid_t; +const pid_t = linux.pid_t; const stack_t = linux.stack_t; const sigset_t = linux.sigset_t; diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 389f110d0a..4b29a1cc27 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -219,7 +219,7 @@ pub fn mmap(address: ?[*]u8, length: usize, prot: usize, flags: u32, fd: i32, of } } -pub fn fcntlFlock(fd: fd_t, cmd: i32, flock_p: *flock) usize { +pub fn fcntl(fd: fd_t, cmd: i32, flock_p: *const c_void) usize { return syscall3(SYS_fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), @ptrToInt(flock_p)); } From f66a607607c20841ed59a1fe11486333f8621426 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 9 Mar 2020 22:04:05 -0600 Subject: [PATCH 04/39] Define Flock for all posix systems --- lib/std/fs.zig | 15 ++++++++------- lib/std/os.zig | 2 +- lib/std/os/bits/darwin.zig | 8 ++++++++ lib/std/os/bits/freebsd.zig | 14 ++++++++++++++ lib/std/os/bits/linux/arm-eabi.zig | 15 +++++++++++++++ lib/std/os/bits/linux/arm64.zig | 14 ++++++++++++++ lib/std/os/bits/linux/i386.zig | 16 ++++++++++------ lib/std/os/bits/linux/mipsel.zig | 15 +++++++++++++++ lib/std/os/bits/linux/riscv64.zig | 14 ++++++++++++++ lib/std/os/bits/linux/x86_64.zig | 12 ++++++------ lib/std/os/bits/netbsd.zig | 12 ++++++++++++ 11 files changed, 117 insertions(+), 20 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 4d00babb65..c508626628 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -681,13 +681,14 @@ pub const Dir = struct { var locked = false; if (flags.lock) { // TODO: integrate async I/O - try os.fcntlFlockBlocking(fd, &os.flock{ - .lock_type = if (flags.write) os.F_WRLCK else os.F_RDLCK, - .whence = os.SEEK_SET, - .start = 0, - .len = 0, - .pid = 0, - }); + // mem.zeroes is used here because flock's structure can vary across architectures and systems + var flock = mem.zeroes(os.Flock); + flock.l_type = if (flags.write) os.F_WRLCK else os.F_RDLCK; + flock.l_whence = os.SEEK_SET; + flock.l_start = 0; + flock.l_len = 0; + flock.l_pid = 0; + try os.fcntlFlockBlocking(fd, &flock); locked = true; } diff --git a/lib/std/os.zig b/lib/std/os.zig index f645f73e26..3b1af4e9cd 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1141,7 +1141,7 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) } /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *const flock) OpenError!void { +pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *const Flock) OpenError!void { const rc = system.fcntl(fd, F_SETLKW, flock_p); if (rc < 0) { std.debug.panic("fcntl error: {}\n", .{rc}); diff --git a/lib/std/os/bits/darwin.zig b/lib/std/os/bits/darwin.zig index 6808af0315..0d6ffc6432 100644 --- a/lib/std/os/bits/darwin.zig +++ b/lib/std/os/bits/darwin.zig @@ -55,6 +55,14 @@ pub const mach_timebase_info_data = extern struct { pub const off_t = i64; pub const ino_t = u64; +pub const Flock = extern struct { + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + l_type: i16, + l_whence: i16, +}; + /// Renamed to Stat to not conflict with the stat function. /// atime, mtime, and ctime have functions to return `timespec`, /// because although this is a POSIX API, the layout and names of diff --git a/lib/std/os/bits/freebsd.zig b/lib/std/os/bits/freebsd.zig index 02fe7e75b3..2648e4a98f 100644 --- a/lib/std/os/bits/freebsd.zig +++ b/lib/std/os/bits/freebsd.zig @@ -51,6 +51,16 @@ pub const dl_phdr_info = extern struct { dlpi_phnum: u16, }; +pub const Flock = extern struct { + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + l_type: i16, + l_whence: i16, + l_sysid: i32, + __unused: [4]u8, +}; + pub const msghdr = extern struct { /// optional address msg_name: ?*sockaddr, @@ -350,6 +360,10 @@ pub const F_GETLK = 5; pub const F_SETLK = 6; pub const F_SETLKW = 7; +pub const F_RDLCK = 1; +pub const F_WRLCK = 3; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/linux/arm-eabi.zig b/lib/std/os/bits/linux/arm-eabi.zig index de71607c30..3788d17a98 100644 --- a/lib/std/os/bits/linux/arm-eabi.zig +++ b/lib/std/os/bits/linux/arm-eabi.zig @@ -8,6 +8,7 @@ const stack_t = linux.stack_t; const sigset_t = linux.sigset_t; const uid_t = linux.uid_t; const gid_t = linux.gid_t; +const pid_t = linux.pid_t; pub const SYS_restart_syscall = 0; pub const SYS_exit = 1; @@ -446,6 +447,10 @@ pub const F_GETLK = 12; pub const F_SETLK = 13; pub const F_SETLKW = 14; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; @@ -493,6 +498,16 @@ pub const HWCAP_IDIV = HWCAP_IDIVA | HWCAP_IDIVT; pub const HWCAP_LPAE = 1 << 20; pub const HWCAP_EVTSTRM = 1 << 21; +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + __pad0: [4]u8, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + __unused: [4]u8, +}; + pub const msghdr = extern struct { msg_name: ?*sockaddr, msg_namelen: socklen_t, diff --git a/lib/std/os/bits/linux/arm64.zig b/lib/std/os/bits/linux/arm64.zig index 386e889873..a715a2fa21 100644 --- a/lib/std/os/bits/linux/arm64.zig +++ b/lib/std/os/bits/linux/arm64.zig @@ -8,6 +8,7 @@ const iovec = linux.iovec; const iovec_const = linux.iovec_const; const uid_t = linux.uid_t; const gid_t = linux.gid_t; +const pid_t = linux.pid_t; const stack_t = linux.stack_t; const sigset_t = linux.sigset_t; @@ -339,6 +340,10 @@ pub const F_GETLK = 5; pub const F_SETLK = 6; pub const F_SETLKW = 7; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; @@ -362,6 +367,15 @@ pub const MAP_NORESERVE = 0x4000; pub const VDSO_CGT_SYM = "__kernel_clock_gettime"; pub const VDSO_CGT_VER = "LINUX_2.6.39"; +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + __unused: [4]u8, +}; + pub const msghdr = extern struct { msg_name: ?*sockaddr, msg_namelen: socklen_t, diff --git a/lib/std/os/bits/linux/i386.zig b/lib/std/os/bits/linux/i386.zig index 0043dd24e9..cffea9e387 100644 --- a/lib/std/os/bits/linux/i386.zig +++ b/lib/std/os/bits/linux/i386.zig @@ -472,6 +472,10 @@ pub const F_GETLK = 12; pub const F_SETLK = 13; pub const F_SETLKW = 14; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; @@ -489,12 +493,12 @@ pub const MMAP2_UNIT = 4096; pub const VDSO_CGT_SYM = "__vdso_clock_gettime"; pub const VDSO_CGT_VER = "LINUX_2.6"; -pub const flock = extern struct { - lock_type: i16, - whence: i16, - start: off_t, - len: off_t, - pid: pid_t, +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, }; pub const msghdr = extern struct { diff --git a/lib/std/os/bits/linux/mipsel.zig b/lib/std/os/bits/linux/mipsel.zig index 638a4b1de7..b398d50760 100644 --- a/lib/std/os/bits/linux/mipsel.zig +++ b/lib/std/os/bits/linux/mipsel.zig @@ -5,6 +5,7 @@ const iovec = linux.iovec; const iovec_const = linux.iovec_const; const uid_t = linux.uid_t; const gid_t = linux.gid_t; +const pid_t = linux.pid_t; pub const SYS_Linux = 4000; pub const SYS_syscall = (SYS_Linux + 0); @@ -412,6 +413,10 @@ pub const F_GETLK = 33; pub const F_SETLK = 34; pub const F_SETLKW = 35; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; @@ -457,6 +462,16 @@ pub const SO_RCVBUFFORCE = 33; pub const VDSO_CGT_SYM = "__kernel_clock_gettime"; pub const VDSO_CGT_VER = "LINUX_2.6.39"; +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + __pad0: [4]u8, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + __unused: [4]u8, +}; + pub const blksize_t = i32; pub const nlink_t = u32; pub const time_t = isize; diff --git a/lib/std/os/bits/linux/riscv64.zig b/lib/std/os/bits/linux/riscv64.zig index e0c10ef7c0..4b95d2739a 100644 --- a/lib/std/os/bits/linux/riscv64.zig +++ b/lib/std/os/bits/linux/riscv64.zig @@ -2,6 +2,7 @@ const std = @import("../../../std.zig"); const uid_t = std.os.linux.uid_t; const gid_t = std.os.linux.gid_t; +const pid_t = std.os.linux.pid_t; pub const SYS_io_setup = 0; pub const SYS_io_destroy = 1; @@ -332,6 +333,10 @@ pub const F_GETOWN = 9; pub const F_SETSIG = 10; pub const F_GETSIG = 11; +pub const F_RDLCK = 0; +pub const F_WRLCK = 1; +pub const F_UNLCK = 2; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; @@ -350,6 +355,15 @@ pub const timespec = extern struct { tv_nsec: isize, }; +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + __unused: [4]u8, +}; + /// Renamed to Stat to not conflict with the stat function. /// atime, mtime, and ctime have functions to return `timespec`, /// because although this is a POSIX API, the layout and names of diff --git a/lib/std/os/bits/linux/x86_64.zig b/lib/std/os/bits/linux/x86_64.zig index 97905e4a3e..ab37391a6a 100644 --- a/lib/std/os/bits/linux/x86_64.zig +++ b/lib/std/os/bits/linux/x86_64.zig @@ -460,12 +460,12 @@ pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; -pub const flock = extern struct { - lock_type: i16, - whence: i16, - start: off_t, - len: off_t, - pid: pid_t, +pub const Flock = extern struct { + l_type: i16, + l_whence: i16, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, }; pub const msghdr = extern struct { diff --git a/lib/std/os/bits/netbsd.zig b/lib/std/os/bits/netbsd.zig index 735485695a..b67881088a 100644 --- a/lib/std/os/bits/netbsd.zig +++ b/lib/std/os/bits/netbsd.zig @@ -22,6 +22,14 @@ pub const dl_phdr_info = extern struct { dlpi_phnum: u16, }; +pub const Flock = extern struct { + start: off_t, + len: off_t, + pid: pid_t, + lock_type: i16, + whence: i16, +}; + pub const msghdr = extern struct { /// optional address msg_name: ?*sockaddr, @@ -312,6 +320,10 @@ pub const F_GETLK = 7; pub const F_SETLK = 8; pub const F_SETLKW = 9; +pub const F_RDLCK = 1; +pub const F_WRLCK = 3; +pub const F_UNLCK = 2; + pub const SEEK_SET = 0; pub const SEEK_CUR = 1; pub const SEEK_END = 2; From 4eff48b12ea90ddeb86f8f089f258ae2d615c7c4 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 9 Mar 2020 22:57:07 -0600 Subject: [PATCH 05/39] Add flock command paramter to `os.fcntlFlock` Also, replace `os.fcntlFlockBlocking` with `os.fcntlFlock` --- lib/std/fs.zig | 2 +- lib/std/os.zig | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index c508626628..44dbcf45fa 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -688,7 +688,7 @@ pub const Dir = struct { flock.l_start = 0; flock.l_len = 0; flock.l_pid = 0; - try os.fcntlFlockBlocking(fd, &flock); + try os.fcntlFlock(fd, .SetLockBlocking, &flock); locked = true; } diff --git a/lib/std/os.zig b/lib/std/os.zig index 3b1af4e9cd..280f4aa710 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1140,9 +1140,22 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) allocator.free(envp_buf); } +pub const LockCmd = enum { + GetLock, + SetLock, + SetLockBlocking, +}; + /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlockBlocking(fd: fd_t, flock_p: *const Flock) OpenError!void { - const rc = system.fcntl(fd, F_SETLKW, flock_p); +pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *const Flock) OpenError!void { + const cmd: i32 = cmdval: { + switch (lock_cmd) { + .GetLock => break :cmdval F_GETLK, + .SetLock => break :cmdval F_SETLK, + .SetLockBlocking => break :cmdval F_SETLKW, + } + }; + const rc = system.fcntl(fd, cmd, flock_p); if (rc < 0) { std.debug.panic("fcntl error: {}\n", .{rc}); } From a636b59cb58caf86dacd9814116bb459075b4cae Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 10 Mar 2020 18:54:05 -0600 Subject: [PATCH 06/39] Add `lock` option to CreateFlags --- lib/std/fs.zig | 20 +++++++++++++++++++- lib/std/fs/file.zig | 6 ++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 44dbcf45fa..a57f9e5c49 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -744,6 +744,19 @@ pub const Dir = struct { try std.event.Loop.instance.?.openatZ(self.fd, sub_path_c, os_flags, flags.mode) else try os.openatC(self.fd, sub_path_c, os_flags, flags.mode); + + if (flags.lock) { + // TODO: integrate async I/O + // mem.zeroes is used here because flock's structure can vary across architectures and systems + var flock = mem.zeroes(os.Flock); + flock.l_type = os.F_WRLCK; + flock.l_whence = os.SEEK_SET; + flock.l_start = 0; + flock.l_len = 0; + flock.l_pid = 0; + try os.fcntlFlock(fd, .SetLockBlocking, &flock); + } + return File{ .handle = fd, .io_mode = .blocking }; } @@ -759,7 +772,12 @@ pub const Dir = struct { @as(u32, w.FILE_OVERWRITE_IF) else @as(u32, w.FILE_OPEN_IF); - return self.openFileWindows(sub_path_w, access_mask, null, creation); + + const share_access = if (flags.lock) + @as(os.windows.ULONG, w.FILE_SHARE_DELETE) + else + null; + return self.openFileWindows(sub_path_w, access_mask, share_access, creation); } /// Deprecated; call `openFile` directly. diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index e892fc3b8f..ee12432496 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -69,6 +69,12 @@ pub const File = struct { /// `error.FileAlreadyExists` to be returned. exclusive: bool = false, + /// Prevent other files from accessing this file while this process has it is open. + /// + /// Note that the lock is only advisory on Linux. This means that a process that does not + /// respect the locking API can still read and write to the file, despite the lock. + lock: bool = false, + /// For POSIX systems this is the file system mode the file will /// be created with. mode: Mode = default_mode, From 43c4faba5516bed725cd058ced6f1a53e166c6af Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 12 Mar 2020 21:12:01 -0600 Subject: [PATCH 07/39] Add test to check that locking works --- lib/std/fs.zig | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index a57f9e5c49..9b45dc6652 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1720,3 +1720,69 @@ test "" { _ = @import("fs/get_app_data_dir.zig"); _ = @import("fs/watch.zig"); } + +const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; + +test "open file with lock twice, make sure it wasn't open at the same time" { + const filename = "file_lock_test.txt"; + + if (builtin.os.tag == .windows) { + var ctxs = [_]FileLockTestContext{ + .{ .filename = filename }, + .{ .filename = filename }, + }; + + const threads = [_]*std.Thread{ + try std.Thread.spawn(&ctxs[0], lock_file), + try std.Thread.spawn(&ctxs[1], lock_file), + }; + + for (threads[0..]) |thread| { + thread.wait(); + } + + std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); + } else { + const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); + defer std.os.munmap(shared_mem); + const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); + + const childpid = try std.os.fork(); + const ctx_idx: usize = if (childpid != 0) 0 else 1; + + ctxs[ctx_idx].filename = filename; + lock_file_for_test(&ctxs[ctx_idx]); + + if (childpid != 0) { + var status: u32 = 0; + _ = std.os.linux.waitpid(childpid, &status, 0); + + std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); + } + } + + cwd().deleteFile(filename) catch |err| switch (err) { + error.FileNotFound => {}, + else => return err, + }; +} + +const FileLockTestContext = struct { + filename: []const u8, + + // Output variables + start_time: u64 = 0, + end_time: u64 = 0, + + fn overlaps(self: *const @This(), other: *const @This()) bool { + return (self.start_time < other.end_time) and (self.end_time > other.start_time); + } +}; + +fn lock_file_for_test(ctx: *FileLockTestContext) void { + const file = cwd().createFile(ctx.filename, .{ .lock = true }) catch unreachable; + ctx.start_time = std.time.milliTimestamp(); + std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); + ctx.end_time = std.time.milliTimestamp(); + file.close(); +} From 43ccc2d81ec4c95c04dad9684f8b24633baab33e Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 12 Mar 2020 21:19:25 -0600 Subject: [PATCH 08/39] Add note about mandatory locks on linux --- lib/std/fs/file.zig | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index ee12432496..d34c6141d9 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -46,8 +46,11 @@ pub const File = struct { /// the file is opened with a shared lock, allowing the other processes to read from the /// file, but not to write to the file. /// - /// Note that the lock is only advisory on Linux. This means that a process that does not - /// respect the locking API can still read and write to the file, despite the lock. + /// Note that the lock is only advisory on Linux, except in very specific cirsumstances[1]. + /// This means that a process that does not respect the locking API can still read and write + /// to the file, despite the lock. + /// + /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt lock: bool = false, /// This prevents `O_NONBLOCK` from being passed even if `std.io.is_async`. @@ -71,8 +74,11 @@ pub const File = struct { /// Prevent other files from accessing this file while this process has it is open. /// - /// Note that the lock is only advisory on Linux. This means that a process that does not - /// respect the locking API can still read and write to the file, despite the lock. + /// Note that the lock is only advisory on Linux, except in very specific cirsumstances[1]. + /// This means that a process that does not respect the locking API can still read and write + /// to the file, despite the lock. + /// + /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt lock: bool = false, /// For POSIX systems this is the file system mode the file will From 72eb9933fd9ab4a7cb19185fce6c06331a325327 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 11:34:38 -0600 Subject: [PATCH 09/39] Call `std.os.waitpid` instead of `std.os.linux.waitpid` --- lib/std/fs.zig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 9b45dc6652..65e20d0176 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1754,8 +1754,7 @@ test "open file with lock twice, make sure it wasn't open at the same time" { lock_file_for_test(&ctxs[ctx_idx]); if (childpid != 0) { - var status: u32 = 0; - _ = std.os.linux.waitpid(childpid, &status, 0); + _ = std.os.waitpid(childpid, 0); std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); } From 873c7a59e9943a944cf157345e9e27275b9069ba Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 13:29:49 -0600 Subject: [PATCH 10/39] Add multiple read lock test --- lib/std/fs.zig | 72 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 65e20d0176..1dd404afe1 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1733,8 +1733,8 @@ test "open file with lock twice, make sure it wasn't open at the same time" { }; const threads = [_]*std.Thread{ - try std.Thread.spawn(&ctxs[0], lock_file), - try std.Thread.spawn(&ctxs[1], lock_file), + try std.Thread.spawn(&ctxs[0], lock_file_for_test), + try std.Thread.spawn(&ctxs[1], lock_file_for_test), }; for (threads[0..]) |thread| { @@ -1772,6 +1772,7 @@ const FileLockTestContext = struct { // Output variables start_time: u64 = 0, end_time: u64 = 0, + bytes_read: ?usize = null, fn overlaps(self: *const @This(), other: *const @This()) bool { return (self.start_time < other.end_time) and (self.end_time > other.start_time); @@ -1785,3 +1786,70 @@ fn lock_file_for_test(ctx: *FileLockTestContext) void { ctx.end_time = std.time.milliTimestamp(); file.close(); } + +test "create file, lock and read from multiple process at once" { + const filename = "file_read_lock_test.txt"; + const filedata = "Hello, world!\n"; + + try std.fs.cwd().writeFile(filename, filedata); + + if (builtin.os.tag == .windows) { + var ctxs = [_]FileLockTestContext{ + .{ .filename = filename }, + .{ .filename = filename }, + }; + + const threads = [_]*std.Thread{ + try std.Thread.spawn(&ctxs[0], lock_file_for_read_test), + try std.Thread.spawn(&ctxs[1], lock_file_for_read_test), + }; + + for (threads[0..]) |thread| { + thread.wait(); + } + + std.debug.assert(ctxs[0].overlaps(&ctxs[1])); + std.debug.assert(ctxs[0].bytes_read.? == filedata.len); + std.debug.assert(ctxs[1].bytes_read.? == filedata.len); + } else { + const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); + defer std.os.munmap(shared_mem); + const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); + + const childpid = try std.os.fork(); + const ctx_idx: usize = if (childpid != 0) 0 else 1; + + ctxs[ctx_idx].filename = filename; + lock_file_for_read_test(&ctxs[ctx_idx]); + + if (childpid != 0) { + _ = std.os.waitpid(childpid, 0); + + std.debug.assert(ctxs[0].overlaps(&ctxs[1])); + std.debug.assert(ctxs[0].bytes_read.? == filedata.len); + std.debug.assert(ctxs[1].bytes_read.? == filedata.len); + } + } + + cwd().deleteFile(filename) catch |err| switch (err) { + error.FileNotFound => {}, + else => return err, + }; +} + +fn lock_file_for_read_test(ctx: *FileLockTestContext) void { + const file = cwd().openFile(ctx.filename, .{ .lock = true }) catch unreachable; + ctx.start_time = std.time.milliTimestamp(); + + var buffer: [100]u8 = undefined; + ctx.bytes_read = 0; + while (true) { + const amt = file.read(buffer[0..]) catch unreachable; + if (amt == 0) break; + ctx.bytes_read.? += amt; + } + std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); + + ctx.end_time = std.time.milliTimestamp(); + file.close(); +} From 7ab77685a8eb69293fc0229f7b01a6a67332740a Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 14:57:56 -0600 Subject: [PATCH 11/39] Make lock tests more flexible --- lib/std/fs.zig | 232 +++++++++++++++++++++++++++++-------------------- 1 file changed, 136 insertions(+), 96 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 1dd404afe1..aeac704aae 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1723,51 +1723,59 @@ test "" { const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; -test "open file with lock twice, make sure it wasn't open at the same time" { - const filename = "file_lock_test.txt"; - - if (builtin.os.tag == .windows) { - var ctxs = [_]FileLockTestContext{ - .{ .filename = filename }, - .{ .filename = filename }, - }; - - const threads = [_]*std.Thread{ - try std.Thread.spawn(&ctxs[0], lock_file_for_test), - try std.Thread.spawn(&ctxs[1], lock_file_for_test), - }; - - for (threads[0..]) |thread| { - thread.wait(); - } - - std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); - } else { - const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); - defer std.os.munmap(shared_mem); - const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); - - const childpid = try std.os.fork(); - const ctx_idx: usize = if (childpid != 0) 0 else 1; - - ctxs[ctx_idx].filename = filename; - lock_file_for_test(&ctxs[ctx_idx]); - - if (childpid != 0) { - _ = std.os.waitpid(childpid, 0); - - std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); - } - } - - cwd().deleteFile(filename) catch |err| switch (err) { - error.FileNotFound => {}, - else => return err, - }; -} +//test "open file with lock twice, make sure it wasn't open at the same time" { +// const filename = "file_lock_test.txt"; +// +// if (builtin.os.tag == .windows) { +// var ctxs = [_]FileLockTestContext{ +// .{ .filename = filename }, +// .{ .filename = filename }, +// }; +// +// const threads = [_]*std.Thread{ +// try std.Thread.spawn(&ctxs[0], lock_file_for_test), +// try std.Thread.spawn(&ctxs[1], lock_file_for_test), +// }; +// +// for (threads[0..]) |thread| { +// thread.wait(); +// } +// +// std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); +// } else { +// const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); +// defer std.os.munmap(shared_mem); +// const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); +// +// const childpid = try std.os.fork(); +// const ctx_idx: usize = if (childpid != 0) 0 else 1; +// +// ctxs[ctx_idx].filename = filename; +// lock_file_for_test(&ctxs[ctx_idx]); +// +// if (childpid != 0) { +// _ = std.os.waitpid(childpid, 0); +// +// std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); +// } else { +// std.os.exit(0); +// } +// } +// +// cwd().deleteFile(filename) catch |err| switch (err) { +// error.FileNotFound => {}, +// else => return err, +// }; +//} const FileLockTestContext = struct { filename: []const u8, + pid: if (builtin.os.tag == .windows) ?void else ?std.os.pid_t = null, + + // use file.createFile + create: bool, + // get a read/write lock, instead of just a read lock + exclusive: bool, // Output variables start_time: u64 = 0, @@ -1777,15 +1785,30 @@ const FileLockTestContext = struct { fn overlaps(self: *const @This(), other: *const @This()) bool { return (self.start_time < other.end_time) and (self.end_time > other.start_time); } -}; -fn lock_file_for_test(ctx: *FileLockTestContext) void { - const file = cwd().createFile(ctx.filename, .{ .lock = true }) catch unreachable; - ctx.start_time = std.time.milliTimestamp(); - std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); - ctx.end_time = std.time.milliTimestamp(); - file.close(); -} + fn run(ctx: *@This()) void { + var file: File = undefined; + if (ctx.create) { + file = cwd().createFile(ctx.filename, .{ .lock = true }) catch unreachable; + } else { + file = cwd().openFile(ctx.filename, .{ .lock = true, .write = ctx.exclusive }) catch unreachable; + } + + ctx.start_time = std.time.milliTimestamp(); + + var buffer: [100]u8 = undefined; + ctx.bytes_read = 0; + while (true) { + const amt = file.read(buffer[0..]) catch unreachable; + if (amt == 0) break; + ctx.bytes_read.? += amt; + } + std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); + + ctx.end_time = std.time.milliTimestamp(); + file.close(); + } +}; test "create file, lock and read from multiple process at once" { const filename = "file_read_lock_test.txt"; @@ -1793,42 +1816,76 @@ test "create file, lock and read from multiple process at once" { try std.fs.cwd().writeFile(filename, filedata); - if (builtin.os.tag == .windows) { - var ctxs = [_]FileLockTestContext{ - .{ .filename = filename }, - .{ .filename = filename }, - }; + const NUM_PROCESSES = 3; + var shared_mem: if (builtin.os.tag == .windows) [NUM_PROCESSES]FileLockTestContext else []align(mem.page_size) u8 = undefined; - const threads = [_]*std.Thread{ - try std.Thread.spawn(&ctxs[0], lock_file_for_read_test), - try std.Thread.spawn(&ctxs[1], lock_file_for_read_test), - }; + var ctxs: []FileLockTestContext = undefined; + if (builtin.os.tag == .windows) { + ctxs = shared_mem[0..NUM_PROCESSES]; + } else { + shared_mem = try std.os.mmap(null, NUM_PROCESSES * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); + const ctxs_ptr = @ptrCast([*]FileLockTestContext, shared_mem.ptr); + ctxs = ctxs_ptr[0..NUM_PROCESSES]; + } + + ctxs[0] = .{ + .filename = filename, + .create = false, + .exclusive = false, + }; + ctxs[1] = .{ + .filename = filename, + .create = false, + .exclusive = false, + }; + ctxs[2] = .{ + .filename = filename, + .create = false, + .exclusive = true, + }; + + if (builtin.os.tag == .windows) { + const threads: [NUM_PROCESSES]*std.Thread = undefined; + for (ctxs) |*ctx, idx| { + threads[idx] = try std.Thread.spawn(ctx, Context.run); + } for (threads[0..]) |thread| { thread.wait(); } - - std.debug.assert(ctxs[0].overlaps(&ctxs[1])); - std.debug.assert(ctxs[0].bytes_read.? == filedata.len); - std.debug.assert(ctxs[1].bytes_read.? == filedata.len); } else { - const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); - defer std.os.munmap(shared_mem); - const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); - - const childpid = try std.os.fork(); - const ctx_idx: usize = if (childpid != 0) 0 else 1; - - ctxs[ctx_idx].filename = filename; - lock_file_for_read_test(&ctxs[ctx_idx]); - - if (childpid != 0) { - _ = std.os.waitpid(childpid, 0); - - std.debug.assert(ctxs[0].overlaps(&ctxs[1])); - std.debug.assert(ctxs[0].bytes_read.? == filedata.len); - std.debug.assert(ctxs[1].bytes_read.? == filedata.len); + var ctx_opt: ?*FileLockTestContext = null; + for (ctxs) |*ctx| { + const childpid = try std.os.fork(); + if (childpid == 0) { + ctx_opt = ctx; + break; + } + ctx.pid = childpid; } + + if (ctx_opt) |ctx| { + ctx.run(); + // Exit so we don't have duplicate test processes + std.os.exit(0); + } else { + for (ctxs) |ctx| { + _ = std.os.waitpid(ctx.pid.?, 0); + } + } + } + + std.debug.assert(ctxs[0].overlaps(&ctxs[1])); + std.debug.assert(!ctxs[2].overlaps(&ctxs[0])); + std.debug.assert(!ctxs[2].overlaps(&ctxs[1])); + if (ctxs[0].bytes_read.? != filedata.len) { + std.debug.warn("\n bytes_read: {}, expected: {} \n", .{ ctxs[0].bytes_read, filedata.len }); + } + std.debug.assert(ctxs[0].bytes_read.? == filedata.len); + std.debug.assert(ctxs[1].bytes_read.? == filedata.len); + + if (builtin.os.tag != .windows) { + std.os.munmap(shared_mem); } cwd().deleteFile(filename) catch |err| switch (err) { @@ -1836,20 +1893,3 @@ test "create file, lock and read from multiple process at once" { else => return err, }; } - -fn lock_file_for_read_test(ctx: *FileLockTestContext) void { - const file = cwd().openFile(ctx.filename, .{ .lock = true }) catch unreachable; - ctx.start_time = std.time.milliTimestamp(); - - var buffer: [100]u8 = undefined; - ctx.bytes_read = 0; - while (true) { - const amt = file.read(buffer[0..]) catch unreachable; - if (amt == 0) break; - ctx.bytes_read.? += amt; - } - std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); - - ctx.end_time = std.time.milliTimestamp(); - file.close(); -} From 49128c86f8eb6cc42f55b6a7a9d0aa66d222b1a6 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 15:31:54 -0600 Subject: [PATCH 12/39] Extract `run_lock_file_test` --- lib/std/fs.zig | 207 +++++++++++++++++++++++++------------------------ 1 file changed, 107 insertions(+), 100 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index aeac704aae..d5b7c72e7d 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1723,50 +1723,70 @@ test "" { const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; -//test "open file with lock twice, make sure it wasn't open at the same time" { -// const filename = "file_lock_test.txt"; -// -// if (builtin.os.tag == .windows) { -// var ctxs = [_]FileLockTestContext{ -// .{ .filename = filename }, -// .{ .filename = filename }, -// }; -// -// const threads = [_]*std.Thread{ -// try std.Thread.spawn(&ctxs[0], lock_file_for_test), -// try std.Thread.spawn(&ctxs[1], lock_file_for_test), -// }; -// -// for (threads[0..]) |thread| { -// thread.wait(); -// } -// -// std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); -// } else { -// const shared_mem = try std.os.mmap(null, 2 * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); -// defer std.os.munmap(shared_mem); -// const ctxs = @ptrCast([*]FileLockTestContext, shared_mem.ptr); -// -// const childpid = try std.os.fork(); -// const ctx_idx: usize = if (childpid != 0) 0 else 1; -// -// ctxs[ctx_idx].filename = filename; -// lock_file_for_test(&ctxs[ctx_idx]); -// -// if (childpid != 0) { -// _ = std.os.waitpid(childpid, 0); -// -// std.debug.assert(!ctxs[0].overlaps(&ctxs[1])); -// } else { -// std.os.exit(0); -// } -// } -// -// cwd().deleteFile(filename) catch |err| switch (err) { -// error.FileNotFound => {}, -// else => return err, -// }; -//} +test "open file with lock twice, make sure it wasn't open at the same time" { + const filename = "file_lock_test.txt"; + + var contexts = [_]FileLockTestContext{ + .{ .filename = filename, .create = true, .exclusive = true }, + .{ .filename = filename, .create = true, .exclusive = true }, + }; + try run_lock_file_test(&contexts); + + // Check for an error + var was_error = false; + for (contexts) |context, idx| { + if (context.err) |err| { + was_error = true; + std.debug.warn("\nError in context {}: {}\n", .{ idx, err }); + } + } + if (was_error) builtin.panic("There was an error in contexts", null); + + std.debug.assert(!contexts[0].overlaps(&contexts[1])); + + cwd().deleteFile(filename) catch |err| switch (err) { + error.FileNotFound => {}, + else => return err, + }; +} + +test "create file, lock and read from multiple process at once" { + const filename = "file_read_lock_test.txt"; + const filedata = "Hello, world!\n"; + + try std.fs.cwd().writeFile(filename, filedata); + + var contexts = [_]FileLockTestContext{ + .{ .filename = filename, .create = false, .exclusive = false }, + .{ .filename = filename, .create = false, .exclusive = false }, + .{ .filename = filename, .create = false, .exclusive = true }, + }; + + try run_lock_file_test(&contexts); + + var was_error = false; + for (contexts) |context, idx| { + if (context.err) |err| { + was_error = true; + std.debug.warn("\nError in context {}: {}\n", .{ idx, err }); + } + } + if (was_error) builtin.panic("There was an error in contexts", null); + + std.debug.assert(contexts[0].overlaps(&contexts[1])); + std.debug.assert(!contexts[2].overlaps(&contexts[0])); + std.debug.assert(!contexts[2].overlaps(&contexts[1])); + if (contexts[0].bytes_read.? != filedata.len) { + std.debug.warn("\n bytes_read: {}, expected: {} \n", .{ contexts[0].bytes_read, filedata.len }); + } + std.debug.assert(contexts[0].bytes_read.? == filedata.len); + std.debug.assert(contexts[1].bytes_read.? == filedata.len); + + cwd().deleteFile(filename) catch |err| switch (err) { + error.FileNotFound => {}, + else => return err, + }; +} const FileLockTestContext = struct { filename: []const u8, @@ -1778,6 +1798,7 @@ const FileLockTestContext = struct { exclusive: bool, // Output variables + err: ?(File.OpenError || std.os.ReadError) = null, start_time: u64 = 0, end_time: u64 = 0, bytes_read: ?usize = null, @@ -1789,69 +1810,65 @@ const FileLockTestContext = struct { fn run(ctx: *@This()) void { var file: File = undefined; if (ctx.create) { - file = cwd().createFile(ctx.filename, .{ .lock = true }) catch unreachable; + file = cwd().createFile(ctx.filename, .{ .lock = true }) catch |err| { + ctx.err = err; + return; + }; } else { - file = cwd().openFile(ctx.filename, .{ .lock = true, .write = ctx.exclusive }) catch unreachable; + file = cwd().openFile(ctx.filename, .{ .lock = true, .write = ctx.exclusive }) catch |err| { + ctx.err = err; + return; + }; } + defer file.close(); ctx.start_time = std.time.milliTimestamp(); - var buffer: [100]u8 = undefined; - ctx.bytes_read = 0; - while (true) { - const amt = file.read(buffer[0..]) catch unreachable; - if (amt == 0) break; - ctx.bytes_read.? += amt; + if (!ctx.create) { + var buffer: [100]u8 = undefined; + ctx.bytes_read = 0; + while (true) { + const amt = file.read(buffer[0..]) catch |err| { + ctx.err = err; + return; + }; + if (amt == 0) break; + ctx.bytes_read.? += amt; + } } + std.time.sleep(FILE_LOCK_TEST_SLEEP_TIME); ctx.end_time = std.time.milliTimestamp(); - file.close(); } }; -test "create file, lock and read from multiple process at once" { - const filename = "file_read_lock_test.txt"; - const filedata = "Hello, world!\n"; - - try std.fs.cwd().writeFile(filename, filedata); - - const NUM_PROCESSES = 3; - var shared_mem: if (builtin.os.tag == .windows) [NUM_PROCESSES]FileLockTestContext else []align(mem.page_size) u8 = undefined; +fn run_lock_file_test(contexts: []FileLockTestContext) !void { + var shared_mem: if (builtin.os.tag == .windows) void else []align(mem.page_size) u8 = undefined; var ctxs: []FileLockTestContext = undefined; if (builtin.os.tag == .windows) { - ctxs = shared_mem[0..NUM_PROCESSES]; + ctxs = contexts; } else { - shared_mem = try std.os.mmap(null, NUM_PROCESSES * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); + shared_mem = try std.os.mmap(null, contexts.len * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); const ctxs_ptr = @ptrCast([*]FileLockTestContext, shared_mem.ptr); - ctxs = ctxs_ptr[0..NUM_PROCESSES]; + ctxs = ctxs_ptr[0..contexts.len]; + + for (contexts) |context, idx| { + ctxs[idx] = context; + } } - ctxs[0] = .{ - .filename = filename, - .create = false, - .exclusive = false, - }; - ctxs[1] = .{ - .filename = filename, - .create = false, - .exclusive = false, - }; - ctxs[2] = .{ - .filename = filename, - .create = false, - .exclusive = true, - }; - if (builtin.os.tag == .windows) { - const threads: [NUM_PROCESSES]*std.Thread = undefined; - for (ctxs) |*ctx, idx| { - threads[idx] = try std.Thread.spawn(ctx, Context.run); + const threads = std.ArrayList(*std.Thread).init(testing.allocator); + defer { + for (threads.toSlice()) |thread| { + thread.wait(); + } + threads.deinit(); } - - for (threads[0..]) |thread| { - thread.wait(); + for (ctxs) |*ctx, idx| { + threads.append(try std.Thread.spawn(ctx, Context.run)); } } else { var ctx_opt: ?*FileLockTestContext = null; @@ -1875,21 +1892,11 @@ test "create file, lock and read from multiple process at once" { } } - std.debug.assert(ctxs[0].overlaps(&ctxs[1])); - std.debug.assert(!ctxs[2].overlaps(&ctxs[0])); - std.debug.assert(!ctxs[2].overlaps(&ctxs[1])); - if (ctxs[0].bytes_read.? != filedata.len) { - std.debug.warn("\n bytes_read: {}, expected: {} \n", .{ ctxs[0].bytes_read, filedata.len }); - } - std.debug.assert(ctxs[0].bytes_read.? == filedata.len); - std.debug.assert(ctxs[1].bytes_read.? == filedata.len); - if (builtin.os.tag != .windows) { + // Copy contexts out of shared memory + for (ctxs) |ctx, idx| { + contexts[idx] = ctx; + } std.os.munmap(shared_mem); } - - cwd().deleteFile(filename) catch |err| switch (err) { - error.FileNotFound => {}, - else => return err, - }; } From 947abb7626cae990c407d347f62a625001c17cdf Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 17:13:46 -0600 Subject: [PATCH 13/39] Fix compile error on windows --- lib/std/fs.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index d5b7c72e7d..d324ad6e3c 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1860,7 +1860,7 @@ fn run_lock_file_test(contexts: []FileLockTestContext) !void { } if (builtin.os.tag == .windows) { - const threads = std.ArrayList(*std.Thread).init(testing.allocator); + var threads = std.ArrayList(*std.Thread).init(std.testing.allocator); defer { for (threads.toSlice()) |thread| { thread.wait(); @@ -1868,7 +1868,7 @@ fn run_lock_file_test(contexts: []FileLockTestContext) !void { threads.deinit(); } for (ctxs) |*ctx, idx| { - threads.append(try std.Thread.spawn(ctx, Context.run)); + try threads.append(try std.Thread.spawn(ctx, FileLockTestContext.run)); } } else { var ctx_opt: ?*FileLockTestContext = null; From 819537d70a3283737474e1b673fc20f574ac6bb6 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Sat, 14 Mar 2020 20:36:26 -0600 Subject: [PATCH 14/39] Skip file lock test in single threaded mode --- lib/std/fs.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index d324ad6e3c..3c7d687bf7 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1724,6 +1724,8 @@ test "" { const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; test "open file with lock twice, make sure it wasn't open at the same time" { + if (builtin.single_threaded) return; + const filename = "file_lock_test.txt"; var contexts = [_]FileLockTestContext{ @@ -1751,6 +1753,8 @@ test "open file with lock twice, make sure it wasn't open at the same time" { } test "create file, lock and read from multiple process at once" { + if (builtin.single_threaded) return; + const filename = "file_read_lock_test.txt"; const filedata = "Hello, world!\n"; From 4532f5ecad23a3478310f159f43d31217c863c35 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 16 Mar 2020 21:50:52 -0600 Subject: [PATCH 15/39] Change fcntl params to ?*c_void As recommended by LemonBoy --- lib/std/os.zig | 2 +- lib/std/os/linux.zig | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index 280f4aa710..832e2901af 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1147,7 +1147,7 @@ pub const LockCmd = enum { }; /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *const Flock) OpenError!void { +pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *Flock) OpenError!void { const cmd: i32 = cmdval: { switch (lock_cmd) { .GetLock => break :cmdval F_GETLK, diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 4b29a1cc27..f47b0b8e3a 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -219,8 +219,8 @@ pub fn mmap(address: ?[*]u8, length: usize, prot: usize, flags: u32, fd: i32, of } } -pub fn fcntl(fd: fd_t, cmd: i32, flock_p: *const c_void) usize { - return syscall3(SYS_fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), @ptrToInt(flock_p)); +pub fn fcntl(fd: fd_t, cmd: i32, arg: ?*c_void) usize { + return syscall3(SYS_fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), @ptrToInt(arg)); } pub fn mprotect(address: [*]const u8, length: usize, protection: usize) usize { From b773a8b175b6be130af2249f267e4113fdbf30ea Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 17 Mar 2020 20:53:43 -0600 Subject: [PATCH 16/39] Make `fcntlFlock` follow conventions of `os.zig` --- lib/std/fs.zig | 2 +- lib/std/fs/file.zig | 2 +- lib/std/os.zig | 28 ++++++++++++++++++---------- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 3c7d687bf7..dd25b29042 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1605,7 +1605,7 @@ pub fn readLinkC(pathname_c: [*]const u8, buffer: *[MAX_PATH_BYTES]u8) ![]u8 { return os.readlinkC(pathname_c, buffer); } -pub const OpenSelfExeError = os.OpenError || os.windows.CreateFileError || SelfExePathError; +pub const OpenSelfExeError = os.OpenError || os.windows.CreateFileError || SelfExePathError || os.FcntlError; pub fn openSelfExe() OpenSelfExeError!File { if (builtin.os.tag == .linux) { diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index d34c6141d9..454f1e503c 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -34,7 +34,7 @@ pub const File = struct { else => 0o666, }; - pub const OpenError = windows.CreateFileError || os.OpenError; + pub const OpenError = windows.CreateFileError || os.OpenError || os.FcntlError; /// TODO https://github.com/ziglang/zig/issues/3802 pub const OpenFlags = struct { diff --git a/lib/std/os.zig b/lib/std/os.zig index 832e2901af..2741d2d8f3 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1146,18 +1146,26 @@ pub const LockCmd = enum { SetLockBlocking, }; +pub const FcntlError = error{ + /// The file is locked by another process + FileLocked, +} || UnexpectedError; + /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *Flock) OpenError!void { - const cmd: i32 = cmdval: { - switch (lock_cmd) { - .GetLock => break :cmdval F_GETLK, - .SetLock => break :cmdval F_SETLK, - .SetLockBlocking => break :cmdval F_SETLKW, - } +pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *Flock) FcntlError!void { + const cmd: i32 = switch (lock_cmd) { + .GetLock => F_GETLK, + .SetLock => F_SETLK, + .SetLockBlocking => F_SETLKW, }; - const rc = system.fcntl(fd, cmd, flock_p); - if (rc < 0) { - std.debug.panic("fcntl error: {}\n", .{rc}); + while (true) { + switch (errno(system.fcntl(fd, cmd, flock_p))) { + 0 => return, + EACCES => return error.FileLocked, + EAGAIN => return error.FileLocked, + EINTR => continue, + else => |err| return unexpectedErrno(err), + } } } From 32c58250305540bf33b109f832956777e4bef73c Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 17 Mar 2020 20:54:06 -0600 Subject: [PATCH 17/39] Match netbsd's flock fields with others --- lib/std/os/bits/netbsd.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/std/os/bits/netbsd.zig b/lib/std/os/bits/netbsd.zig index b67881088a..fba38e002d 100644 --- a/lib/std/os/bits/netbsd.zig +++ b/lib/std/os/bits/netbsd.zig @@ -23,11 +23,11 @@ pub const dl_phdr_info = extern struct { }; pub const Flock = extern struct { - start: off_t, - len: off_t, - pid: pid_t, - lock_type: i16, - whence: i16, + l_start: off_t, + l_len: off_t, + l_pid: pid_t, + l_type: i16, + l_whence: i16, }; pub const msghdr = extern struct { From 613956cc47c2513cdd27d57e5eb2fa36a368c21b Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 17 Mar 2020 21:02:19 -0600 Subject: [PATCH 18/39] Remove `fcntlFlock` and replace with plain `fcntl` --- lib/std/fs.zig | 4 ++-- lib/std/os.zig | 13 +------------ 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index dd25b29042..7452dd0e0e 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -688,7 +688,7 @@ pub const Dir = struct { flock.l_start = 0; flock.l_len = 0; flock.l_pid = 0; - try os.fcntlFlock(fd, .SetLockBlocking, &flock); + try os.fcntl(fd, os.F_SETLKW, &flock); locked = true; } @@ -754,7 +754,7 @@ pub const Dir = struct { flock.l_start = 0; flock.l_len = 0; flock.l_pid = 0; - try os.fcntlFlock(fd, .SetLockBlocking, &flock); + try os.fcntl(fd, os.F_SETLKW, &flock); } return File{ .handle = fd, .io_mode = .blocking }; diff --git a/lib/std/os.zig b/lib/std/os.zig index 2741d2d8f3..f6fdb9482f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1140,24 +1140,13 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) allocator.free(envp_buf); } -pub const LockCmd = enum { - GetLock, - SetLock, - SetLockBlocking, -}; - pub const FcntlError = error{ /// The file is locked by another process FileLocked, } || UnexpectedError; /// Attempts to get lock the file, blocking if the file is locked. -pub fn fcntlFlock(fd: fd_t, lock_cmd: LockCmd, flock_p: *Flock) FcntlError!void { - const cmd: i32 = switch (lock_cmd) { - .GetLock => F_GETLK, - .SetLock => F_SETLK, - .SetLockBlocking => F_SETLKW, - }; +pub fn fcntl(fd: fd_t, cmd: i32, flock_p: *Flock) FcntlError!void { while (true) { switch (errno(system.fcntl(fd, cmd, flock_p))) { 0 => return, From 5b278fb60615751bef5e521a613e8798d783dd26 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 23 Mar 2020 20:59:09 -0600 Subject: [PATCH 19/39] Use locking open flags if they are defined --- lib/std/fs.zig | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 7452dd0e0e..80d679aed5 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -666,8 +666,19 @@ pub const Dir = struct { const path_w = try os.windows.cStrToPrefixedFileW(sub_path); return self.openFileW(&path_w, flags); } + + // Use the O_ locking flags if the os supports them + const lock_flag: u32 = lock_flag: { + if (!flags.lock) break :lock_flag 0; + if (flags.write) { + break :lock_flag if (@hasDecl(os, "O_EXLOCK")) os.O_EXLOCK else 0; + } else { + break :lock_flag if (@hasDecl(os, "O_SHLOCK")) os.O_SHLOCK else 0; + } + }; + const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; - const os_flags = O_LARGEFILE | os.O_CLOEXEC | if (flags.write and flags.read) + const os_flags = lock_flag | O_LARGEFILE | os.O_CLOEXEC | if (flags.write and flags.read) @as(u32, os.O_RDWR) else if (flags.write) @as(u32, os.O_WRONLY) @@ -678,18 +689,14 @@ pub const Dir = struct { else try os.openatC(self.fd, sub_path, os_flags, 0); - var locked = false; - if (flags.lock) { + // use fcntl file locking if no lock flag was given + if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O // mem.zeroes is used here because flock's structure can vary across architectures and systems var flock = mem.zeroes(os.Flock); flock.l_type = if (flags.write) os.F_WRLCK else os.F_RDLCK; flock.l_whence = os.SEEK_SET; - flock.l_start = 0; - flock.l_len = 0; - flock.l_pid = 0; try os.fcntl(fd, os.F_SETLKW, &flock); - locked = true; } return File{ @@ -735,8 +742,15 @@ pub const Dir = struct { const path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); return self.createFileW(&path_w, flags); } + + // Use the O_ locking flags if the os supports them + const lock_flag: u32 = lock_flag: { + if (!flags.lock) break :lock_flag 0; + break :lock_flag if (@hasDecl(os, "O_EXLOCK")) os.O_EXLOCK else 0; + }; + const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; - const os_flags = O_LARGEFILE | os.O_CREAT | os.O_CLOEXEC | + const os_flags = lock_flag | O_LARGEFILE | os.O_CREAT | os.O_CLOEXEC | (if (flags.truncate) @as(u32, os.O_TRUNC) else 0) | (if (flags.read) @as(u32, os.O_RDWR) else os.O_WRONLY) | (if (flags.exclusive) @as(u32, os.O_EXCL) else 0); @@ -745,15 +759,12 @@ pub const Dir = struct { else try os.openatC(self.fd, sub_path_c, os_flags, flags.mode); - if (flags.lock) { + if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O // mem.zeroes is used here because flock's structure can vary across architectures and systems var flock = mem.zeroes(os.Flock); flock.l_type = os.F_WRLCK; flock.l_whence = os.SEEK_SET; - flock.l_start = 0; - flock.l_len = 0; - flock.l_pid = 0; try os.fcntl(fd, os.F_SETLKW, &flock); } From 0b93932a2103b178d3ab5235c837df14173ed38c Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 23 Mar 2020 21:07:50 -0600 Subject: [PATCH 20/39] Add O_SHLOCK and O_EXLOCK to freebsd and netbsd --- lib/std/os/bits/freebsd.zig | 3 +++ lib/std/os/bits/netbsd.zig | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/std/os/bits/freebsd.zig b/lib/std/os/bits/freebsd.zig index 2648e4a98f..f0490ee23a 100644 --- a/lib/std/os/bits/freebsd.zig +++ b/lib/std/os/bits/freebsd.zig @@ -325,6 +325,9 @@ pub const O_WRONLY = 0x0001; pub const O_RDWR = 0x0002; pub const O_ACCMODE = 0x0003; +pub const O_SHLOCK = 0x0010; +pub const O_EXLOCK = 0x0020; + pub const O_CREAT = 0x0200; pub const O_EXCL = 0x0800; pub const O_NOCTTY = 0x8000; diff --git a/lib/std/os/bits/netbsd.zig b/lib/std/os/bits/netbsd.zig index fba38e002d..4599baddff 100644 --- a/lib/std/os/bits/netbsd.zig +++ b/lib/std/os/bits/netbsd.zig @@ -287,6 +287,9 @@ pub const O_WRONLY = 0x0001; pub const O_RDWR = 0x0002; pub const O_ACCMODE = 0x0003; +pub const O_SHLOCK = 0x0010; +pub const O_EXLOCK = 0x0020; + pub const O_CREAT = 0x0200; pub const O_EXCL = 0x0800; pub const O_NOCTTY = 0x8000; From 8bec1304c35585ceacfbda3696d8af854c7ab08a Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 23 Mar 2020 22:34:00 -0600 Subject: [PATCH 21/39] Fix compile error on windows --- lib/std/os.zig | 2 +- lib/std/os/windows.zig | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index 2dec2c4c1d..846943b050 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1651,7 +1651,7 @@ pub fn renameatW( ReplaceIfExists: windows.BOOLEAN, ) RenameError!void { const access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE; - const src_fd = try windows.OpenFileW(old_dir_fd, old_path, null, access_mask, windows.FILE_OPEN); + const src_fd = try windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, windows.FILE_OPEN); defer windows.CloseHandle(src_fd); const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION) + (MAX_PATH_BYTES - 1); diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 813a77c275..a2082b2deb 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -107,6 +107,7 @@ pub fn OpenFileW( sub_path_w: [*:0]const u16, sa: ?*SECURITY_ATTRIBUTES, access_mask: ACCESS_MASK, + share_access_opt: ?ULONG, creation: ULONG, ) OpenError!HANDLE { if (sub_path_w[0] == '.' and sub_path_w[1] == 0) { @@ -135,6 +136,7 @@ pub fn OpenFileW( .SecurityQualityOfService = null, }; var io: IO_STATUS_BLOCK = undefined; + const share_access = share_access_opt orelse FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; const rc = ntdll.NtCreateFile( &result, access_mask, @@ -142,7 +144,7 @@ pub fn OpenFileW( &io, null, FILE_ATTRIBUTE_NORMAL, - FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, + share_access, creation, FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, null, From 1a6c3aeec9226eba02655c77872bd01eaa9be711 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 23 Mar 2020 23:20:17 -0600 Subject: [PATCH 22/39] Block until file is unlocked on windows --- lib/std/os/windows.zig | 62 ++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index a2082b2deb..3ccd861444 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -137,32 +137,42 @@ pub fn OpenFileW( }; var io: IO_STATUS_BLOCK = undefined; const share_access = share_access_opt orelse FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; - const rc = ntdll.NtCreateFile( - &result, - access_mask, - &attr, - &io, - null, - FILE_ATTRIBUTE_NORMAL, - share_access, - creation, - FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, - null, - 0, - ); - switch (rc) { - .SUCCESS => return result, - .OBJECT_NAME_INVALID => unreachable, - .OBJECT_NAME_NOT_FOUND => return error.FileNotFound, - .OBJECT_PATH_NOT_FOUND => return error.FileNotFound, - .NO_MEDIA_IN_DEVICE => return error.NoDevice, - .INVALID_PARAMETER => unreachable, - .SHARING_VIOLATION => return error.SharingViolation, - .ACCESS_DENIED => return error.AccessDenied, - .PIPE_BUSY => return error.PipeBusy, - .OBJECT_PATH_SYNTAX_BAD => unreachable, - .OBJECT_NAME_COLLISION => return error.PathAlreadyExists, - else => return unexpectedStatus(rc), + + var delay: usize = 1; + while (true) { + const rc = ntdll.NtCreateFile( + &result, + access_mask, + &attr, + &io, + null, + FILE_ATTRIBUTE_NORMAL, + share_access, + creation, + FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, + null, + 0, + ); + switch (rc) { + .SUCCESS => return result, + .OBJECT_NAME_INVALID => unreachable, + .OBJECT_NAME_NOT_FOUND => return error.FileNotFound, + .OBJECT_PATH_NOT_FOUND => return error.FileNotFound, + .NO_MEDIA_IN_DEVICE => return error.NoDevice, + .INVALID_PARAMETER => unreachable, + .SHARING_VIOLATION => { + std.time.sleep(delay); + if (delay < 1 * std.time.ns_per_s) { + delay *= 2; + } + continue; // TODO: don't loop for async + }, + .ACCESS_DENIED => return error.AccessDenied, + .PIPE_BUSY => return error.PipeBusy, + .OBJECT_PATH_SYNTAX_BAD => unreachable, + .OBJECT_NAME_COLLISION => return error.PathAlreadyExists, + else => return unexpectedStatus(rc), + } } } From e7cf3f92a98cd2bd94d9d94fb361819b6ab89e8d Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 2 Apr 2020 22:12:45 -0600 Subject: [PATCH 23/39] Add FileLocksNotSupported error to OpenError --- lib/std/os.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/std/os.zig b/lib/std/os.zig index 49edda23ba..197857e61f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -846,6 +846,9 @@ pub const OpenError = error{ /// The path already exists and the `O_CREAT` and `O_EXCL` flags were provided. PathAlreadyExists, DeviceBusy, + + /// The underlying filesystem does not support file locks + FileLocksNotSupported } || UnexpectedError; /// Open and possibly create a file. Keeps trying if it gets interrupted. @@ -931,6 +934,7 @@ pub fn openatZ(dir_fd: fd_t, file_path: [*:0]const u8, flags: u32, mode: mode_t) EPERM => return error.AccessDenied, EEXIST => return error.PathAlreadyExists, EBUSY => return error.DeviceBusy, + EOPNOTSUPP => return error.FileLocksNotSupported, else => |err| return unexpectedErrno(err), } } From ea6525797d0db83a8560179b317837cb58634049 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 2 Apr 2020 22:57:02 -0600 Subject: [PATCH 24/39] Use `flock` instead of `fcntl` to lock files `flock` locks based on the file handle, instead of the process id. This brings the file locking on unix based systems closer to file locking on Windows. --- lib/std/c.zig | 1 + lib/std/fs.zig | 71 ++++-------------------------- lib/std/fs/file.zig | 2 +- lib/std/os.zig | 32 ++++++++++---- lib/std/os/bits/darwin.zig | 5 +++ lib/std/os/bits/linux/arm-eabi.zig | 5 +++ lib/std/os/bits/linux/arm64.zig | 5 +++ lib/std/os/bits/linux/i386.zig | 5 +++ lib/std/os/bits/linux/mipsel.zig | 5 +++ lib/std/os/bits/linux/riscv64.zig | 5 +++ lib/std/os/bits/linux/x86_64.zig | 5 +++ lib/std/os/linux.zig | 4 ++ 12 files changed, 73 insertions(+), 72 deletions(-) diff --git a/lib/std/c.zig b/lib/std/c.zig index 1261974d65..114d12afe8 100644 --- a/lib/std/c.zig +++ b/lib/std/c.zig @@ -122,6 +122,7 @@ pub extern "c" fn sysctlnametomib(name: [*:0]const u8, mibp: ?*c_int, sizep: ?*u pub extern "c" fn tcgetattr(fd: fd_t, termios_p: *termios) c_int; pub extern "c" fn tcsetattr(fd: fd_t, optional_action: TCSA, termios_p: *const termios) c_int; pub extern "c" fn fcntl(fd: fd_t, cmd: c_int, ...) c_int; +pub extern "c" fn flock(fd: fd_t, operation: c_int) c_int; pub extern "c" fn uname(buf: *utsname) c_int; pub extern "c" fn gethostname(name: [*]u8, len: usize) c_int; diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 325e2b2834..ccfe54c5c4 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -617,14 +617,9 @@ pub const Dir = struct { else try os.openatZ(self.fd, sub_path, os_flags, 0); - // use fcntl file locking if no lock flag was given if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O - // mem.zeroes is used here because flock's structure can vary across architectures and systems - var flock = mem.zeroes(os.Flock); - flock.l_type = if (flags.write) os.F_WRLCK else os.F_RDLCK; - flock.l_whence = os.SEEK_SET; - _ = try os.fcntl(fd, os.F_SETLKW, @ptrToInt(&flock)); + _ = try os.flock(fd, if (flags.write) os.LOCK_EX else os.LOCK_SH); } return File{ @@ -695,10 +690,7 @@ pub const Dir = struct { if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O // mem.zeroes is used here because flock's structure can vary across architectures and systems - var flock = mem.zeroes(os.Flock); - flock.l_type = os.F_WRLCK; - flock.l_whence = os.SEEK_SET; - _ = try os.fcntl(fd, os.F_SETLKW, @ptrToInt(&flock)); + _ = try os.flock(fd, os.LOCK_EX); } return File{ .handle = fd, .io_mode = .blocking }; @@ -1801,59 +1793,14 @@ const FileLockTestContext = struct { }; fn run_lock_file_test(contexts: []FileLockTestContext) !void { - var shared_mem: if (builtin.os.tag == .windows) void else []align(mem.page_size) u8 = undefined; - - var ctxs: []FileLockTestContext = undefined; - if (builtin.os.tag == .windows) { - ctxs = contexts; - } else { - shared_mem = try std.os.mmap(null, contexts.len * @sizeOf(FileLockTestContext), std.os.PROT_READ | std.os.PROT_WRITE, std.os.MAP_SHARED | std.os.MAP_ANONYMOUS, -1, 0); - const ctxs_ptr = @ptrCast([*]FileLockTestContext, shared_mem.ptr); - ctxs = ctxs_ptr[0..contexts.len]; - - for (contexts) |context, idx| { - ctxs[idx] = context; + var threads = std.ArrayList(*std.Thread).init(std.testing.allocator); + defer { + for (threads.toSlice()) |thread| { + thread.wait(); } + threads.deinit(); } - - if (builtin.os.tag == .windows) { - var threads = std.ArrayList(*std.Thread).init(std.testing.allocator); - defer { - for (threads.toSlice()) |thread| { - thread.wait(); - } - threads.deinit(); - } - for (ctxs) |*ctx, idx| { - try threads.append(try std.Thread.spawn(ctx, FileLockTestContext.run)); - } - } else { - var ctx_opt: ?*FileLockTestContext = null; - for (ctxs) |*ctx| { - const childpid = try std.os.fork(); - if (childpid == 0) { - ctx_opt = ctx; - break; - } - ctx.pid = childpid; - } - - if (ctx_opt) |ctx| { - ctx.run(); - // Exit so we don't have duplicate test processes - std.os.exit(0); - } else { - for (ctxs) |ctx| { - _ = std.os.waitpid(ctx.pid.?, 0); - } - } - } - - if (builtin.os.tag != .windows) { - // Copy contexts out of shared memory - for (ctxs) |ctx, idx| { - contexts[idx] = ctx; - } - std.os.munmap(shared_mem); + for (contexts) |*ctx, idx| { + try threads.append(try std.Thread.spawn(ctx, FileLockTestContext.run)); } } diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index 52b33d5660..63ec013dda 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -34,7 +34,7 @@ pub const File = struct { else => 0o666, }; - pub const OpenError = windows.CreateFileError || os.OpenError || os.FcntlError; + pub const OpenError = windows.CreateFileError || os.OpenError || os.FlockError; /// TODO https://github.com/ziglang/zig/issues/3802 pub const OpenFlags = struct { diff --git a/lib/std/os.zig b/lib/std/os.zig index 197857e61f..aa302490d0 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -819,36 +819,28 @@ pub const OpenError = error{ SystemFdQuotaExceeded, NoDevice, FileNotFound, - /// The path exceeded `MAX_PATH_BYTES` bytes. NameTooLong, - /// Insufficient kernel memory was available, or /// the named file is a FIFO and per-user hard limit on /// memory allocation for pipes has been reached. SystemResources, - /// The file is too large to be opened. This error is unreachable /// for 64-bit targets, as well as when opening directories. FileTooBig, - /// The path refers to directory but the `O_DIRECTORY` flag was not provided. IsDir, - /// A new path cannot be created because the device has no room for the new file. /// This error is only reachable when the `O_CREAT` flag is provided. NoSpaceLeft, - /// A component used as a directory in the path was not, in fact, a directory, or /// `O_DIRECTORY` was specified and the path was not a directory. NotDir, - /// The path already exists and the `O_CREAT` and `O_EXCL` flags were provided. PathAlreadyExists, DeviceBusy, - /// The underlying filesystem does not support file locks - FileLocksNotSupported + FileLocksNotSupported, } || UnexpectedError; /// Open and possibly create a file. Keeps trying if it gets interrupted. @@ -3222,6 +3214,28 @@ pub fn fcntl(fd: fd_t, cmd: i32, arg: usize) FcntlError!usize { } } +pub const FlockError = error{ + WouldBlock, + + /// The kernel ran out of memory for allocating file locks + SystemResources, +} || UnexpectedError; + +pub fn flock(fd: fd_t, operation: i32) FlockError!usize { + while (true) { + const rc = system.flock(fd, operation); + switch (errno(rc)) { + 0 => return @intCast(usize, rc), + EBADF => unreachable, + EINTR => continue, + EINVAL => unreachable, // invalid parameters + ENOLCK => return error.SystemResources, + EWOULDBLOCK => return error.WouldBlock, // TODO: integrate with async instead of just returning an error + else => |err| return unexpectedErrno(err), + } + } +} + pub const RealPathError = error{ FileNotFound, AccessDenied, diff --git a/lib/std/os/bits/darwin.zig b/lib/std/os/bits/darwin.zig index 0d6ffc6432..3fa37fe0f5 100644 --- a/lib/std/os/bits/darwin.zig +++ b/lib/std/os/bits/darwin.zig @@ -1394,3 +1394,8 @@ pub const F_UNLCK = 2; /// exclusive or write lock pub const F_WRLCK = 3; + +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; diff --git a/lib/std/os/bits/linux/arm-eabi.zig b/lib/std/os/bits/linux/arm-eabi.zig index d85b6b53c1..b32d3ef9d4 100644 --- a/lib/std/os/bits/linux/arm-eabi.zig +++ b/lib/std/os/bits/linux/arm-eabi.zig @@ -462,6 +462,11 @@ pub const F_GETOWN_EX = 16; pub const F_GETOWNER_UIDS = 17; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + /// stack-like segment pub const MAP_GROWSDOWN = 0x0100; diff --git a/lib/std/os/bits/linux/arm64.zig b/lib/std/os/bits/linux/arm64.zig index 038cfd92d4..0745f19fb0 100644 --- a/lib/std/os/bits/linux/arm64.zig +++ b/lib/std/os/bits/linux/arm64.zig @@ -349,6 +349,11 @@ pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/linux/i386.zig b/lib/std/os/bits/linux/i386.zig index 731c3ba261..868e4f537b 100644 --- a/lib/std/os/bits/linux/i386.zig +++ b/lib/std/os/bits/linux/i386.zig @@ -482,6 +482,11 @@ pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/linux/mipsel.zig b/lib/std/os/bits/linux/mipsel.zig index 8f2064192f..5946e821ad 100644 --- a/lib/std/os/bits/linux/mipsel.zig +++ b/lib/std/os/bits/linux/mipsel.zig @@ -424,6 +424,11 @@ pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/linux/riscv64.zig b/lib/std/os/bits/linux/riscv64.zig index 50fdb905b8..4ac30a979b 100644 --- a/lib/std/os/bits/linux/riscv64.zig +++ b/lib/std/os/bits/linux/riscv64.zig @@ -343,6 +343,11 @@ pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/linux/x86_64.zig b/lib/std/os/bits/linux/x86_64.zig index f5eaf30544..c87bca6f8e 100644 --- a/lib/std/os/bits/linux/x86_64.zig +++ b/lib/std/os/bits/linux/x86_64.zig @@ -462,6 +462,11 @@ pub const REG_TRAPNO = 20; pub const REG_OLDMASK = 21; pub const REG_CR2 = 22; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_RDLCK = 0; pub const F_WRLCK = 1; pub const F_UNLCK = 2; diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 1ed8370274..87ee198985 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -592,6 +592,10 @@ pub fn fcntl(fd: fd_t, cmd: i32, arg: usize) usize { return syscall3(.fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), arg); } +pub fn flock(fd: fd_t, operation: i32) usize { + return syscall2(.flock, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, operation))); +} + var vdso_clock_gettime = @ptrCast(?*const c_void, init_vdso_clock_gettime); // We must follow the C calling convention when we call into the VDSO From 733f1c25bd34270b76c5fa43928dfffda1ecd5e3 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 2 Apr 2020 23:39:25 -0600 Subject: [PATCH 25/39] Fix compile errors in stage2 --- lib/std/child_process.zig | 1 + lib/std/fs.zig | 1 + lib/std/os.zig | 8 ++++++++ lib/std/zig/system.zig | 5 ++--- src-self-hosted/stage2.zig | 5 +---- src/error.cpp | 1 - src/stage2.h | 1 - 7 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index e1c489bf05..1bd0becfd0 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -357,6 +357,7 @@ pub const ChildProcess = struct { error.NoSpaceLeft => unreachable, error.FileTooBig => unreachable, error.DeviceBusy => unreachable, + error.FileLocksNotSupported => unreachable, else => |e| return e, } else diff --git a/lib/std/fs.zig b/lib/std/fs.zig index ccfe54c5c4..f64bd61569 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -843,6 +843,7 @@ pub const Dir = struct { error.IsDir => unreachable, // we're providing O_DIRECTORY error.NoSpaceLeft => unreachable, // not providing O_CREAT error.PathAlreadyExists => unreachable, // not providing O_CREAT + error.FileLocksNotSupported => unreachable, // locking folders is not supported else => |e| return e, }; return Dir{ .fd = fd }; diff --git a/lib/std/os.zig b/lib/std/os.zig index aa302490d0..e0e998a4a6 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -819,26 +819,34 @@ pub const OpenError = error{ SystemFdQuotaExceeded, NoDevice, FileNotFound, + /// The path exceeded `MAX_PATH_BYTES` bytes. NameTooLong, + /// Insufficient kernel memory was available, or /// the named file is a FIFO and per-user hard limit on /// memory allocation for pipes has been reached. SystemResources, + /// The file is too large to be opened. This error is unreachable /// for 64-bit targets, as well as when opening directories. FileTooBig, + /// The path refers to directory but the `O_DIRECTORY` flag was not provided. IsDir, + /// A new path cannot be created because the device has no room for the new file. /// This error is only reachable when the `O_CREAT` flag is provided. NoSpaceLeft, + /// A component used as a directory in the path was not, in fact, a directory, or /// `O_DIRECTORY` was specified and the path was not a directory. NotDir, + /// The path already exists and the `O_CREAT` and `O_EXCL` flags were provided. PathAlreadyExists, DeviceBusy, + /// The underlying filesystem does not support file locks FileLocksNotSupported, } || UnexpectedError; diff --git a/lib/std/zig/system.zig b/lib/std/zig/system.zig index 2ce7b406a4..f6a45860be 100644 --- a/lib/std/zig/system.zig +++ b/lib/std/zig/system.zig @@ -468,9 +468,8 @@ pub const NativeTargetInfo = struct { error.InvalidUtf8 => unreachable, error.BadPathName => unreachable, error.PipeBusy => unreachable, - error.PermissionDenied => unreachable, - error.FileBusy => unreachable, - error.Locked => unreachable, + error.FileLocksNotSupported => unreachable, + error.WouldBlock => unreachable, error.IsDir, error.NotDir, diff --git a/src-self-hosted/stage2.zig b/src-self-hosted/stage2.zig index 802493b7c0..2ea414106c 100644 --- a/src-self-hosted/stage2.zig +++ b/src-self-hosted/stage2.zig @@ -115,7 +115,6 @@ const Error = extern enum { InvalidOperatingSystemVersion, UnknownClangOption, NestedResponseFile, - PermissionDenied, FileBusy, Locked, }; @@ -849,9 +848,7 @@ export fn stage2_libc_parse(stage1_libc: *Stage2LibCInstallation, libc_file_z: [ error.NoDevice => return .NoDevice, error.NotDir => return .NotDir, error.DeviceBusy => return .DeviceBusy, - error.PermissionDenied => return .PermissionDenied, - error.FileBusy => return .FileBusy, - error.Locked => return .Locked, + error.FileLocksNotSupported => unreachable, }; stage1_libc.initFromStage2(libc); return .None; diff --git a/src/error.cpp b/src/error.cpp index 1ad9589a14..03597686b8 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -85,7 +85,6 @@ const char *err_str(Error err) { case ErrorInvalidOperatingSystemVersion: return "invalid operating system version"; case ErrorUnknownClangOption: return "unknown Clang option"; case ErrorNestedResponseFile: return "nested response file"; - case ErrorPermissionDenied: return "permission is denied"; case ErrorFileBusy: return "file is busy"; case ErrorLocked: return "file is locked by another process"; } diff --git a/src/stage2.h b/src/stage2.h index 76b6683ba3..0e5b28b0e8 100644 --- a/src/stage2.h +++ b/src/stage2.h @@ -107,7 +107,6 @@ enum Error { ErrorInvalidOperatingSystemVersion, ErrorUnknownClangOption, ErrorNestedResponseFile, - ErrorPermissionDenied, ErrorFileBusy, ErrorLocked, }; From 4dd0822a3680de84ce5e6fd2536ffc91e0aaefb1 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Thu, 2 Apr 2020 23:50:12 -0600 Subject: [PATCH 26/39] Add LOCK_* constants to BSD `os/bits` --- lib/std/os/bits/dragonfly.zig | 5 +++++ lib/std/os/bits/freebsd.zig | 5 +++++ lib/std/os/bits/netbsd.zig | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/lib/std/os/bits/dragonfly.zig b/lib/std/os/bits/dragonfly.zig index 4a9b51d472..bfbdd79623 100644 --- a/lib/std/os/bits/dragonfly.zig +++ b/lib/std/os/bits/dragonfly.zig @@ -692,6 +692,11 @@ pub const F_DUP2FD = 10; pub const F_DUPFD_CLOEXEC = 17; pub const F_DUP2FD_CLOEXEC = 18; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const Flock = extern struct { l_start: off_t, l_len: off_t, diff --git a/lib/std/os/bits/freebsd.zig b/lib/std/os/bits/freebsd.zig index 1417381051..9999dca62f 100644 --- a/lib/std/os/bits/freebsd.zig +++ b/lib/std/os/bits/freebsd.zig @@ -367,6 +367,11 @@ pub const F_RDLCK = 1; pub const F_WRLCK = 3; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const F_SETOWN_EX = 15; pub const F_GETOWN_EX = 16; diff --git a/lib/std/os/bits/netbsd.zig b/lib/std/os/bits/netbsd.zig index fa733fd0b5..91e101620f 100644 --- a/lib/std/os/bits/netbsd.zig +++ b/lib/std/os/bits/netbsd.zig @@ -447,6 +447,11 @@ pub const F_RDLCK = 1; pub const F_WRLCK = 3; pub const F_UNLCK = 2; +pub const LOCK_SH = 1; +pub const LOCK_EX = 2; +pub const LOCK_UN = 8; +pub const LOCK_NB = 4; + pub const FD_CLOEXEC = 1; pub const SEEK_SET = 0; From ea32a7d2bc395b268e7e5be4fccb3cdb91233e78 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Fri, 3 Apr 2020 00:27:34 -0600 Subject: [PATCH 27/39] Fix compile errors about adding error.FileLocksNotSupported --- lib/std/fs.zig | 2 +- lib/std/os.zig | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index f64bd61569..407fe38a2c 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1550,7 +1550,7 @@ pub fn walkPath(allocator: *Allocator, dir_path: []const u8) !Walker { return walker; } -pub const OpenSelfExeError = os.OpenError || os.windows.CreateFileError || SelfExePathError || os.FcntlError; +pub const OpenSelfExeError = os.OpenError || os.windows.CreateFileError || SelfExePathError || os.FlockError; pub fn openSelfExe() OpenSelfExeError!File { if (builtin.os.tag == .linux) { diff --git a/lib/std/os.zig b/lib/std/os.zig index e0e998a4a6..1df22b0456 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -3295,7 +3295,10 @@ pub fn realpathZ(pathname: [*:0]const u8, out_buffer: *[MAX_PATH_BYTES]u8) RealP return realpathW(&pathname_w, out_buffer); } if (builtin.os.tag == .linux and !builtin.link_libc) { - const fd = try openZ(pathname, linux.O_PATH | linux.O_NONBLOCK | linux.O_CLOEXEC, 0); + const fd = openZ(pathname, linux.O_PATH | linux.O_NONBLOCK | linux.O_CLOEXEC, 0) catch |err| switch (err) { + error.FileLocksNotSupported => unreachable, + else => |e| return e, + }; defer close(fd); var procfs_buf: ["/proc/self/fd/-2147483648".len:0]u8 = undefined; From 49886d2e452fd57f996795d92cd951649e4bc255 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 6 Apr 2020 22:07:27 -0600 Subject: [PATCH 28/39] Remove return value from os.flock() --- lib/std/fs.zig | 5 ++--- lib/std/os.zig | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index bd000c9693..c11ac74850 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -619,7 +619,7 @@ pub const Dir = struct { if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O - _ = try os.flock(fd, if (flags.write) os.LOCK_EX else os.LOCK_SH); + try os.flock(fd, if (flags.write) os.LOCK_EX else os.LOCK_SH); } return File{ @@ -689,8 +689,7 @@ pub const Dir = struct { if (flags.lock and lock_flag == 0) { // TODO: integrate async I/O - // mem.zeroes is used here because flock's structure can vary across architectures and systems - _ = try os.flock(fd, os.LOCK_EX); + try os.flock(fd, os.LOCK_EX); } return File{ .handle = fd, .io_mode = .blocking }; diff --git a/lib/std/os.zig b/lib/std/os.zig index 1df22b0456..ec05a4a68a 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -3229,11 +3229,11 @@ pub const FlockError = error{ SystemResources, } || UnexpectedError; -pub fn flock(fd: fd_t, operation: i32) FlockError!usize { +pub fn flock(fd: fd_t, operation: i32) FlockError!void { while (true) { const rc = system.flock(fd, operation); switch (errno(rc)) { - 0 => return @intCast(usize, rc), + 0 => return, EBADF => unreachable, EINTR => continue, EINVAL => unreachable, // invalid parameters From 20597c85968432e1de22f17d5593471eeb7475f2 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 6 Apr 2020 22:28:43 -0600 Subject: [PATCH 29/39] Only call `os.flock` on systems that lack openat locks --- lib/std/fs.zig | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index c11ac74850..6202377f31 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -596,13 +596,12 @@ pub const Dir = struct { } // Use the O_ locking flags if the os supports them + const has_flock_open_flags = @hasDecl(os, "O_EXLOCK") and @hasDecl(os, "O_SHLOCK"); const lock_flag: u32 = lock_flag: { - if (!flags.lock) break :lock_flag 0; - if (flags.write) { - break :lock_flag if (@hasDecl(os, "O_EXLOCK")) os.O_EXLOCK else 0; - } else { - break :lock_flag if (@hasDecl(os, "O_SHLOCK")) os.O_SHLOCK else 0; + if (has_flock_open_flags and flags.lock) { + break :lock_flag if (flags.write) os.O_EXLOCK else os.O_SHLOCK; } + break :lock_flag 0; }; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; @@ -617,7 +616,7 @@ pub const Dir = struct { else try os.openatZ(self.fd, sub_path, os_flags, 0); - if (flags.lock and lock_flag == 0) { + if (!has_flock_open_flags and flags.lock) { // TODO: integrate async I/O try os.flock(fd, if (flags.write) os.LOCK_EX else os.LOCK_SH); } @@ -672,10 +671,8 @@ pub const Dir = struct { } // Use the O_ locking flags if the os supports them - const lock_flag: u32 = lock_flag: { - if (!flags.lock) break :lock_flag 0; - break :lock_flag if (@hasDecl(os, "O_EXLOCK")) os.O_EXLOCK else 0; - }; + const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + const lock_flag: u32 = if (has_flock_open_flags and flags.lock) os.O_EXLOCK else 0; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; const os_flags = lock_flag | O_LARGEFILE | os.O_CREAT | os.O_CLOEXEC | @@ -687,7 +684,7 @@ pub const Dir = struct { else try os.openatZ(self.fd, sub_path_c, os_flags, flags.mode); - if (flags.lock and lock_flag == 0) { + if (!has_flock_open_flags and flags.lock) { // TODO: integrate async I/O try os.flock(fd, os.LOCK_EX); } From 28d71c97d1874abdeb782a82f655d250a77a2f69 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Mon, 6 Apr 2020 23:19:39 -0600 Subject: [PATCH 30/39] Fix compile error on darwin --- lib/std/fs.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 6202377f31..72b6aca350 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -599,9 +599,9 @@ pub const Dir = struct { const has_flock_open_flags = @hasDecl(os, "O_EXLOCK") and @hasDecl(os, "O_SHLOCK"); const lock_flag: u32 = lock_flag: { if (has_flock_open_flags and flags.lock) { - break :lock_flag if (flags.write) os.O_EXLOCK else os.O_SHLOCK; + break :lock_flag if (flags.write) @as(u32, os.O_EXLOCK) else @as(u32, os.O_SHLOCK); } - break :lock_flag 0; + break :lock_flag @as(u32, 0); }; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; From 71c5aab3e740c131e63766be57de19b3dd451972 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 7 Apr 2020 16:23:12 -0600 Subject: [PATCH 31/39] Make lock option an enum For some reason, this breaks file locking on windows. Not sure if this is a problem with wine. --- lib/std/fs.zig | 75 +++++++++++++++++++++++++++------------------ lib/std/fs/file.zig | 31 +++++++++++++------ 2 files changed, 67 insertions(+), 39 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 72b6aca350..203dda1c40 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -596,13 +596,12 @@ pub const Dir = struct { } // Use the O_ locking flags if the os supports them - const has_flock_open_flags = @hasDecl(os, "O_EXLOCK") and @hasDecl(os, "O_SHLOCK"); - const lock_flag: u32 = lock_flag: { - if (has_flock_open_flags and flags.lock) { - break :lock_flag if (flags.write) @as(u32, os.O_EXLOCK) else @as(u32, os.O_SHLOCK); - } - break :lock_flag @as(u32, 0); - }; + const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { + .None => @as(u32, 0), + .Shared => os.O_SHLOCK, + .Exclusive => os.O_EXLOCK, + } else 0; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; const os_flags = lock_flag | O_LARGEFILE | os.O_CLOEXEC | if (flags.write and flags.read) @@ -616,9 +615,13 @@ pub const Dir = struct { else try os.openatZ(self.fd, sub_path, os_flags, 0); - if (!has_flock_open_flags and flags.lock) { + if (!has_flock_open_flags and flags.lock != .None) { // TODO: integrate async I/O - try os.flock(fd, if (flags.write) os.LOCK_EX else os.LOCK_SH); + try os.flock(fd, switch (flags.lock) { + .None => unreachable, + .Shared => os.LOCK_SH, + .Exclusive => os.LOCK_EX, + }); } return File{ @@ -638,11 +641,13 @@ pub const Dir = struct { const access_mask = w.SYNCHRONIZE | (if (flags.read) @as(u32, w.GENERIC_READ) else 0) | (if (flags.write) @as(u32, w.GENERIC_WRITE) else 0); - const share_access = if (flags.lock) - w.FILE_SHARE_DELETE | - (if (flags.write) @as(os.windows.ULONG, 0) else w.FILE_SHARE_READ) - else - null; + + const share_access = switch (flags.lock) { + .None => @as(?w.ULONG, null), + .Shared => w.FILE_SHARE_READ, + .Exclusive => @as(?w.ULONG, 0), + }; + return @as(File, .{ .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, w.FILE_OPEN), .io_mode = .blocking, @@ -672,7 +677,11 @@ pub const Dir = struct { // Use the O_ locking flags if the os supports them const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); - const lock_flag: u32 = if (has_flock_open_flags and flags.lock) os.O_EXLOCK else 0; + const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { + .None => @as(u32, 0), + .Shared => os.O_SHLOCK, + .Exclusive => os.O_EXLOCK, + } else 0; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; const os_flags = lock_flag | O_LARGEFILE | os.O_CREAT | os.O_CLOEXEC | @@ -684,9 +693,13 @@ pub const Dir = struct { else try os.openatZ(self.fd, sub_path_c, os_flags, flags.mode); - if (!has_flock_open_flags and flags.lock) { + if (!has_flock_open_flags and flags.lock != .None) { // TODO: integrate async I/O - try os.flock(fd, os.LOCK_EX); + try os.flock(fd, switch (flags.lock) { + .None => unreachable, + .Shared => os.LOCK_SH, + .Exclusive => os.LOCK_EX, + }); } return File{ .handle = fd, .io_mode = .blocking }; @@ -705,10 +718,12 @@ pub const Dir = struct { else @as(u32, w.FILE_OPEN_IF); - const share_access = if (flags.lock) - @as(os.windows.ULONG, w.FILE_SHARE_DELETE) - else - null; + const share_access = switch (flags.lock) { + .None => @as(?w.ULONG, null), + .Shared => w.FILE_SHARE_READ, + .Exclusive => @as(?w.ULONG, 0), + }; + return @as(File, .{ .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, creation), .io_mode = .blocking, @@ -1671,8 +1686,8 @@ test "open file with lock twice, make sure it wasn't open at the same time" { const filename = "file_lock_test.txt"; var contexts = [_]FileLockTestContext{ - .{ .filename = filename, .create = true, .exclusive = true }, - .{ .filename = filename, .create = true, .exclusive = true }, + .{ .filename = filename, .create = true, .lock = .Exclusive }, + .{ .filename = filename, .create = true, .lock = .Exclusive }, }; try run_lock_file_test(&contexts); @@ -1703,9 +1718,9 @@ test "create file, lock and read from multiple process at once" { try std.fs.cwd().writeFile(filename, filedata); var contexts = [_]FileLockTestContext{ - .{ .filename = filename, .create = false, .exclusive = false }, - .{ .filename = filename, .create = false, .exclusive = false }, - .{ .filename = filename, .create = false, .exclusive = true }, + .{ .filename = filename, .create = false, .lock = .Shared }, + .{ .filename = filename, .create = false, .lock = .Shared }, + .{ .filename = filename, .create = false, .lock = .Exclusive }, }; try run_lock_file_test(&contexts); @@ -1740,8 +1755,8 @@ const FileLockTestContext = struct { // use file.createFile create: bool, - // get a read/write lock, instead of just a read lock - exclusive: bool, + // the type of lock to use + lock: File.Lock, // Output variables err: ?(File.OpenError || std.os.ReadError) = null, @@ -1756,12 +1771,12 @@ const FileLockTestContext = struct { fn run(ctx: *@This()) void { var file: File = undefined; if (ctx.create) { - file = cwd().createFile(ctx.filename, .{ .lock = true }) catch |err| { + file = cwd().createFile(ctx.filename, .{ .lock = ctx.lock }) catch |err| { ctx.err = err; return; }; } else { - file = cwd().openFile(ctx.filename, .{ .lock = true, .write = ctx.exclusive }) catch |err| { + file = cwd().openFile(ctx.filename, .{ .lock = ctx.lock }) catch |err| { ctx.err = err; return; }; diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index 63ec013dda..32eb1460c4 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -36,22 +36,29 @@ pub const File = struct { pub const OpenError = windows.CreateFileError || os.OpenError || os.FlockError; + pub const Lock = enum { + None, Shared, Exclusive + }; + /// TODO https://github.com/ziglang/zig/issues/3802 pub const OpenFlags = struct { read: bool = true, write: bool = false, - /// Open the file with exclusive access. If `write` is true, then the file is opened with an - /// exclusive lock, meaning that no other processes can read or write to the file. Otherwise - /// the file is opened with a shared lock, allowing the other processes to read from the - /// file, but not to write to the file. + /// Open the file with a lock to prevent other processes from accessing it at the + /// same time. An exclusive lock will prevent other processes from acquiring a lock. + /// A shared lock will prevent other processes from acquiring a exclusive lock, but + /// doesn't prevent other process from getting their own shared locks. /// /// Note that the lock is only advisory on Linux, except in very specific cirsumstances[1]. - /// This means that a process that does not respect the locking API can still read and write + /// This means that a process that does not respect the locking API can still get access /// to the file, despite the lock. /// + /// Windows' file locks are mandatory, and any process attempting to access the file will + /// receive an error. + /// /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt - lock: bool = false, + lock: Lock = .None, /// This prevents `O_NONBLOCK` from being passed even if `std.io.is_async`. /// It allows the use of `noasync` when calling functions related to opening @@ -72,14 +79,20 @@ pub const File = struct { /// `error.FileAlreadyExists` to be returned. exclusive: bool = false, - /// Prevent other files from accessing this file while this process has it is open. + /// Open the file with a lock to prevent other processes from accessing it at the + /// same time. An exclusive lock will prevent other processes from acquiring a lock. + /// A shared lock will prevent other processes from acquiring a exclusive lock, but + /// doesn't prevent other process from getting their own shared locks. /// /// Note that the lock is only advisory on Linux, except in very specific cirsumstances[1]. - /// This means that a process that does not respect the locking API can still read and write + /// This means that a process that does not respect the locking API can still get access /// to the file, despite the lock. /// + /// Windows' file locks are mandatory, and any process attempting to access the file will + /// receive an error. + /// /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt - lock: bool = false, + lock: Lock = .None, /// For POSIX systems this is the file system mode the file will /// be created with. From 117d15ed7a230d065cf88faa8156c08f7160e08b Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 7 Apr 2020 16:49:37 -0600 Subject: [PATCH 32/39] Fix file locking on windows The share_access bitfield was being ORed with what was supposed to be parts of the default value, meaning that the share_access would be more permissive than expected. --- lib/std/os/windows.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index efc85d38ac..817244e089 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -136,7 +136,7 @@ pub fn OpenFileW( .SecurityQualityOfService = null, }; var io: IO_STATUS_BLOCK = undefined; - const share_access = share_access_opt orelse FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; + const share_access = share_access_opt orelse (FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE); var delay: usize = 1; while (true) { From 317f06dc7741dcb5579381fc4ad26dd23346eb67 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 7 Apr 2020 18:00:12 -0600 Subject: [PATCH 33/39] Add lock_nonblocking flag for creating or opening files Also, make windows share delete access. Rationale: this is how it works on Unix systems, mostly because locks are (usually) advisory on Unix. --- lib/std/fs.zig | 43 ++++++++++++++++++++++++++++++------------ lib/std/fs/file.zig | 10 ++++++++++ lib/std/os/windows.zig | 5 +++++ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 203dda1c40..f965ac2453 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -597,10 +597,11 @@ pub const Dir = struct { // Use the O_ locking flags if the os supports them const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + const nonblocking_lock_flag = if (has_flock_open_flags and flags.lock_nonblocking) (os.O_NONBLOCK | os.O_SYNC) else @as(u32, 0); const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { .None => @as(u32, 0), - .Shared => os.O_SHLOCK, - .Exclusive => os.O_EXLOCK, + .Shared => os.O_SHLOCK | nonblocking_lock_flag, + .Exclusive => os.O_EXLOCK | nonblocking_lock_flag, } else 0; const O_LARGEFILE = if (@hasDecl(os, "O_LARGEFILE")) os.O_LARGEFILE else 0; @@ -617,10 +618,11 @@ pub const Dir = struct { if (!has_flock_open_flags and flags.lock != .None) { // TODO: integrate async I/O + const lock_nonblocking = if (flags.lock_nonblocking) os.LOCK_NB else @as(i32, 0); try os.flock(fd, switch (flags.lock) { .None => unreachable, - .Shared => os.LOCK_SH, - .Exclusive => os.LOCK_EX, + .Shared => os.LOCK_SH | lock_nonblocking, + .Exclusive => os.LOCK_EX | lock_nonblocking, }); } @@ -644,12 +646,12 @@ pub const Dir = struct { const share_access = switch (flags.lock) { .None => @as(?w.ULONG, null), - .Shared => w.FILE_SHARE_READ, - .Exclusive => @as(?w.ULONG, 0), + .Shared => w.FILE_SHARE_READ | w.FILE_SHARE_DELETE, + .Exclusive => w.FILE_SHARE_DELETE, }; return @as(File, .{ - .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, w.FILE_OPEN), + .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, flags.lock_nonblocking, w.FILE_OPEN), .io_mode = .blocking, }); } @@ -677,6 +679,7 @@ pub const Dir = struct { // Use the O_ locking flags if the os supports them const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + const nonblocking_lock_flag = if (has_flock_open_flags and flags.lock_nonblocking) (os.O_NONBLOCK | os.O_SYNC) else @as(u32, 0); const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { .None => @as(u32, 0), .Shared => os.O_SHLOCK, @@ -695,10 +698,11 @@ pub const Dir = struct { if (!has_flock_open_flags and flags.lock != .None) { // TODO: integrate async I/O + const lock_nonblocking = if (flags.lock_nonblocking) os.LOCK_NB else @as(i32, 0); try os.flock(fd, switch (flags.lock) { .None => unreachable, - .Shared => os.LOCK_SH, - .Exclusive => os.LOCK_EX, + .Shared => os.LOCK_SH | lock_nonblocking, + .Exclusive => os.LOCK_EX | lock_nonblocking, }); } @@ -720,12 +724,12 @@ pub const Dir = struct { const share_access = switch (flags.lock) { .None => @as(?w.ULONG, null), - .Shared => w.FILE_SHARE_READ, - .Exclusive => @as(?w.ULONG, 0), + .Shared => w.FILE_SHARE_READ | w.FILE_SHARE_DELETE, + .Exclusive => w.FILE_SHARE_DELETE, }; return @as(File, .{ - .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, creation), + .handle = try os.windows.OpenFileW(self.fd, sub_path_w, null, access_mask, share_access, flags.lock_nonblocking, creation), .io_mode = .blocking, }); } @@ -1680,6 +1684,21 @@ test "" { const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; +test "open file with exclusive nonblocking lock twice" { + const dir = cwd(); + const filename = "file_nonblocking_lock_test.txt"; + + const file1 = try dir.createFile(filename, .{ .lock = .Exclusive, .lock_nonblocking = true }); + + const file2 = dir.createFile(filename, .{ .lock = .Exclusive, .lock_nonblocking = true }); + std.debug.assert(std.meta.eql(file2, error.WouldBlock)); + + dir.deleteFile(filename) catch |err| switch (err) { + error.FileNotFound => {}, + else => return err, + }; +} + test "open file with lock twice, make sure it wasn't open at the same time" { if (builtin.single_threaded) return; diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index 32eb1460c4..2a6ad875c5 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -60,6 +60,11 @@ pub const File = struct { /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt lock: Lock = .None, + /// Sets whether or not to wait until the file is locked to return. If set to true, + /// `error.WouldBlock` will be returned. Otherwise, the file will wait until the file + /// is available to proceed. + lock_nonblocking: bool = false, + /// This prevents `O_NONBLOCK` from being passed even if `std.io.is_async`. /// It allows the use of `noasync` when calling functions related to opening /// the file, reading, and writing. @@ -94,6 +99,11 @@ pub const File = struct { /// [1]: https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt lock: Lock = .None, + /// Sets whether or not to wait until the file is locked to return. If set to true, + /// `error.WouldBlock` will be returned. Otherwise, the file will wait until the file + /// is available to proceed. + lock_nonblocking: bool = false, + /// For POSIX systems this is the file system mode the file will /// be created with. mode: Mode = default_mode, diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 817244e089..72593b92c7 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -98,6 +98,7 @@ pub const OpenError = error{ PathAlreadyExists, Unexpected, NameTooLong, + WouldBlock, }; /// TODO rename to CreateFileW @@ -108,6 +109,7 @@ pub fn OpenFileW( sa: ?*SECURITY_ATTRIBUTES, access_mask: ACCESS_MASK, share_access_opt: ?ULONG, + share_access_nonblocking: bool, creation: ULONG, ) OpenError!HANDLE { if (sub_path_w[0] == '.' and sub_path_w[1] == 0) { @@ -161,6 +163,9 @@ pub fn OpenFileW( .NO_MEDIA_IN_DEVICE => return error.NoDevice, .INVALID_PARAMETER => unreachable, .SHARING_VIOLATION => { + if (share_access_nonblocking) { + return error.WouldBlock; + } std.time.sleep(delay); if (delay < 1 * std.time.ns_per_s) { delay *= 2; From 858aefac7f6085b60d32de053f109f36896ab2ca Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Tue, 7 Apr 2020 21:26:46 -0600 Subject: [PATCH 34/39] Add `OpenFileW` `share_access_nonblocking` parameter --- lib/std/os.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index ec05a4a68a..aefaeddca4 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1680,7 +1680,7 @@ pub fn renameatW( ReplaceIfExists: windows.BOOLEAN, ) RenameError!void { const access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE; - const src_fd = try windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, windows.FILE_OPEN); + const src_fd = try windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, false, windows.FILE_OPEN); defer windows.CloseHandle(src_fd); const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION) + (MAX_PATH_BYTES - 1); From 45d6fb9e3691120428a70f659cc211faf6cb1897 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Wed, 8 Apr 2020 00:39:17 -0600 Subject: [PATCH 35/39] Catch error.WouldBlock as unreachable --- lib/std/os.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index aefaeddca4..bda54a0458 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1680,7 +1680,10 @@ pub fn renameatW( ReplaceIfExists: windows.BOOLEAN, ) RenameError!void { const access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE; - const src_fd = try windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, false, windows.FILE_OPEN); + const src_fd = windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, false, windows.FILE_OPEN) catch |err| switch (err) { + error.WouldBlock => unreachable, + else => return err, + }; defer windows.CloseHandle(src_fd); const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION) + (MAX_PATH_BYTES - 1); From d0d7895d3308763422e4da0eb5d6e03c445f1b08 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Wed, 8 Apr 2020 08:37:17 -0600 Subject: [PATCH 36/39] Return error from `else` That removes the other switch cases (`error.WouldBlock` here) from the error set, I think. --- lib/std/os.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index bda54a0458..0ce10601b7 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1682,7 +1682,7 @@ pub fn renameatW( const access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE; const src_fd = windows.OpenFileW(old_dir_fd, old_path, null, access_mask, null, false, windows.FILE_OPEN) catch |err| switch (err) { error.WouldBlock => unreachable, - else => return err, + else => |e| return e, }; defer windows.CloseHandle(src_fd); From 772bb1ade372ad625381ccee198c812fcccc0e3d Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Wed, 8 Apr 2020 16:29:44 -0600 Subject: [PATCH 37/39] Disable open flock flags on darwin The tests were put into a deadlock, and it seems that darwin doesn't support `O_SYNC`, though it supports `O_NONBLOCK`. It shouldn't block even with that, but I'm not sure why else it would fail. --- lib/std/fs.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index f965ac2453..94edd6e734 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -596,7 +596,8 @@ pub const Dir = struct { } // Use the O_ locking flags if the os supports them - const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + // (Or if it's darwin, as darwin's `open` doesn't support the O_SYNC flag) + const has_flock_open_flags = @hasDecl(os, "O_EXLOCK") and !builtin.os.tag.isDarwin(); const nonblocking_lock_flag = if (has_flock_open_flags and flags.lock_nonblocking) (os.O_NONBLOCK | os.O_SYNC) else @as(u32, 0); const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { .None => @as(u32, 0), @@ -678,7 +679,8 @@ pub const Dir = struct { } // Use the O_ locking flags if the os supports them - const has_flock_open_flags = @hasDecl(os, "O_EXLOCK"); + // (Or if it's darwin, as darwin's `open` doesn't support the O_SYNC flag) + const has_flock_open_flags = @hasDecl(os, "O_EXLOCK") and !builtin.os.tag.isDarwin(); const nonblocking_lock_flag = if (has_flock_open_flags and flags.lock_nonblocking) (os.O_NONBLOCK | os.O_SYNC) else @as(u32, 0); const lock_flag: u32 = if (has_flock_open_flags) switch (flags.lock) { .None => @as(u32, 0), From d4161e1667e8b5c63859e524645d04eaf479c0b2 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Wed, 8 Apr 2020 16:42:11 -0600 Subject: [PATCH 38/39] Close file1 in nonblocking lock test --- lib/std/fs.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 94edd6e734..ddbd278d76 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1691,6 +1691,7 @@ test "open file with exclusive nonblocking lock twice" { const filename = "file_nonblocking_lock_test.txt"; const file1 = try dir.createFile(filename, .{ .lock = .Exclusive, .lock_nonblocking = true }); + defer file1.close(); const file2 = dir.createFile(filename, .{ .lock = .Exclusive, .lock_nonblocking = true }); std.debug.assert(std.meta.eql(file2, error.WouldBlock)); From 5951211d3fc348fc37a86abc9906acf4ee796883 Mon Sep 17 00:00:00 2001 From: LeRoyce Pearson Date: Wed, 8 Apr 2020 18:03:52 -0600 Subject: [PATCH 39/39] Reduce file lock test sleep time --- lib/std/fs.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index ddbd278d76..afad7d1789 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1684,7 +1684,7 @@ test "" { _ = @import("fs/watch.zig"); } -const FILE_LOCK_TEST_SLEEP_TIME = 1 * std.time.ns_per_s; +const FILE_LOCK_TEST_SLEEP_TIME = 5 * std.time.millisecond; test "open file with exclusive nonblocking lock twice" { const dir = cwd();