Pitch #3: ConcurrentValue and @concurrent closures

Evolution Pitches
7

This is shaping up pretty good!

I'm glad to see it's not an unsafeSend anymore, and just a marker protocol :+1:

The closure things need more details though I feel, allow me to share some questions and cases that are troublesome:

@concurrent closure inference

This is looking good overall, but misses one very tricky aspect of inferring @concurrent that we need to define in a bit more detail.

The @concurrent attribute to function types is orthogonal to the existing @escaping attribute, but it works the same way.

Sadly they do interact a little bit - because in order for the model to actually guarantee safety we need to infer that if a closure escapes, it is potentially concurrent.

The proposal here mentions that @concurrent can be inferred, but does not dive deeper into the semantics of the inference. We can lean on Doug's design doc for a take on this though:

Swift already provides semantics for non-escaping closures, ensuring that they can only be passed "down" the stack. Any attempt to save a non-escaping closure elsewhere (e.g., into a local or global variable, or pass it to a function requiring an escaping closure) is already an error. Therefore, we consider a closure to execute concurrently if it is an escaping closure. [emphasis mine]

Long story short:

  • non-escaping closure:
    • it cannot be invoked concurrently, since in order to do so, we'd have to escape it to that "other" execution context, and in order to do so, we'd have to escape it.
    • We can consider such closures to be implicitly "@nonconcurrent".
  • @escaping closure:
    • we must be pessimistic about it potentially running concurrently and infer it as @conrurrent; We don't know if someone will store it, pass it around etc, and eventually invoke it concurrently. Someone might as well store it, and invoke it when called by some other actor, or even invoked it via a DispatchQueue etc.
    • We must assume it is @concurrent, because we can not prove it isn't.
    • Assuming only @concurrent functions can be passed to other actors addresses the issue but only for passing between actors. We'd have to say we ignore any other execution mechanism in the safety model (could be a thing, but needs to be super explicit in the design).

So, given "@escaping implies @concurrent", we end up with the following world, where escaping functions which are not concurrent don't have a way to express this guarantee.

Case in point is something I bumped into with OSLog, see below:

import os.log
let logger = os.Logger()

actor class Hello {
    var isRefreshing: Bool = false

    func log() {
        logger.log("Hello \(self.isRefreshing)")
// error: Actor-isolated property 'isRefreshing' is unsafe
// to reference in code that may execute concurrently
    }
}

This is fairly surprising, but happens because of the auto closure inside the OSLog's Message:

  public mutating func appendInterpolation(
    _ argumentString: @autoclosure @escaping () -> String,
    align: OSLogStringAlignment = .none,
    privacy: OSLogPrivacy = .auto
  )

It is not invoked concurrently though. Thus, we need to allow such APIs to be @autoclosure @escaping @nonconcurrent - that's a mouthful, but fully expresses the semantics of this closure. Without this, we would not be able to log any actor state using OSLog.

Given such inference rules the model can prevent any racy code involving closures, regardless what the source of concurrency might be. And we need to introduce @nonconcurrent, in order for the model support those few cases which are escaping but not concurrent.

The alternative is to not infer concurrent from escaping. That, sadly, leaves holes in the model and any non-actor non-child task concurrent invocation of this closure (say, via dispatch or anything else) will be able to invoke such "not @concurrent" closure concurrently (:scream:) -- thus breaking the promise we try make with this model.

IMHO, it is not a good idea to follow through with the alternate approach since it makes @concurrent be a small lie, and we can't really rely on it for providing the safety it seems to guarantee. It's up to the team to decide how safe we want the language to be, but I'm not super comfortable with an annotation meant to solve races that can not really prevent those races half of the time (until everything in swift is actors) :thinking:

It means we need @nonconcurrent for those weird uncommon APIs which are escaping but not concurrent (for whatever weird insane reason they do so). Thankfully, there should be very few such APIs, as the normal way is either a) non-escaping and non-concurrent and b) escaping and indeed concurrent.

1 Like