Normative: Add Mutex and Condition

[syg]

Uses the reusable token-based design from #27

cc @bakkot

[bakkot]

Quick skim of the mutex API mostly looks good, some comments. Didn't review memory model stuff at all.

[bakkot]

I expect we'll eventually make tokens disposable, which makes the common pattern for timeouts look like

using token = mutex.lock(null, timeout);

Returning a string here means that will throw, which is... probably the thing you want? It'll be kind of a cryptic error message, but that's better than thinking you have the lock when you don't (using token = null is legal, so returning null here would make that silently succeed).

[rbuckton]

Agreed, we definitely shouldn't mix return types here. If the method returns an UnlockToken, it shouldn't also return a string. I'd much rather we throw in the event of a timeout.

[syg]

I'm against throwing because:

1. A timeout isn't an exceptional thing though, but one of the expected uses of the proposal.
2. The method can throw for parameter validation reasons. Are you asking users to check instanceof? Do string comparison on the error message?

That said the string thing is not the cleanest, but I don't know how else to express it. The constraints are:

• We need a distinguishing timeout value
• We can't allocate a result object on every call

Open to ideas.

[syg]

In conjunction with the wait and waitFor split below, I've made Mutex.lock return UndefinedToken when the lock is acquired, or null when timed out. WDYT?

EDIT: Oops, null is pretty bad as this has the problem @bakkot pointed out above, which is that if you couldn't acquire the lock, you don't error out of the using, since using foo = null is fine, so you go to the next line without having acquired the lock. Maybe it should return false?

[bakkot]

Yeah, null has this problem. The options I see are:

• stop using tokens
• return null or the token and have this footgun
• make tokens not disposable
• allocate an additional object so you can do { success: boolean, token?: UnlockToken }
• have some static frozen placeholder object, like Mutex.TIMED_OUT = Object.freeze({}), and return that
• return a string and just live with the mixed return type

Of these, while I share @rbuckton's distaste for the mixed return type, I think the last is the best option.

[rbuckton]

Sidebar: I have no idea what set.addIfNotPresent is supposed to do. "Add if not present" is how set.add already works.

This is moving further off topic so I'll summarize and we can discuss offline if necessary. What I'd want is a tryAdd(value: T): boolean that does the work of .has() and .add() in one step. I've always found it unfortunate that Set.prototype.add() returns this rather than something informative such as whether the value was added (true) or was already present (false), given that my most frequent uses for Set involve this pattern.

[syg]

tryLock could be used on the main thread even with a timeout

Only if the timeout is 0, non-0 clamped timeouts are too hard to do IMO for the same reasons that I abandoned it in Atomics.pause.

I'm going with the string-returning version for the spec draft. Please open an issue for the alternative and let's hammer it out during Stage 2.

[syg]

Chatted through with Ron and understand better where he's coming from: that resource-returning methods being conditional was always part of the explicit design of using. I'd like to hear committee's thoughts on the footgun here. Given Ron's a co-champion, I'll put return null for the timeout case and call it out as an open question.

[bakkot]

null is fine for resources where you are going to use them after acquiring them, like files. null is not fine for anything where the purpose is acquisition and release, like mutexes and other guards. A lock on a mutex is not a resource.

[syg]

I see the footgun, let's discuss it in plenary.

[bakkot]

Do we want tryWithLock? Not sure what to return in the already-locked case, though. Maybe have a parameter to use for that value?

[syg]

I don't see that often in other PLs/stdlibs. I'm not against it though.

[bakkot]

Swift 6 (new as of 2 days ago) has withLockIfAvailable¹, which is this. I also think that's a better name than tryLock / tryWithLock, incidentally.

Really it's just filling out the missing quadrant on the (lock()+unlock() vs callback) x (blocking vs only-if-available) matrix, though, so it's pretty natural if we already have the other three quadrants filled out.

Footnotes

1. You can read through its history in this thread if you want to. ↩

[rbuckton]

While I generally prefer lock/tryLock for both the callback and instance APIs, if we are going to differ for the callback taking API my preference would be withLock/tryWithLock to maintain some symmetry. I generally prefer to continue to use try for the convention of "do this action only if it would succeed", which I'd like to extend to other places in the language eventually (i.e., Set.prototype.tryAdd).

[rbuckton]

Not sure what to return in the already-locked case, though. Maybe have a parameter to use for that value?

It might be best to throw if you try to reenter a mutex locked on the same thread as it is a fairly decent indication of a potential deadlock. Reentering a non-recursive mutex is a bad practice, and if we at some point want to introduce a recursive mutex we don't want users to establish bad habits early on.

In the tryWithLock case we'd probably want to return a value indicating the lock was already owned rather than invoking the callback to maintain symmetry.

[syg]

In the tryWithLock case we'd probably want to return a value indicating the lock was already owned rather than invoking the callback to maintain symmetry.

I didn't add tryWithLock (whatever its name) because I couldn't figure out how to do this. I suppose we can return a distinguishing value if we always discard the return value of the callback, instead of trying to return that.

[rbuckton]

I didn't add tryWithLock (whatever its name) because I couldn't figure out how to do this. I suppose we can return a distinguishing value if we always discard the return value of the callback, instead of trying to return that.

Isn't that what Mutex.tryLock did in the dev trial? It always returned a boolean, so if you needed a value from the callback you would have to use a closure.

[syg]

Yeah, but I got some feedback that was annoying.

[rbuckton]

This is a case where returning an object with a { success, result } object might make sense. If we have a lock-token producing tryLock, then maybe a callback version isn't quite as necessary? If you are conditionally locking then you are already buying into more complexity anyways.

[syg]

I've waffled on this and think we shouldn't propose withLock for the initial proposal, as the unlock token design means the callback-taking approach doesn't add that much value. My thinking is the main value of withLock is you don't have to think about locking or unlocking operations, but with unlock tokens, the simple version doesn't compose well with condition variables. If you want to wait for a condition inside the callback, that means the callback needs to be passed an unlock token.

[bakkot]

Presumably lockAsync to be added later?

[syg]

Yeah, async variants are pretty complex, and TBH I'd prefer to do them as a follow-up proposal entirely.

[bakkot]

The token use for Atomics.Condition.wait seems fine for now, but if tokens become shareable across threads, or we add a Condition.waitAsync, we'll need to think more about what happens if someone else touches the token during the wait.

[syg]

The token use for Atomics.Condition.wait seems fine for now, but if tokens become shareable across threads, or we add a Condition.waitAsync, we'll need to think more about what happens if someone else touches the token during the wait.

Absolutely. The current version only works for blocking waits.

[rbuckton]

While I generally prefer lock/tryLock for both the callback and instance APIs, if we are going to differ for the callback taking API my preference would be withLock/tryWithLock to maintain some symmetry. I generally prefer to continue to use try for the convention of "do this action only if it would succeed", which I'd like to extend to other places in the language eventually (i.e., Set.prototype.tryAdd).

[rbuckton]

I contend that tryLock is a better name for this and is a fairly common idiom across multiple PLs (C++, C#, Java, Go, Rust, etc.). tryLock is shorter, and "Available" is moderately harder to spell and thus very easy to typo outside of an editor with completions.

[rbuckton]

Not sure what to return in the already-locked case, though. Maybe have a parameter to use for that value?

It might be best to throw if you try to reenter a mutex locked on the same thread as it is a fairly decent indication of a potential deadlock. Reentering a non-recursive mutex is a bad practice, and if we at some point want to introduce a recursive mutex we don't want users to establish bad habits early on.

In the tryWithLock case we'd probably want to return a value indicating the lock was already owned rather than invoking the callback to maintain symmetry.

[rbuckton]

Agreed, we definitely shouldn't mix return types here. If the method returns an UnlockToken, it shouldn't also return a string. I'd much rather we throw in the event of a timeout.

[rbuckton]

We shouldn't mix UnlockToken and string return types here.