lock and lock(wait:)

Author: milseman

Sources/System/FileControl.swift

@@ -12,7 +12,7 @@
 
 extension FileDescriptor {
   internal func _fcntl(
-    _ cmd: Control.Command, _ lock: inout FileDescriptor.FileLock,
+    _ cmd: Command, _ lock: inout FileDescriptor.FileLock,
     retryOnInterrupt: Bool
   ) -> Result<(), Errno> {
     nothingOrErrno(retryOnInterrupt: retryOnInterrupt) {

Sources/System/FileControlRaw.swift

@@ -20,75 +20,40 @@ import Glibc
 
 #if !os(Windows)
 
-extension FileDescriptor {
-  /// A namespace for types and values for `FileDescriptor.control()`, aka `fcntl`.
-  ///
-  /// TODO: a better name? "Internals", "Raw", "FCNTL"? I feel like a
-  /// precedent would be useful for sysctl, ioctl, and other grab-bag
-  /// things. "junk drawer" can be an anti-pattern, but is better than
-  /// trashing the higher namespace.
-  //  public
-  internal enum Control {}
-
-}
 // - MARK: Commands
 
-extension FileDescriptor.Control {
+// TODO: Make below API as part of broader `fcntl` support.
+extension FileDescriptor {
   /// Commands (and various constants) to pass to `fcntl`.
-  //  @frozen
-  //  public
   internal struct Command: RawRepresentable, Hashable {
-//    @_alwaysEmitIntoClient
-//    public
     internal let rawValue: CInt
 
-//    @_alwaysEmitIntoClient
-//    public
     internal init(rawValue: CInt) { self.rawValue = rawValue }
 
     @_alwaysEmitIntoClient
     private init(_ raw: CInt) { self.init(rawValue: raw) }
+
     /// Get open file description record locking information.
     ///
-    /// TODO: link to https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
-    /// TODO: reference FileDesciptor.isLocked() or something like that
-    ///
     /// The corresponding C constant is `F_GETLK`.
-//    @_alwaysEmitIntoClient
-//    public
     internal static var getOFDLock: Command { Command(_F_OFD_GETLK) }
 
     /// Set open file description record locking information.
     ///
-    /// TODO: link to https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
-    /// TODO: reference FileDesciptor.lock()
-    ///
     /// The corresponding C constant is `F_SETLK`.
-//    @_alwaysEmitIntoClient
-//    public
     internal static var setOFDLock: Command { Command(_F_OFD_SETLK) }
 
     /// Set open file description record locking information and wait until
     /// the request can be completed.
     ///
-    /// TODO: link to https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
-    /// TODO: reference FileDesciptor.lock()
-    ///
     /// The corresponding C constant is `F_SETLKW`.
-//    @_alwaysEmitIntoClient
-//    public
     internal static var setOFDLockWait: Command { Command(_F_OFD_SETLKW) }
 
 #if !os(Linux)
     /// Set open file description record locking information and wait until
     /// the request can be completed, returning on timeout.
     ///
-    /// TODO: link to https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
-    /// TODO: reference FileDesciptor.lock()
-    ///
     /// The corresponding C constant is `F_SETLKWTIMEOUT`.
-//    @_alwaysEmitIntoClient
-//    public
     internal static var setOFDLockWaitTimout: Command {
       Command(_F_OFD_SETLKWTIMEOUT)
     }

Sources/System/FileLock.swift

@@ -123,6 +123,18 @@ extension FileDescriptor.FileLock {
     public static var none: Self {
       Self(rawValue: CInterop.CShort(truncatingIfNeeded: F_UNLCK))
     }
+
+    /// Shared (alias for `read`)
+    @_alwaysEmitIntoClient
+    public static var shared: Self { .read }
+
+    /// Exclusive (alias for `write`)
+    @_alwaysEmitIntoClient
+    public static var exclusive: Self { .write }
+
+    /// Unlock (alias for `none`)
+    @_alwaysEmitIntoClient
+    public static var unlock: Self { .none }
   }
 }
 
@@ -131,7 +143,8 @@ extension FileDescriptor {
   ///
   /// If the open file description already has a lock, the old lock is
   /// replaced. If the lock cannot be set because it is blocked by an existing lock,
-  /// this will wait until the lock can be set.
+  /// that is if the syscall would throw `.resourceTemporarilyUnavailable`
+  /// (aka `EAGAIN`), this will return `false`.
   ///
   /// Open file description locks are associated with an open file
   /// description (see `FileDescriptor.open`). Duplicated
@@ -156,29 +169,31 @@ extension FileDescriptor {
   ///   - retryOnInterrupt: Whether to retry the operation if it throws
   ///     ``Errno/interrupted``. The default is `true`. Pass `false` to try
   ///     only once and throw an error upon interruption.
+  /// - Returns: `true` if the lock was aquired, `false` otherwise
   ///
-  /// The corresponding C function is `fcntl` with `F_OFD_SETLKW`.
+  /// The corresponding C function is `fcntl` with `F_OFD_SETLK`.
   @_alwaysEmitIntoClient
   public func lock(
     _ kind: FileDescriptor.FileLock.Kind = .read,
     byteRange: (some RangeExpression<Int64>)? = Range?.none,
     retryOnInterrupt: Bool = true
-  ) throws {
+  ) throws -> Bool {
     let (start, len) = _mapByteRangeToByteOffsets(byteRange)
-    try _lock(
+    return try _lock(
       kind,
       start: start,
       length: len,
+      wait: false,
+      waitUntilTimeout: false,
       retryOnInterrupt: retryOnInterrupt
-    ).get()
+    )?.get() != nil
   }
 
-  /// Try to set an advisory open file description lock.
+  /// Set an advisory open file description lock.
   ///
   /// If the open file description already has a lock, the old lock is
-  /// replaced. If the lock cannot be set because it is blocked by an existing lock,
-  /// that is if the syscall would throw `.resourceTemporarilyUnavailable`
-  /// (aka `EAGAIN`), this will return `false`.
+  /// replaced. If the lock cannot be set because it is blocked by an existing lock and
+  /// `wait` is true, this will wait until the lock can be set, otherwise returns `false`.
   ///
   /// Open file description locks are associated with an open file
   /// description (see `FileDescriptor.open`). Duplicated
@@ -200,38 +215,38 @@ extension FileDescriptor {
   ///   - kind: The kind of lock to set
   ///   - byteRange: The range of bytes over which to lock. Pass
   ///     `nil` to consider the entire file.
+  ///   - wait: if `true` will wait until the lock can be set
   ///   - retryOnInterrupt: Whether to retry the operation if it throws
   ///     ``Errno/interrupted``. The default is `true`. Pass `false` to try
   ///     only once and throw an error upon interruption.
-  /// - Returns: `true` if the lock was aquired, `false` otherwise
   ///
-  /// The corresponding C function is `fcntl` with `F_OFD_SETLK`.
+  /// The corresponding C function is `fcntl` with `F_OFD_SETLK` or `F_OFD_SETLKW`.
+  @discardableResult
   @_alwaysEmitIntoClient
-  public func tryLock(
+  public func lock(
     _ kind: FileDescriptor.FileLock.Kind = .read,
     byteRange: (some RangeExpression<Int64>)? = Range?.none,
+    wait: Bool,
     retryOnInterrupt: Bool = true
   ) throws -> Bool {
     let (start, len) = _mapByteRangeToByteOffsets(byteRange)
-    guard let _ = try _tryLock(
+    return try _lock(
       kind,
-      waitUntilTimeout: false,
       start: start,
       length: len,
+      wait: wait,
+      waitUntilTimeout: false,
       retryOnInterrupt: retryOnInterrupt
-    )?.get() else {
-      return false
-    }
-    return true
+    )?.get() != nil
   }
 
-  #if !os(Linux)
-  /// Try to set an advisory open file description lock.
+#if !os(Linux)
+  /// Set an advisory open file description lock.
   ///
   /// If the open file description already has a lock, the old lock is
-  /// replaced. If the lock cannot be set because it is blocked by an existing lock,
-  /// that is if the syscall would throw `.resourceTemporarilyUnavailable`
-  /// (aka `EAGAIN`), this will return `false`.
+  /// replaced. If the lock cannot be set because it is blocked by an existing lock and
+  /// `waitUntilTimeout` is true, this will wait until the lock can be set (or the operating
+  /// system's timeout expires), otherwise returns `false`.
   ///
   /// Open file description locks are associated with an open file
   /// description (see `FileDescriptor.open`). Duplicated
@@ -253,33 +268,30 @@ extension FileDescriptor {
   ///   - kind: The kind of lock to set
   ///   - byteRange: The range of bytes over which to lock. Pass
   ///     `nil` to consider the entire file.
-  ///   - waitUntilTimeout: If `true`, will wait until a timeout (determined by the operating system)
+  ///   - waitUntilTimeout: if `true` will wait until the lock can be set or a timeout expires
   ///   - retryOnInterrupt: Whether to retry the operation if it throws
   ///     ``Errno/interrupted``. The default is `true`. Pass `false` to try
   ///     only once and throw an error upon interruption.
-  /// - Returns: `true` if the lock was aquired, `false` otherwise
   ///
-  /// The corresponding C function is `fcntl` with `F_OFD_SETLK` or `F_OFD_SETLKWTIMEOUT` .
+  /// The corresponding C function is `fcntl` with `F_OFD_SETLK` or `F_SETLKWTIMEOUT`.
   @_alwaysEmitIntoClient
-  public func tryLock(
+  public func lock(
     _ kind: FileDescriptor.FileLock.Kind = .read,
     byteRange: (some RangeExpression<Int64>)? = Range?.none,
     waitUntilTimeout: Bool,
     retryOnInterrupt: Bool = true
   ) throws -> Bool {
     let (start, len) = _mapByteRangeToByteOffsets(byteRange)
-    guard let _ = try _tryLock(
+    return try _lock(
       kind,
-      waitUntilTimeout: waitUntilTimeout,
       start: start,
       length: len,
+      wait: false,
+      waitUntilTimeout: waitUntilTimeout,
       retryOnInterrupt: retryOnInterrupt
-    )?.get() else {
-      return false
-    }
-    return true
+    )?.get() != nil
   }
-  #endif
+#endif
 
   /// Remove an open file description lock.
   ///
@@ -311,49 +323,48 @@ extension FileDescriptor {
   @_alwaysEmitIntoClient
   public func unlock(
     byteRange: (some RangeExpression<Int64>)? = Range?.none,
-    wait: Bool = false, // FIXME: needed?
     retryOnInterrupt: Bool = true
   ) throws {
     let (start, len) = _mapByteRangeToByteOffsets(byteRange)
-    guard let res = _tryLock(
+    guard try _lock(
       .none,
-      waitUntilTimeout: false, // TODO: or we wait for timeout?
       start: start,
       length: len,
+      wait: false,
+      waitUntilTimeout: false,
       retryOnInterrupt: retryOnInterrupt
-    ) else {
-      preconditionFailure("TODO: Unlock should always succeed?")
+    )?.get() != nil else {
+      // NOTE: Errno and syscall composition wasn't designed for the modern
+      // world. Releasing locks should always succeed and never be blocked
+      // by an existing lock held elsewhere. But there's always a chance
+      // that some effect (e.g. from NFS) causes `EGAIN` to be thrown for a
+      // different reason/purpose. Here, in the very unlikely situation
+      // that we somehow saw it, we convert the `nil` back to the error.
+      throw Errno.resourceTemporarilyUnavailable
     }
-    return try res.get()
   }
 
+  /// Internal lock entry point, returns `nil` if blocked by existing lock.
+  /// Both `wait` and `waitUntilTimeout` cannot both be true (passed as bools to avoid
+  /// spurious enum in the ABI).
   @usableFromInline
   internal func _lock(
     _ kind: FileDescriptor.FileLock.Kind,
     start: Int64,
     length: Int64,
-    retryOnInterrupt: Bool
-  ) -> Result<(), Errno> {
-    var lock = FileDescriptor.FileLock(
-      ofdType: kind, start: start, length: length)
-    return _fcntl(.setOFDLockWait, &lock, retryOnInterrupt: retryOnInterrupt)
-  }
-
-  @usableFromInline
-  internal func _tryLock(
-    _ kind: FileDescriptor.FileLock.Kind,
+    wait: Bool,
     waitUntilTimeout: Bool,
-    start: Int64,
-    length: Int64,
     retryOnInterrupt: Bool
   ) -> Result<(), Errno>? {
+    precondition(!wait || !waitUntilTimeout)
+    let cmd: FileDescriptor.Command
+    if waitUntilTimeout {
 #if os(Linux)
-    precondition(!waitUntilTimeout, "`waitUntilTimeout` unavailable on Linux")
+      preconditionFailure("`waitUntilTimeout` unavailable on Linux")
 #endif
-
-    let cmd: Control.Command
-    if waitUntilTimeout {
       cmd = .setOFDLockWaitTimout
+    } else if wait {
+      cmd = .setOFDLockWait
     } else {
       cmd = .setOFDLock
     }
@@ -363,5 +374,6 @@ extension FileDescriptor {
       _fcntl(cmd, &lock, retryOnInterrupt: retryOnInterrupt))
   }
 }
-#endif
+
+#endif // !os(Windows)
 

Sources/System/Internals/CInterop.swift

@@ -70,7 +70,6 @@ public enum CInterop {
 #if !os(Windows)
   /// The C `struct flock` type
   public typealias FileLock = flock
-#endif
 
   /// The C `pid_t` type
   public typealias PID = pid_t
@@ -81,6 +80,7 @@ public enum CInterop {
   /// might otherwise appear. This typealias allows conversion code to be
   /// emitted into client.
   public typealias Offset = off_t
+#endif
 
 }
 

Sources/System/Util.swift

@@ -43,7 +43,7 @@ internal func nothingOrErrno<I: FixedWidthInteger>(
   valueOrErrno(retryOnInterrupt: retryOnInterrupt, f).map { _ in () }
 }
 
-/// Promote `Errno.wouldBlcok` to `nil`.
+/// Promote `Errno.wouldBlock` / `Errno.resourceTemporarilyUnavailable` to `nil`.
 internal func _extractWouldBlock<T>(
   _ value: Result<T, Errno>
 ) -> Result<T, Errno>? {
@@ -166,32 +166,9 @@ internal func _mapByteRangeToByteOffsets(
 
   let start = br.lowerBound == Int64.min ? 0 : br.lowerBound
 
-  let len: Int64 = {
-    if br.upperBound == Int64.max {
-      // l_len == 0 means until end of file
-      return 0
-    }
-    return br.upperBound - start
-  }()
-  return (start, len)
-
-  /*
-
-   let len: Int64 = {
-     if byteRange.upperBound == Int64.max {
-       // l_len == 0 means until end of file
-       0
-     } else {
-       byteRange.upperBound - start
-     }
-   }()
-
-   let len = if byteRange.upperbound == Int64.max {
-     // l_len == 0 means until end of file
-     0
-   } else {
-     byteRange.upperBound - start
-   }
-
-   */
+  if br.upperBound == Int64.max {
+    // l_len == 0 means until end of file
+    return (start, 0)
+  }
+  return (start, br.upperBound - start)
 }