Preventing Data Races in the Swift Concurrency Model

[Chris_Lattner3 (Chris Lattner)]

@Douglas_Gregor

@Chris_Lattner3

I'm really happy to see embracing something like the ActorSendable idea. I agree with your premise that we should find a better name than ActorSendable for this concept, given that it should be applies to structured concurrency as well. I'm not sure that Sharable is descriptive enough though. Would you be open to a longer name like ConcurrentlySharable or something else to scope this and make it more obvious that this is a specific kind of sharing?

Yes, I'm open to longer names to make things more clear here. In my experimental implementation of this idea I ended up going with ConcurrentValue , because the fundamental property here is that values of the type can be used concurrently. I think @Joe_Groff's formulation as being about reachability to unsynchronized shared mutable state is the thing to capture.

Cool, ConcurrentValue sounds great to me!

@Douglas_Gregor

I disagree fairly strongly with the main direction of that paper,
...
Do you want to debate that point further, or are there other models you want to discuss?

Yes I absolutely do, but I think there are several other parts of the model that are worth converging on first (which also have nothing to do with global variable semantics), so I suggest we focus effort there and come back to this topic.

@Douglas_Gregor

@Chris_Lattner3

1. They break our API evolution and resilience model: it is important to Swift evolution that we can turn stored properties into computed properties (and visa versa) without affecting API. However, Swift doesn’t have computed let properties. This is a major reason why Swift doesn't have other "let-only" behavior in its design - we want everything to tie nicely into the resilience model.

This is incorrect. You can evolve a let into an @actorIndependent var without breaking API or ABI.

Ok, the confusion is that @actorIntependent hasn't come up in this particular thread.

To recap the other threads, I have not seen clear motivation on the design of @actorIntependent. My understanding is that it has something to do with wanting actors to conform to protocols with sync requirements. If that is the case, then we need to discuss the topic and possible solutions, because there are better (in my opinion) ways to solve that problem.

However, it is entirely possible I've misread the goals here (again, because I haven't seen the motivation). In any case, we need to discuss this further.

@Douglas_Gregor

I've been writing code against this model, and it is so very, very useful in practice. This current pitch on Preventing Data Races takes away the special case from actors and makes all let 's with ActorSendable/ConcurrentValue/Shareable/wathever types accessible concurrently.

I'd love to know more about the use cases, because it has simply been presented as "the design" without motivation in the proposal. I offered to write up a doc exploring the issue but you requested that I not do that. I gave a short version in response. I haven't had a chance to get back to that thread, so it is on me I guess, but I'd prefer to see actual rationale in a doc that explains @actorIndependent so I'm not trying to understand and discuss a moving target.

In any case, the entire design of actors + sync requirements has alternate designs that haven't been evaluated (in the forums). I don't see it as a forgone conclusion that @actorIndependent is a good design, and if it doesn't stick, then the let resilience / API migration issue is a very important concern. I suggest that we split this issue out to its own discussion thread.

@Douglas_Gregor

@Chris_Lattner3

In short, I'm not sure why initialization-only data is worth special casing like this: it's not very expressive to only allow cross-actor/task sharing of init-only data, and there are alternate (much more general ways) to allow actors to provide sync requirements, including vars and function requirements. I can elaborate if you'd like.

I truly don't understand where you're coming from with this line of reasoning. Immutability is perhaps the simplest and most pervasive tool we have for making concurrent code safe from data races. Swift has nudged programmers toward using let because immutability makes code simpler to reason about, and it also makes it safe to use concurrently when working with ActorSendable/ConcurrentValue types---something Swift has also nudged programmers worked with pervasive value semantics. Why would we not embrace this wholeheartedly in our concurrency design?

I'm a huge fan of immutability and immutable types, even immutable reference semantic types (that's one of the motivations for the ConcurrentValue proposal). However that isn't what this proposal does or is about. This concept is about allowing actors - which are inherently bags of mutable state that are accessed asynchronously - to have narrow initialization semantics with special case sync access behavior.

This muddies the water by complicating the model, and isn't motivated anywhere that I've seen. Arguing that this is motivated by "immutability being good" doesn't seem sufficient: while we can agree that immutability is good :-) we also need to look at the effect of the design decisions on the resultant programming model because there are many factors to weigh.

@Douglas_Gregor

@Chris_Lattner3

Are you opposed to having a @concurrent attribute on function types, which is orthogonal to but works similar to @escaping ?

I'm not opposed to having a @concurrent attribute on function types. I think we'll need them regardless.

This is fantastic news. I'd love to see this get nailed down, I think it will help clarify a bit chunk of the global concurrency model.

@Douglas_Gregor

With @concurrent being a purely opt-in feature, we'll either silently admit the data race in the first example, or have to reject the second example.

I don't understand this, can you please explain more? There are two designs possible here (the "matrix" and the "tower" approach). I don't have a strong opinion either way, but I'll use the matrix to explain my understanding of how this works.

In the matrix design, @concurrent is a function type attribute that is orthogonal to @escaping and you can have the full cross product of attributes. This means that you get a natural subtype relationship as well: @concurrent () -> () can implicitly convert to () -> () just like @escaping () -> () implicitly converts to () -> ().

Given a design where @concurrent is a new opt-in feature, you get no data race and don't reject the second example. The first example fails to build when you implement runLater with an actor:

func runLater(_ body: @escaping () -> Void) {
  // toss this on a timer somewhere to run
  otherActor.enqueueAfterTime(1.0, body)
  // ^ error, cannot pass non-concurrent closure across actor boundaries because it isn't a `ConcurrentValue`.
}

or with structured concurrency:

func runLater(_ body: @escaping () -> Void) {
  // toss this on a timer somewhere to run
  taskGroup.spawn {stuff(body) }
  // ^ error, cannot capture 'body' in a concurrent closure because non-concurrent closures aren't `ConcurrentValue`s.
}

This is because spawn (or async let, or whatever) takes a concurrent closure, and they require ConcurrentValue conformance of anything they capture and pass/return as argument/results.

In your second example:

extension MyActor {
  func silly(array: [Int]) {
    array.forEach { i in count += i } // okay because closure is non-escaping, so it's in the actor's domain
  }
}

This is totally fine. forEach doesn't take a concurrent closure, so it doesn't have ConcurrentValue checks etc. It is just local synchronous code, protected by the actor as you'd expect. There's no change to type checking for existing code.

@Douglas_Gregor

@Chris_Lattner3

A local function is always assumed to execute concurrently, because they are always assumed to escape.

Why?

Because it took me longer to figure out that this can be checked directly .

I don't have time to read your large patch, can you please explain what behavior you are proposing now?

The obvious and simple thing is to require the @concurrent attribute on the local function to keep it consistent with closures: the decl attribute would change the type of the local function.

Trying to infer syntactically might be possible, but it seems like it will cause problems with non-trivial cases. That said, I don't know what you're proposal is and shouldn't speculate.

@Douglas_Gregor

@Chris_Lattner3

Furthermore, concurrent closures really don't want by-ref captures, they want by-value captures by default. This is a pretty huge behavior change, which should be opt-in.

It would still be implicit, because most closures will become @concurrent implicitly based on context. That's probably okay in practice.

Yeah totally agree, it should work just like the existing @escaping logic, where "the escaping bit" (and thus, "the concurrent bit") is inferred and checked based on the type inferred by the closure if there is context. This is very common since you're often passing the closure to a function.

As a concrete example, in parallelMap(stuff, {...}) the closure is inferred to be @concurrent, so it gets ConcurrentValue checks applied to it.

-Chris