Commit ea6525797d

LeRoyce Pearson <leroycepearson@geemili.xyz>
2020-04-03 06:57:02
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.
1 parent e7cf3f9
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 {
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;
 
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;
 
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;
 
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;
 
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;
 
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;
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;
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
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;
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;
-        }
-    }
-
-    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);
-            }
+    var threads = std.ArrayList(*std.Thread).init(std.testing.allocator);
+    defer {
+        for (threads.toSlice()) |thread| {
+            thread.wait();
         }
+        threads.deinit();
     }
-
-    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));
     }
 }
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,