Lock implementation in the Swift Async Algorithms package

Would it make sense to create a separate package with the lock implementation here?

This is code that I've implemented may times (almost all incorrectly, and none portably) and is tricky to get right in general. I don't know what the long-term plans for locks are in the standard library, especially to do with correctness wrt how to check at the compiler level that State doesn't escape in withCriticalRegion, but it seems like a useful snippet to reuse. I guess the license would allow anyone to copy the code and make it public, but it feels like it might be something that could benefit from official Apple support, especially for migration for when something like this appears in the standard library.

Would releasing the file as a separate package be on the cards or should I just copy the snippet for now?

I think it's a good idea to let the compiler and standard library lead the way on the API for this particular type. It'll certainly need to tie in to the overall concurrency story, and maybe even deserves special features to make it easier to use. For now it feels a lot less risky to make the type internal to packages where it's needed instead of encouraging it directly as a public API.

3 Likes

The only caveat to that type is that State must be structural - letting it escape is fine because the structure is inherently just a value type. The only other use potential for that type is to add a unique reference check on the managed buffer.

Per homing; there are obviously a few potential packages that it could live in; swift-system (but that does not seem thematically accurate), swift-atomics (perhaps a slight misnomer to live there), Platform (but again does not seem thematically correct), _Concurrency (a bit of an odd duck there since it is the antithesis of an actor), and finally the standard library (perhaps the most "correct" in my view of homes).

Particularly with the APIs like withTaskCancellationHandler needing a sendable but NON async function to handle cancels it makes this approach pretty attractive. Many of the places this is used are to account for cancellation. It does perhaps beg the question: do we need to adjust cancellation to make this less attractive (and perhaps letting other more approachable APIs take center stage)?

Though in the end - it has considerable usability outside of cancellation when dealing with thread/queue based concurrency. I know this has come up before in discussion.

1 Like

Wrapping an UnsafeMutablePointer into a struct and then having to manually call deinitialize seems like a really bad practice. They should be using a class. (Or if there are performance reasons for that, there should be a comment about that in the code)

I have a ReadWriteLock version of this, it is tested on Apple platforms and Ubuntu: TypedNotificationCenter/ReadWriteLock.swift at edb50239dd9bd82cc5237566d789a930ecb5159d · Cyberbeni/TypedNotificationCenter · GitHub

It only had a slight performance increase in the "read paralelly" test in my case and had the same performance as NSLock in the other cases.

1 Like