Sending a var risks causing data races

I have the following code:

actor Pipeline {
    let updater = Updater()
    
    func startUpdates() {
        updater.startUpdates()
    }
    
    func stopUpdates() async {
        await updater.stopUpdates() //Error: Sending 'self.updater' risks causing data races
    }
}

final class Updater {
    
    func startUpdates() {
        print("started updates")
    }
    
    func stopUpdates() async {
        print("stopped updates")
    }
}

While I can fix the error by marking Updater as Sendable, I will not be able to add member variables to this class. I also tried adding nonisolated to updater but it gives error again:

 ` nonisolated let updater = Updater()`

   `'nonisolated' can not be applied to variable with non-'Sendable' type 'Updater'

Updater either needs to be Sendable and use Mutex (or similar) for data storage or it needs to be an actor.

1 Like

Because Updater is not sendable, it does not guarantee that it is safe to use concurrently.

By default, if you mark Updater as Sendable, you are limited to doing things that the compiler can prove are safe all by itself. For classes, it's pretty severe; for instance, you cannot add any mutable data members, of any type.

final class Problem: Sendable {
  var x: Int = 0
      `- error: stored property 'x' of 'Sendable'-conforming class 'Problem' is mutable
}

That doesn't mean you can't do it! You have a couple of options:

  1. Convert the class in to an actor. This will automatically ensure accesses to your mutable data are serialised. The compiler knows this - in fact, you don't even need to mark actors as Sendable because they just inherently are.

  2. Perform your own serialisation/locking. If you mark the updater as @unchecked Sendable, you will not be restricted to things the compiler can prove are safe -- of course, this means you are responsible for proving that the type is thread-safe.

2 Likes

I think there's more nuance here since the example, as given, doesn't have any mutation in the first place.

Fundamentally, it's due to the fact that Updater's async work isn't isolated, so the call to stopUpdates() is performed on the default executor rather than the Pipeline context you may expect. Since Updater itself isn't Sendable, trying to send the updater instance from the Pipeline context to the default context will fail. That's in addition to any future mutation that may make the instance more unsafe. There's been some discussion of making non isolated async funcs default to using the current context, which I think would fix the original issue.

3 Likes

Very generally speaking, I think trying to make reference (class) types Sendable is a trap. It's tempting because it can make some immediate problems go away. But as you are seeing it can be quite hard to pull off.

And that will lead you down the path of trying out actors. Which also fixes some immediate problems. But, now all the types inputs and outputs have to be Sendable and the interface must now be async.

If you can solve this problem by removing concurrency from the system, that's definitely what I would recommend.

(What @Jon_Shier says is true, there is on-going to work to try to make this less problematic. However, this is a long-ways off.)

3 Likes

@mattie By removing concurrency, you mean getting rid of Swift Concurrency and using other concurrency tools such as mutex?

1 Like

Sorry I wasn't nearly clear enough. I meant reduce the amount of concurrency constructs in use. For example, I have encountered many situations actors are used when really a system would be much easier to model with MainActor state and just a handful of non-isolated async functions to shift some work to background threads.

I don't see enough of your project here to know if that's a good idea or not, but it's something to think about.

Of course you could also use mutexes/queues or other mechanisms. Those do still work.

3 Likes

This should also help once it lands: [Pitch] Inherit isolation by default for async functions.

2 Likes

I do not dislike the idea of "inheriting isolation by default", but I wonder if that can help in this situation.

Even though one day func stopUpdates() async can be called without changing isolation domain, the concurrency problem does not go away. Because practically, this function will probably need to access some shared mutable state stored in Update. At the end of the day, the developer still need to think about how to make Updater safe to use concurrently.

@dsharma you can already achieve the same semantics manually, if that's what you want:

final class Updater {
    func stopUpdates(_ isolation: isolated (any Actor)? = #isolation) async {
        print("stopped updates")
    }
}

actor Pipeline {
    let updater = Updater()
    func stopUpdates() async {
        await updater.stopUpdates() // This is OK now, but remember under this way you are still not free to access updater in random contexts
    }
}

2 Likes

Yeah, that works great. I see there are plenty of options and opinions to solve a basic problem. But as you said in the code comment that stopUpdates should not be called from random contexts, I tried the following and it still compiles (without an actor).

final class AnotherPipeline {
    let updater = Updater()
    
    
    func startUpdates() {
        updater.startUpdates()
    }
    
    func stopUpdates() async {
        await updater.stopUpdates()
    }
}

final class Updater {
    
    func startUpdates() {
        print("started updates")
    }
    
    func stopUpdates(_ isolation: isolated (any Actor)? = #isolation) async {
            print("stopped updates") // This is OK now, but remember under this way you are still not free to access updater in random contexts
        }
}

Could you please explain a bit more what exactly is going on when you mark the function as

 `func stopUpdates(_ isolation: isolated (any Actor)? = #isolation) async `

I can't find much documentation about this syntax in the language but is it just statically erasing type information so that it compiles but flag raise issues in the runtime?

This is a feature brought by SE-0420.

1 Like

Is it not possible to make whole class initialiser isolated to a particular isolation domain? In other words, if a class has an init method with #isolation parameter, then all the methods and properties of that class instance be automatically isolated to that domain.

I don't think Swift's type system supports value-based isolation nowadays. However, if you can model your isolation as a globally shared one, then it's appropriate to make the actor a @globalActor.

1 Like

You are right that calling an async function through inheriting isolation domain of the caller does not solve anything as far as concurrency is concerned. Calling function like stopUpdates on a member variable of an Actor (which is not an Actor or confirms to Sendable) is an issue.