Add advisory file locking API

[milseman]

When trying to resuscitate fcntl support, file locks came up again. Here I try to carve off flock support.

Some questions:

• Should it be named advisoryLock?
• Should unlock exist or should we have a 3-way enum for shared, exclusive, unlock?

[milseman]

We'll probably want to use OFD locks.

[simonjbeaumont]

• Should unlock exist or should we have a 3-way enum for shared, exclusive, unlock?

In a previous life I wrote a flock(2) wrapper which took a 3-way enum and a non-blocking flag. However, I on reflection, I think the API you have now, i.e. a separate unlock, is more reflective of usage since IIUC the LOCK_UN cannot block?

I guess the question is: how faithful/thin a wrapper do you want this to be over flock(2) vs. how much would you like to abstract away the bit flags?

[milseman]

I am definitely going to use OFD locks for the higher level API. This means that there is a getLock() which returns read/write/none. We need the 3-way enum anyways to answer the API, and unlock() can be a simple wrapper around lock(.none).

Since it's easier/better for the developer to call unlock(), we can choose names that are better geared towards getLock(). That is we can call F_UNLCK .none since that is what the constant is used for.

[milseman]

Ok, this has been switched over entirely to OFD locks.

Questions still open:

• Is getConflictingLock() the right API, or should it be something like canLock(_:FileLock.Kind)?
• For docs, can/should we link somewhere for OFD locks? Can we reduce duplication?
• Are we ready to require 5.7?
  • otherwise embrace the overloads for some RangeExpression
  • speaking of... do we need to not use some?
• Is non-blocking/non-waiting the default?
• Need to get availability hooked up

[milseman]

Also note that getConflictingLock has no atomicity guarantee between it and actually trying to set a lock, nor is there atomicity inside it's algorithm trying to find the strongest lock (though I weakly-believe that races within the algorithm could be benign).

An alternative is to drop the API and have lock() return a Bool indicating success. I.e. we handle EAGAIN in this top-level API under similar-ish reasoning to handling EINTR. EAGAIN / EWOULDBLOCK serves a somewhat analogous purpose: errno happens to be the vehicle used for fine-grained communication regarding the API, but it's not intrinsically used for "errors".

We could craft some philosophy around this as well which could extend to e.g. read under EWOULDBLOCK.

[numist]

I feel like the API here may still be more faithful to the history of file locking APIs than that history deserves.

In mutexing APIs it's common to have a try method that returns Bool immediately and a lock method that blocks until success[^opinion]; with that pair you would resolve your atomicity problems and gain the opportunity to use F_SETLKWTIMEOUT as an implementation detail in the blocking method (on platforms that support it) to get priority donation on contention.

[^opinion]: Also—more opinionatedly—lock state checking functions that always crash if the lock is not in the expected state.

[milseman]

I like the idea of os_unfair_lock_assert_not_owner, though terminating the process is a little hostile to testing. In Swift, we'd probably want something to compose with assert precondition. E.g. precondition(try myFD.ownedLock == .write) or something. We would have documentation recommending that it be used for these kind of purposes rather than program logic as it wouldn't provide atomicity with other code. I wonder if there's a name that would hint a little more strongly at this (we can also discuss that during API review).

For the lock operation, it seems there are 3 variants:

• block-til-lock
• lock-no-block
• block-til-lock-unless-timeout (Darwin only)

When you say using F_SETLKWTIMEOUT as an implementation detail, do you mean that semantics of the operation would differ across platforms? I'd lean a little more closely to having a separate API (or a boolean parameter), but I'm curious your thoughts here.

For example, we could have:

extension FileDescriptor {
    var ownedLock: throws FileLock.Kind?

    func tryLock(...) throws -> Bool
    func lock(...) throws
    func unlock(...) throws

#if !os(Linux)
    func tryLock(..., blockUntilTimeout:...) throws -> Bool
#endif    
}

Though I'm sure we'll iterate on the shape and names of those API during review. E.g. I'm not clear if the timeout taking lock should be a variant of tryLock (which matches the API shape and use site) or lock.

[milseman]

This has the API as lock/tryLock/unlock. Alternate names could be lockOrWait/lock/unlock, etc. There is no way to do ownedLock with the current syscalls, unfortunately.

This is ready for review and close to mergeable. We will need to go through some steps before the release:

1. We need commented out availability annotations
2. We'll want to do some API review for names
3. We'll want to test wait and timeouts (@lorentey ideas on how?)
4. We need to figure out if unlocking without waiting is always expected to work

[milseman]

Early draft of API doc: https://gist.github.com/milseman/fbf1ec12ac9a6635d830d9457fec0776

[milseman]

@swift-ci please test