While I think that it's fine to force Atomic clients to use @unchecked Sendable as additional cue to really think about the implications of the atomic operations, I don't find this example compelling:
This feels like the same as something like this:
actor AccessCounted {
private var counter: Int = 0
func doSomething() async {
let counter = self.counter
await ...
self.counter = counter + 1
}
}
There is at least a visual cue with the await that there could be a suspension point the developer has to worry about and we don't have an equivalent for atomics, but I don't think there's ever going to be a way to eliminate every race that arises because of developers not thinking about the concurrency implications of their code.
Async locks are a real and reasonable thing ā Swift's actors are essentially async locks. They're just not a total replacement for sync locks.
I agree with Jordan that making Atomic non-Sendable seems like a step too far. Atomic will always be a feature for experts, but once someone has opted in to that, adding extra obstacles to its use doesn't seem like a good idea.
I do think we should require the argument type to be unconditionally Sendable, though.
Can't speak for Jordan as most of my "knowledge" regarding composability derives from using Xcode (in this context). Otherwise, I believe that UnsafeAtomic is in line with UnsafeContinuation with regards to memory safety.
A good chunk of the use of atomics is for lock-free data structures, though, and those will probably make heavy use of Unsafe*Pointer types by their nature, which aren't Sendable.
These objections match the thinking that led to swift-atomics originally marking its constructs conditionally Sendable.
It is indeed quite possible that this was the right choice.
I'm raising this as an issue though, if only to force us to clarify what Sendable means in this context. In the post that introduced the bad AccessCounted implementation, I pretended this question was already settled:
This code has a race condition by most definitions of this term. However, it never engages in undefined behavior: while it can produce incorrect results, it still operates entirely within Swift's abstract memory model. Sendable checking does not protect against logic bugs like this -- atomic operations can be tricky to use correctly, even in the "simplest" use cases.
However, it isn't quite clear to me that this has been previously covered. SE-0302 talks a lot about "the safety of passing data between concurrency domains", but I can't remember if I saw a clear definition of precisely what that means.
Do we consider it safe to pass borrows of AtomicCountBy2 between concurrency domains, even if doing so can lead to (high-level) race conditions?
The construct does avoid undefined behavior, which is already a quite strong guarantee -- is it okay to consider that our official litmus test, then?
If we do consider this safe, then we can allow the Sendable conformance to be automatically inferred, as usual, and so it seems technically okay to allow struct Atomic to be conditionally Sendable.
If we consider such defects to violate sendability, then we'd need to force developers to spell @unchecked Sendable whenever they want to use atomics. But that would still reintroduce the same potential issue one level higher on the stack! In effect, we'd just be training developers to mindlessly use @unchecked Sendable without addressing the core issue, which is that conventional assumptions about composition are fundamentally incompatible with concurrency. This does seem unpalatable.
There is a third possibility though -- that we accept the idea that Sendable is "only" supposed to prevent undefined behavior, but we still refrain from declaring struct Atomic conditionally Sendable, as a judgment call.
In particular, do we really want to allow types that rely on memory orderings other than .sequentiallyConsistent to be implicitly inferred as Sendable? (Keep in mind that it is difficult to make good use of the semantics of acquiring/releasing ordering in such implicitly sendable contexts (as mutating access to other components is disallowed), and, worse, as I understand it, the meaning of relaxed ordering isn't currently well-defined, and that makes it difficult to formally reason about the behavior of code that uses it.)
Thereās no use for an atomic that isnāt accessed concurrently. Thatās not the same thing as access from multiple Swift actors. Not all concurrency is not Swift concurrency, and therefore not all concurrency-safe types are Sendable.
Rust does not expose a generic Atomic type like the one we're proposing, instead they have stamped out things like AtomicUsize, AtomicBool, AtomicPtr, etc. for your favorite integer size / signedness. Thus, all of these are both Sync and Send. AtomicPtr is generic over T, but is unconditionally both Sync and Send.
The Rust argument for why it isn't a "hole" is because once you extract the raw pointer from the AtomicPtr, all the operations that could violate thread-safety (loads/stores/deref) still require you to write unsafe, so you've still declared it's Your Fault if something is messed up with the data/logic that got you there.
Similarly it's "fine" if someone only ever uses Relaxed loads/stores on their AtomicUsize, because, hey, it's Just An Integer. It can't do anything dangerous unless someone writes unsafe, and then they're taking responsibility for all the dataflow and logic that got to that block. :)
I think Swift is definitely aiming for āall concurrency is Sendable-checkedā, though, even with classic thread or task queue models.
EDIT: But if we expose the address of an Atomic, it could be passed to non-Swift code without going through Sendable. So yeah, my initial statement was too strong.
We can never fully protect against misuses of an API, and some things are just inherently complex and their use requires great care. We try to design and document APIs such that they guide developers to correct usage, but we can't make anything entirely foolproof.
String, for instance, attempts to provide an easy-to-use interface for Unicode text. It is still possible to mess it up. For instance, a recent discussion surfaced the following example from the Vapor framework:
Canonically-equivalent-to-the-grapheme-cluster-"/" is not the correct way to process a path string, which should be split on the ASCII character U+002F SOLIDUS.
Floating-point numbers are another example: they often look like decimal numbers, but they don't work that way. They're generally fine if you're computing geometry to render on-screen, but you wouldn't want pension fund calculations to be subject to floating-point inaccuracies.
In this case, AtomicCountBy2 fundamentally has misunderstood atomics: despite the object model inferring that atomicity is a property of memory locations, in fact it is a property of operations. Fundamentally, atomic operations do not compose -- the combination of 2 atomic operations is not, itself, atomic. If the developer of AtomicCountBy2 was trying to make their increment() method atomic, they used the wrong API calls for that.
Once you think of atomicity at the level of operations, it is clear why this happened, and what the solution is. "Atomic" comes from the Greek Atomos, meaning indivisible. If you an operation can be split in to constituent operations, clearly it cannot be atomic.
I don't think it's possible for Sendable to protect against high-level bugs like this, just as we can't in general protect against people using the wrong String view or the wrong numeric type/operation.
It may be possible, however, to introduce some kind of macro which validates that APIs which wrap atomics truly only perform a single atomic operation along each path.
And while traditional locks shouldn't generally mix with async code and actors, it does seem like atomics could potentially be useful implementation details for actors too, since you could use an atomic to implement nonisolated but atomically-updatable storage in limited situations, like if you wanted to implement a read counter for some otherwise-immutable field without forcing a full context switch to the actor on every access:
actor CountedReader {
private nonisolated let _value: String
private nonisolated let _count: Atomic<UInt>
nonisolated var value: String {
_count.wrappingAdd(1, ordering: .relaxed)
return _value
}
nonisolated var readCount: UInt {
return _count.load(ordering: .relaxed)
}
}
With a bit less than a week left in this round of review, I wanted to re-prompt some of the other naming and organizational concerns introduced by this revision, in order to ensure the Language Steering Group has adequate community feedback:
In their review decision, the steering group had offered the name Atomics to contain the new APIs. This revision instead uses Synchronization, both to allow for additional synchronization-related APIs to be placed here, and to allow for the new standard API to coexist with the existing Atomics module defined by the swift-atomics package in use today. Are there other interesting names to consider for the module?
The language steering group had also suggested renaming the AtomicValue to AtomicWrappable to more strictly follow the API naming guidelines, but the revised proposal sticks with AtomicValue in order to reduce friction for code migrating from swift-atomics.
Should the WordPair type go into the Synchronization module along with atomics? If it goes into the other module, should it be renamed to DoubleWord as used in a previous proposal? One justification for the renaming to WordPair was to keep DoubleWord from showing up in code completion when developers reach for the much more common type Double, which would be less of an issue if the API must be explicitly imported. Going back to DoubleWord would again reduce the friction in moving from the APIs in the swift-atomics module to the new standard APIs.
There is a bit of a running theme through these of a tension between maintaining compatibility with the existing swift-atomics package as much as possible, and reducing the friction of migrating code to the standard library atomic types when they become available, versus taking the opportunity of promoting them to the standard library to also select "better" names. It'd be good to get the community's perspective on this tradeoff, particularly from users who have experience with the existing swift-atomics package.
Is it possible to put out a new release of swift-atomics that just aliases the new types/operations under the old names, as a transitional thing? That could tip the scales towards ābetter long term namesā if we actually think theyāre worth changing, though thereās still the ātutorial invalidationā downside.
I dislike AtomicWrappable because it implies an extra storage cost. If an alternative to AtomicValue must be found, how about AtomicWritable, AtomicAccessible or AtomicStorable?
Since code is more often read than migrated, I think distinct names would be a benefit, to avoid confusion.
It's also confusing to associate these with swift value types (since not immutable), or to think of them as wrappers (since inline) or even to think of them as storage modifiers, since it's the operations that are atomic.
So how about AtomicAccess, to emphasize the operations being atomic, rather than the data?
On Synchronization:
I think the key distinction here, particularly given the existing Apple-supported atomics library, is that this is now in the standard library (as an added module). It would be nice to have some distinct naming convention for all such standard library modules, even if just Std/Stdlib (like AtomicStd or Swift (like SwiftAtomics). Most projects have a policy preferring stdlib's over other mechanisms, and this would be a clear way to signal that at the import level.
As for bundling atomics in a broader Synchronization module: yes there may be version dependencies where a new atomic operation or ordering makes possible a new lock form, but otherwise I would guess SwiftAtomics changes slowly and rarely relative to other sync features -- at the measured pace of a stdlib. However, many synch features not needed by stdlib but actively used by key projects would want a faster version cadence, driven by consumers.