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]

It's been many years since I wrote a flock(2) wrapper, but in those tests I ended up shelling out to flock(1) as an interop test on Linux. I guess we won't be able to do anything like that in this project?

What do you have in mind for this test?

We could potentially at least check the lock-lock, lock-unlock, and lock-upgrade flows?

Given that two independent file descriptors for the same underlying file can still block each other there are some tests we can write in a single process.

[milseman]

I think we should pursue OFD locks which would be easier to check. Two separate open calls in the same process should have different lock sets. When we get fork support we can also test some behavior there.

[simonjbeaumont]

I appreciate this is as specific as man 2 flock gets and its implicit that the upgrade only works on the same file descriptor I suppose, i.e. if a process has two independent file descriptors to the same underlying file, they can still block each other.

[milseman]

I believe flock(2) would be associated between process/file, while OFD locks would be file-description associated (but not descriptor, i.e. dup would share locks).

[simonjbeaumont]

We may already be on the same page here, but I believe that, with flock(2), using dup(2) will allow the new fd to upgrade a lock, but using open(2) will not.

Here's a toy program that accepts NEW_FD_METHOD as an environment variable and attempts to upgrade a shared lock to an exclusive one using a new file descriptor pointing to the same file as was already locked:

https://gist.github.com/simonjbeaumont/7bd12f1f97e162734b6fec847d3a969f

Running it with both options and dup is fine, open fails:

❯ clang -o flock-test flock-test.c && NEW_FD_METHOD=dup ./flock-test

❯ clang -o flock-test flock-test.c && NEW_FD_METHOD=open ./flock-test
error obtaining exclusive lock with new fd 4: Resource temporarily unavailable

[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¹; 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.

Footnotes

1. 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