I am getting this error in a couple of places in my code with Task closure after setting Swift 6 as Language version in XCode.
Passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure
Below is minimally reproducible sample code.
import Foundation
final class Recorder {
var writer = Writer()
func startRecording() {
Task {
await writer.startRecording()
print("started recording")
}
}
func stopRecording() {
Task {
await writer.stopRecording()
print("stopped recording")
}
}
}
actor Writer {
var isRecording = false
func startRecording() {
isRecording = true
}
func stopRecording() {
isRecording = false
}
}
While making the class Recorder as Actor would fix the problem, I fear I will have to make too many classes as Actors in the class and in my scenario, there could be performance implications where real time audio and video frames are being processed. Further, I don't see where exactly is the race condition in my code.
You have two seperate tasks that both capture self and use your writer. These functions can be called concurrently which would mean the tasks also run concurrently. And that's where your race condition is.
You don't have to make Recorder an actor. If you can make writer a let constant then you could mark the Recorder as Sendable which should also solve your issue in this case.
Keep in mind that the compiler will flag any possible data races without checking that your code will cause data races at runtime.
As @donny_wals said, the problem is the implicit capture of self. Since it happens implicitly (you don't write await self.writer...) it's admittedly less obvious.
A quicker fix than using a let constant and making RecorderSendable (which might be hard in your actual code) is simply only capturing what you need with an explicit capture list, i.e. in this case write Task { [writer] in for your closure. Since Writer is an actor it can safely be passed to the new execution contexts for the closures.
Small thing that's perhaps only relevant to the example here: Of course should you switch the writer (as it's a var) in between calls to the start and stop methods, you might miss a call to stop (assuming they should also be paired).
It looks like there are myriad of Sendable errors one might face. Just modifying the above code as follows gives another set of errors which are harder to fix:
import Foundation
final class Recorder {
var writer = Writer()
var foo = 5
var isRecording = false
func startRecording() {
Task { [writer] in
await writer.startRecording()
print("started recording")
}
}
func stopRecording() {
Task { [writer] in
await writer.stopRecording()
print("stopped recording")
}
}
func observeValues() {
Task {
// Await new capture activity values from the capture service.
for await value in await writer.$isRecording.values {
isRecording = value
}
}
}
}
actor Writer {
@Published private(set) public var isRecording = false
func startRecording() {
isRecording = true
}
func stopRecording() {
isRecording = false
}
}
Non-sendable type 'Published<Bool>.Publisher' in implicitly asynchronous access to actor-isolated property '$isRecording' cannot cross actor boundary
I'm not particular familiar with how @Published works and how/if it can be made to work nicely with concurrency. I have used similar patterns, however, with AsyncStream and that's certainly an option.
However, stepping back, I want to return to the Recorder. This is a 1) non-Sendable type that is 2) participating in concurrency (using Task). You can make this work, but I urge caution. Types like this are really hard to use unless you are extremely familiar with how Swift concurrency works.
For starters, because these methods are non-isolated, there are no ordering guarantees.
It is 100% possible that the stop occurs before the start.
You can overcome these problems, but it's going to require some significant work. Perhaps you can push the Task creation further up the stack and make these calls async?
Or, could it be this type is directly connected to your UI? That might allow you to make this @MainActor, which would make all these problems go away. It would first establish static isolation to give you better ordering (with a Swift 6 compiler) and would also make the type Sendable.
It seems your Recorder is designed to be callable in any context, and it wants to synchronize some state from an actor.
I would argue you should make RecorderSendable in the first place, you can achieve that using synchronization primitives.
As for the $isRecording problem, since $isRecording.values has type AsyncPublisher, which is not Sendable, it is not safe to send it across the domain of Writer. For this I would want to encapsulate the observation logic into a member method. After all, the users of Writer does not need to know anything about Publisher or AsyncPublisher, they just want to get notified of Bools.
final class Recorder: Sendable {
let writer = Writer()
struct State {
var foo = 5
var isRecordingCache = false
}
struct TaskHandles {
var recordingObservation: Task<Void, Never>?
}
// I choose `Mutex` in this example. feel free to use other constructs to implement `Sendable`
let state = Mutex(State())
let secondaryTasks = Mutex(TaskHandles())
func startRecording() {
Task {
await writer.startRecording()
print("started recording")
}
}
func stopRecording() {
Task {
await writer.stopRecording()
print("stopped recording")
}
}
func observeOnWriter() {
let observationTask = Task {
await writer.observe { value in
state.withLock {
$0.isRecordingCache = value
print("value incoming")
}
}
}
secondaryTasks.withLock {
$0.observationTask = task
}
}
func cancel() {
secondaryTasks.withLock {
if let recordingObservation = $0.recordingObservation {
recordingObservation.cancel()
}
$0.recordingObservation = nil
}
}
}
actor Writer {
@Published private(set) public var isRecording = false
func observe(_ observer: (Bool) -> Void) async {
for await value in $isRecording.values {
observer(value)
}
}
func startRecording() {
isRecording = true
}
func stopRecording() {
isRecording = false
}
}
Of course, the best way is to utilize async/await upper in your call chain, as mattie has suggested.
If you plan to use concurrency, then think this fear is unreasonable, actors are quite performant. Was testing some time ago and as I remember the only thing faster were Mutex implementation, which btw you could also use, especially if you don't wanna land to early in concurrency.
Yes, I hear OSAllocatedUnfairLock is fastest and I could use it for small piece of code that is performance critical. But what I hear from you is that having 2-3 actors processing and passing data among themselves is not much of an issue.