I have set Strict Concurrency Checking to Complete, and the following code compiles without warnings in Xcode 15.0.1 and Xcode 15.1 beta 3.
When running it, it shows a concurrency problem. The inc() method is allowed to run on two tasks at the same time.
My questions:
Task in run(). But maybe the non-MainActor inc() method should not be synchronously callable from the MainActor.class Counter {
private var counter = 0
func inc() -> Int {
let v = counter + 1
for _ in 0...1000 {}
counter = v
return v
}
@MainActor
func run() {
Task {
while true {
try? await Task.sleep(for: .seconds(1))
let v = inc()
print("xxx t1", v)
}
}
}
}
class Experiment {
func start() {
Task {
let counter = Counter()
await counter.run()
while true {
try? await Task.sleep(for: .seconds(1))
let v = counter.inc()
print("xxx t2", v)
}
}
}
}
I tested this code with the current Swift 5.10 snapshot and the line await counter.run() now triggers the warning "Passing argument of non-sendable type 'Counter' into main actor-isolated context may introduce data races"
It looks like this PR fixed it: https://github.com/apple/swift/pull/67730
You asked:
- Am I correct to assume that this should not compile without a warning?
I agree. I would have expected a compile-time warning. This code is not thread-safe. Counter is not Sendable and is being captured within a @Sendable closure.
FWIW, the behavior I experience (in Xcode 15.0.1 and 15.1 Beta 3) is as follows:
If I remove the @MainActor isolation on the run method, I receive the appropriate compile-time error when using the “Strict Concurrency Checking” build setting of “Complete”:
Capture of 'self' with non-sendable type 'Counter' in a '@Sendable' closure.
The “Strict Concurrency Checking” build setting controls the compile-time warnings that you will receive. It primarily is checking that objects crossing task boundaries are Sendable or not. (For those unfamiliar with these issues, the WWDC 2022 video Eliminate data races using Swift Concurrency is a good primer.)
So this warning is correct: Counter is non-sendable. This code is not thread-safe.
But if I restore the @MainActor isolation of run, as shown in your original example, the compiler fails to produce the relevant warnings (even though Counter is no more thread-safe than above; it is still non-sendable).
This having been said, you reported runtime errors. I only manifested runtime errors when I turned on Thread Sanitizer (TSAN). As an aside, this is a reason why the compile-time warnings of “Strict Concurrency Checking” are normally much better than runtime checking: Many thread-unsafe behaviors are not consistently manifested at runtime. The “Strict Concurrency Checking” can detect thread-safety mistakes that might be hard to manifest at runtime.
But getting back to your question, it is unclear why the compiler fails to detect that Counter is non-sendable in the presence of the @MainActor isolation of only the run method. Isolating just this one function makes Counter no more thread-safe than it is without the @MainActor isolation. (In defense of the Swift compiler authors, all of this sendability checking code must be pretty complex.)
You went on to ask:
- Which part of this code is illegal?
Regardless of whether run is isolated to the main actor or not, the problem is that this code is simply not thread-safe. Counter is not Sendable. It exposes the inc function which is mutating a non-isolated property, which means that while the task initiated by run is executing, it is possible for another thread to call inc in parallel. (Or, multiple threads could call inc simultaneously, regardless of whether run was ever called or not.) This code is not thread-safe.
There are a number of ways of fixing this code. We want to make Counter a Sendable type. We could isolate the whole Counter type to @MainActor (or any global actor). Or we could this class an actor, instead. Or you could add your own synchronization and mark this as @unchecked Sendable (but the burden for the correctness of the thread-safety now rests on your shoulders and the compiler will be unable to verify this). But, as it stands, Counter is not thread-safe.