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:
- Am I correct to assume that this should not compile without a warning?
- Which part of this code is illegal? I think it's starting the
Task
inrun()
. But maybe the non-MainActorinc()
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)
}
}
}
}
Update 2023-12-04:
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:
I agree. I would have expected a compile-time warning. This code is not thread-safe.
Counter
is notSendable
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 therun
method, I receive the appropriate compile-time error when using the “Strict Concurrency Checking” build setting of “Complete”: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 ofrun
, as shown in your original example, the compiler fails to produce the relevant warnings (even thoughCounter
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 therun
method. Isolating just this one function makesCounter
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:
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 notSendable
. It exposes theinc
function which is mutating a non-isolated property, which means that while the task initiated byrun
is executing, it is possible for another thread to callinc
in parallel. (Or, multiple threads could callinc
simultaneously, regardless of whetherrun
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
aSendable
type. We could isolate the wholeCounter
type to@MainActor
(or any global actor). Or we could thisclass
anactor
, 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.