Pitch #4: ConcurrentValue and @concurrent closures Evolution Pitches

Hi all,

Thank you for all the great feedback on the proposal. Doug and I have been iterating on it a bit and have a draft #4 up. My sense is that this is getting close to convergence for a formal review.

This proposal differs from draft #3 in the following major ways:

  • This includes a more explicit definition of marker protocols.
  • Conformance to ConcurrentValue is explicit, instead of being implicitly synthesized (for reasons explained in the rationale section).
  • A new UnsafeConcurrentValue protocol is added to encourage thought about non-obvious cases.
  • A couple of alternatives were dropped: Codable conformance depended on other alternative features, and “Mark conformance to ConcurrentValue as explicitly unsafe” is subsumed into the UnsafeConcurrentValue marker protocol.
  • Bug fixes, wording changes, and other minor tweaks.

Thank you for all the great feedback, this is a core part of the type system, and the discussion and iteration has really helped push this into a good place.

-Chris

19 Likes

It seems like the latest draft is not publicly available, I'm getting a "You need access" error message when trying to open it.

Sorry about that, fixed!

2 Likes

Thank you for including metatype conformances! Will that be implemented in a way that make it easier to allow metatypes to conform to Hashable as well in a follow on proposal?

As such, the compiler rejects conformance of structs and enums to the ConcurrentValue protocol when one of their members (or associated values) does not itself conform to ConcurrentValue (or is known to conform to ConcurrentValue through a generic constraint)

Should that say "or is not known to conform to ConcurrentValue through a generic constraint"?

@concurrent

The purpose of @concurrent is to support function values that conform to ConcurrentValue and can therefore be used in contexts that require ConcurrentValue. The attribute-based design feels somewhat ad-hoc and not scalable. For example, we have discussed function values that could conform to Equatable (using equatable captures combined with source identity).

How would additional protocols fit into the current design? Would we introduce new attributes every time we want function values that have a new conformance (for example, an @equatable)?

I'm wondering if you have considered a more general approach. The immediate option that comes to mind is to use an @ConcurrentValue attribute on the function. This would allow @Equatable, @Hashable, and any other conformances that could make sense in the future.

3 Likes

There was some comparison between @concurrent and @escaping. To continue the analogy, is there a case for non-@concurrent functions to ever be used in concurrent context, necessitating a withoutTrueConcurrency function?

Honestly I have no idea on this.

Yep fixed.

I see what you're saying here, and it is an interesting question. I don't agree with the framing of this though - I would explain it as @concurrent functions having additional semantic constraints on expressions of that type (e.g. the closure capture rules). Those constraints happen to allow the function type to participate as a ConcurrentValue, but it isn't the conformance that drives the discussion.

I don't think this is the right way to go for the reason above, but also because we don't want functions to be equatable or hashable for implementation reasons. It isn't definable at the implementation level in a reasonable way.

One of the interesting-to-me realizations is that these marker protocols are vaguely attribute-like (since they are compile time only). The problem with this viewpoint is that we want ConcurrentValue to participate in the generics system, and making attributes do that would open a ton of complexity we don't want.

Sure, we could define such a thing pretty easily as a library function. This is what the "interop with legacy stuff" helpers allow.

-Chris

I don't want to sidetrack this thread, but this has been discussed in the past. There is a reasonable case for function type equality defined by equatable captures and source identity of the function body. This is similar to how C++ uses anonymous types for each closure.

1 Like

I'll reply with comments on the proposal itself separately, but I wanted to mention two things. First, this proposal has an implementation that one can look at. @concurrent function types are already in the Swift compiler from main (behind the same "experimental concurrency" flag). The rest of this proposal is implemented* in this pull request behind another compiler flag, so one can see the effects of this checking. The same pull request also implements the actor-isolation checking based on ConcurrentValue.

Second, ConcurrentValue and @concurrent are incorporated into the third pitch on actors.

Doug

[*] The implementation differs from the proposal here in a few ways that I'll get back to as comments here.

1 Like

I’m mostly staying away from this one, but I’ll put in a few thoughts I had while reading the proposal:

  • One other reason to not make ConcurrentValue implicit is for library evolution reasons: adding members to a struct is not meant to be source-breaking by default.

  • Having UnsafeTransfer is super important, especially if retroactive conformances to ConcurrentValue are disallowed. I’d say that’s even a reason to remove the AnyObject constraint on the parameter.

  • I get why a closure’s captures affect whether or not it’s @concurrent, but why do the parameters and return type matter? If you send a closure across actors, it’s only going to run on the receiving actor; it doesn’t carry any “provenance” from the sending actor.

  • It’s weird to read this whole part of the proposal without any reference to Rust’s Send and Sync, which work very similarly. I don’t think we’re going to need both protocols in Swift, but it’s absolutely prior art.

2 Likes

Thanks for pointing this out, I hadn't seen this. I'm not going to wade into this debate because it is OT for this thread though.

Added!

Completely agreed.

Keep in mind that this proposal applies equally well to actors and structured concurrency. It models the idea of interacting across concurrency domains, not something actor specific.

It ties into the constraints of concurrent functions at the invocation site. If I have "funcptr(x)" and funcptr is a value of @concurrent function type, then it is invalid to pass an x if it is not a ConcurrentValue. You need to diagnose this (for memory safety reasons) at either the definition site (where you form the @concurrent value) or at the call site.

Both can be made to work, but diagnosing it at the definition site @concurrent func foo(NSMutableString) gives a more predictable user model and better diagnostics.

I'm happy to add more background if you're interested in drafting it, but I'm not a rust expert. I'm also a bit concerned that the differences in the model will cause confusion.

Thank you for the feedback!

-Chris

2 Likes

Since captures are by-value in a @concurrent function, it seems to me this will produce a different result depending on the presence or absence of the @concurrent attribute:

func test() {
   var state: Int = 0
   @concurrent func printState() {
      print(state)
   }
   for _ in 0...10 {
      state += 1
      printState()
   }
}

And it can be more subtle:

func test() {
   var state: Int = 0
   let printer = Printer { state }
   for _ in 0...10 {
      state += 1
      printer.printState()
   }
}

Here, we don't know if the closure passed to Printer is @concurrent or not unless we look at Printer.init's signature.

Is this how it's intended to works?

2 Likes

I see, it’s not just “this function can safely be transferred” like ConcurrentValue, but “there’s no way to call this function unsafely”, a stronger condition. But I’m still unable to come up with a situation where that could cause unsafety. For something like applyInParallel(to: nsMutableString, [appendHello, appendHello]), the problem is with the argument nsMutableString not being a ConcurrentValue, not the argument type of appendHello. But applyToYourOwnMutableString(appendHello) would be okay.

EDIT: including some signatures:

func appendHello(to str: NSMutableString)

func applyInParallel<Argument: ConcurrentValue>(
  to arg: Argument,
  _ operations: [@concurrent (Argument) -> Void]
)

func applyToYourOwnMutableString(
  _ op: @concurrent (NSMutableString) -> Void
) async

Hello,

This revision of the proposal is looking good. Here are a couple of suggestions:

ConcurrentValue conformances

A conformance to ConcurrentValue should be restricted to the same source file as the type definition. This ensures that every stored property or associated value is visible (even the private ones). From outside that file or module, one can only declare conformance to UnsafeConcurrentValue. This ensures that retroactive conformances acknowledge that they are fundamentally unsafe.

I think we should allow a class to conform to ConcurrentValue, with these checks:

  • Each stored property must be immutable (introduced with let) and conform to ConcurrentValue,
  • If the class has a superclass, that superclass must conform to ConcurrentValue (or be NSObject; the same carve-out we have for actors), and
  • The conformance must be in the same source file as the class definition.

Note that ConcurrentValue is a protocol, so conformance to ConcurrentValue is inherited by subclasses. Therefore, if you create an ConcurrentValue class:

class Point : ConcurrentValue { // okay: all stored properties are immutable and conform to ConcurrentValue
  let x, y: Double 
}

The subclasses also conform to ConcurrentValue and get the same checking:

class ColoredPoint: Point {
  let color: Color
}

class Point3D: Point {
  var z: Double // error: mutable stored property in `ConcurrentValue`-conforming class `Point3D`
}

Again, if you can't meet those requirements, use UnsafeConcurrentValue and take matters into your own hands. You could even do that for Point3D.

This really only picks up one case, but it's a useful one: immutable classes. Immutable classes can be safely shared so long as their stored properties conform to ConcurrentValue. Having the compiler check that makes this safe use of classes supported.

It's a small thing, but should we infer ConcurrentValue conformances on imported C enums? You can't make them unsafe in C.

Standard library conformances

While perhaps we don't want to list every single standard library type that should conform to ConcurrentValue, I think the proposal shouldn't try to push the work on a separate library-focused proposal.

I did go through all of the types in the standard library, and my suggestion is that nearly every type conform to ConcurrentValue, using conditional conformances on all generic parameters when those types have generic parameters. The exceptions are:

  • AnyKeyPath: key paths can become ConcurrentValue at PartialKeyPath, using a conditional conformance extension PartialKeyPath: ConcurrentValue where Root: ConcurrentValue.
  • Several Codable-related types: CodingKey is a protocol that cannot add ConcurrentValue conformance retroactively, so EncodingError / DecodingError cannot conform, nor can Keyed(Encoding|Decoding)Container.
  • ManagedBuffer: it's a class that's meant to be shared. There's no way for it to be safe.
  • Unsafe(Mutable)?(Raw?)Pointer: these should unconditionally conform to UnsafeConcurrentValue. They've already fully acknowledged that they are unsafe in every way; this will reduce friction for those that have taken matters into their own hands.
  • Lazy algorithm adapter types. When you do a something.lazy.map { ... }, you get back some kind of lazy sequence adapter type. That type embeds a function type that is not @concurrent, so the adapter types should not conform to ConcurrentValue.

@concurrent functions

I'm confused by this rule:

Functions have @concurrent function type if their parameter and result types all conform to ConcurrentValue, they capture no values (e.g. global functions and closures / nested functions with no captures), and have no inout arguments.

I think this means that an existing function

func doSomething(i: Int) -> String { ... }

would have its type changed from (Int) -> String to @concurrent (Int) -> String by this proposal. That's going to break both source code and ABIs. I also don't feel that it's correct, because existing global functions may very well access global state.

I think the rule here should be that @concurrent is opt-in for functions, and the first/third bullets in should be compressed into something like:

  • A function can be marked @concurrent. Its parameter and result types must conform to ConcurrentValue, it may not have any inout parameters, and any captures must also conform to ConcurrentValue.

There's a place where you mention that @concurrent is "exactly like @escaping", which is a little misleading. @escaping is essentially the default for closures, for cases where you have no context:

let fn = { $0 + $1 } // fn will have escaping function type

@concurrent is not the default for closures. It's more like "noescape" in that sense, because it can only be inferred through context:

let concurrentFn: @concurrent (Int, Int) -> Int = { $0 + $1 } // concurrent, escaping function type

or via the proposed { @concurrent in $0 + $1 }.

I think we should tweak the rule about captures in @concurrent functions and closures. I mentioned it before (although the idea is @Joe_Groff 's), and @michelf ended up providing a good example:

The by-value vs. by-capture is invisible here, which is the subtlety @michelf is pointing out. Joe suggested we introduce a flow-sensitive rule banning mutation of the original variable after the point of capture. @concurrent closures, as already in the proposal, cannot ever mutate their captures. Joe's rule says that once you hit the point of capturing a variable, nobody can mutate it. In the example above, the compiler would produce an error at the line state += 1 if Printer took its closure as @concurrent:

error: mutation of variable `state` after it was captured by a concurrent closure

This checking eliminates the surprise, by making sure that the value can never change (for anyone) after it's been captured in a concurrent closure.

There's one extra check that I've found useful with local functions, which is to require them to be marked @concurrent if they are referenced from other @concurrent code. It's best to start with an example:

func globalFunc() {
  var k = 17
  acceptConcurrentClosure {
    local() // note: access in concurrently-executed code here
  }

  func local() { // error: concurrently-executed local function 'local()' must be marked as '@concurrent'
    k = 25 // error: mutation of captured var 'k' in concurrently-executing code
  }
}

local is being used in code that executes concurrently with the body of globalFunc, so it needs to be marked @concurrent. That makes the requirements on captured local variables kick in. The "may execute concurrently with" predicate described in Preventing Data Races in the Swift Concurrency Model covers the rule here. (That proposal is mostly subsumed by this proposal; it just a reference now).

Interaction of Actor self and @concurrent closures

Most of this section is also covered in the recent revision of the actors proposal, which introduces cross-actor references and ties in to ConcurrentValue. I recommend cutting this section way down and referring over there, so the actor rules can be in one place.

That may sound like a lot, but it's fine tuning. I feel like we're getting close.

Doug

3 Likes

I agree with Jordan: the rule restricting the parameters and return types is unnecessary. A concurrent function is simply one that's safe to share across concurrent contexts; calling it doesn't introduce concurrency directly. If a concurrent function introduces concurrency as part of its implementation, Swift will enforce the correct use of non-concurrent values when checking that implementation; in any case, that's a power that's not restricted in any way to concurrent functions.

Similarly, we can simplify this, because a key path is basically like a function: the concurrent-value-ness of the path in no way depends on the root and value type. Thus, all the KeyPath classes can simply be unconditionally concurrent... almost.

Unfortunately, a key path is also like a function in a second way: it can capture values (as part of applying a subscript), and the types of those captured values are completely erased. This is a problem independent of the root/value type issue, and I see two ways to solve it:

  • Introduce the concept of both concurrent and non-concurrent key paths.
  • Forbid capturing non-concurrent values in key paths.

Since capturing values in key paths is relatively uncommon in the first place, and it seems extremely likely that vanishingly few captured values are of non-concurrent types rather than types like Int and String, I strongly favor the second option. Like all other concurrent value enforcement, this will have to be a warning initially. We can provide a way to disable this check explicitly if necessary; it's possible to work around by instead capturing an UnsafeTransfer, but you'd have to also define a new subscript that takes an UnsafeTransfer and unwraps it, so it'd be quite annoying.

4 Likes

Here’s my attempt at a quick Rust comparison, but it should probably be checked by someone who’s worked with Rust longer than I have:

This model is very similar to Rust’s, with the ConcurrentValue protocol behaving like the Send trait for value types and the Sync trait for reference types. A major difference is that conformance to ConcurrentValue is not automatically added by the compiler for non-actors for reasons detailed elsewhere; UnsafeTransfer provides an escape hatch when such a conformance would have been convenient.

Yes, that is how the proposal is intended to work. We can choose different models though, e.g. it would be workable to require a capture list in concurrent closures. This would cause boilerplate, but could be argued to improve clarity. In your case, you'd have to write:

let printer = Printer { [state] in state }

making it explicit that the capture is by-value.

A value of @concurrent function type models something where the invocation can cross concurrency domains, so it isn't safe to share state across them. If you allowed arguments or result values to be non-ConcurrentValue, then you can get this sharing. For example, consider something like this: func applyToElementsInParallel(arr: [NSMutableString], @concurrent (NSMutableString)->())

Here the calling concurrency domain may be poking at the internals of the NSMutableStrings. when they are passed into the closure, the pointer to the same data is now potentially crossing into another concurrency domain (i.e. another thread) which can cause race conditions and other unsafety problems.

@rjmcall I see your post downthread, does this make sense to you?

Edit: After wrestling with this, I agree with you both. I will changed this in the proposal, thank you for clarifying this!

I'm ok to lock this down to be conservative in the first revision (like we did with the first version of DynamicMemberLookup), but I think this is the wrong way to go for several reasons:

  1. Conformance to ConcurrentValue is a semantic property of a type not tied to its implementation details. Types can evolve over their lifetime, so making a strictly "does this work with this implementation" judgement isn't the right way to think about it. One should instead reason about the intention and purpose of a type, and decide whether you want to commit to this over time, just like any other protocol conformance restricts the evolution of a type over time.

  2. Swift-with-concurrency will need to interop with a lot of Swift <5 code for a long time, and if you believe the point above, there is no reason to prohibit this. Allowing users to add this tag retroactively will ease working with lots of types (e.g. simple structs and enums that are composition of obvious value types) and make Swift concurrency much nicer to use in practice. I think that the checking that you get from the ConcurrentValue conformance (vs the Unsafe version) will improve safety -- pushing people to use the unsafe one doesn't have value.

  3. We need a way to deal with imported C and C++ APIs, and I don't see how marking all conformances as "unsafe" is necessary. Perhaps we can make the importer "do the right thing" in all cases that this isn't a concern though?

  4. I don't think that rampant abuse is going to be a problem here, and I don't think this is a solution to that problem if it were.

That said, to reiterate, I'm fine with staying conservative if you think this is the right way to go. Just let me know. I didn't incorporate this into the proposal yet.

I'm ok with this if you feel strongly about this, but I think it would be conservatively fine to just not allow classes to conform to ConcurrentValue in the first proposal (requiring them to use UnsafeConcurrentValue). Such a conformance has strange interactions with resiliences - for example, you can't add this to open class since you don't know if all the subclasses will be ConcurrentValues and therefore adding it may break code. It isn't unworkable, just non-obvious.

Yeah this is cool, are you ok with further limiting this to the case where the class is final? If so, that seems obvious to me.

That said, I'm ok both ways, so if you feel strongly for this, let's do it! Just let me know.

Yeah totally! I think that many imported C types should get ConcurrentValue inferred. I excluded a detailed analysis of C and ObjC interop from the base type system proposal but I agree this is super important to nail down. Broad swaths of types should be ok (e.g. _Complex float). Probably everything that doesn't contain a pointer. I added a section to future work to capture this.

I agree, I think this should just be handled in the patch, but I don't think the proposal should nail it all down. I agree completely with your list, lemme highlight these two:

Again, I agree with you. Do you think it would be good to call these out in the proposal, or do you want to include a full list?

Yes that was the intention. You're right that it didn't consider ABI impact of that, I didn't realize that it would need a new mangling -- now that you mention it you're right.

Well yes, that is also an issue. I think that is solvable in a simple and clean way, but don't want to bog this down with controversy. I think your observation above is enough to sink this idea.

Agreed, this works for me. Incorporated, thanks!

Good point, I removed the word "exactly" and clarify the default when no context is implied with an example. Thanks!

This makes me uncomfortable for a variety of reasons, including unpredictability and "spooky action at a distance". What do you think about just requiring explicit capture list for the first round of proposal? We can loosen this up in the future with more usage experience.

Let me know what you think and I'll incorporate it.

That makes sense to me, but doesn't this fall out of the capture rules? In your example, you're capturing local, so of course it needs to be a ConcurrentValue. Maybe the compiler isn't modeling it this way?

Yes, I agree that this makes more sense to live in the actors proposal. I moved it to future work but can drop it entirely if you prefer.

Totally agree, thank you for iterating on this!

I agree, the second approach seems best to me. Doug, let me know how you want to revise the stdlib types in general.

I made all the updates in the google doc. Thank you to everyone for the feedback!

-Chris

3 Likes

I updated the text to drop this requirement, but I think this logic applies to the inout issue as well. Actors should require ConcurrentValue argument and results for async actor method crossings, and should prohibit inout arguments, but I don't think that normal functions need to.

WDYT?

-Chris

That change would make a lot of sense. Here's a bridge analogy:

What matters is when does a value crosses the bridge to go visit another executor. When you call an actor asynchronously from another executor, it's the arguments that are crossing that bridge so arguments need to be ConcurrentValue. When you define a @concurrent function, it indicates that the function can cross that bridge; arguments are supplied from the other side of the bridge so they don't need to be ConcurrentValue. Same apply to inout: inout arguments can't cross the bridge, but they work if supplied from the other side.

@concurrent is a way to spell that the function checks all the requirement to itself conform to ConcurrentValue. It can conform if all the data it contains (the capture list) is also a ConcurrentValue, and if it does not reference global mutable state (which I don't think we can check right now). It'd be nice if a @concurrent function type could actually conform to ConcurrentValue so it can be passed around in generics. For instance, it'd allow [@concurrent (Int) -> Int] to cross the bridge.


Can thrown Errors cross the bridge? There's no example with a throws in the proposal. That seems like a giant omission.

1 Like

Well that's simple, the giant omission exists because I simply didn't think about it at all! Thank you for pointing this out!

In the case of @concurrent closures, I think there is nothing to do here: thrown errors are equivalent to returning a Result, and result values don't need to conform to ConcurrentValue per the logic above.

This is a serious issue with actor memory safety though! I'm not sure how best to handle this at the actor level. I can see three theoretical different designs:

  1. we can't make everyone use typed throws with concurrent closures. It doesn't exist yet and would not be ergonomic.
  2. I guess we could theoretically require all throw's in Swift 6 code require that their value is a ConcurrentValue everywhere.
  3. We could make it a dynamic error that isn't caught statically (in the untyped throws case) to throw across concurrency domains. This would require @concurrent to have a runtime representation though, not just be a marker protocol.

I'm very curious to know what @Douglas_Gregor and @John_McCall think about this.

-Chris

To apply a type-based analysis to it, what you're saying is that an inout reference is not a ConcurrentValue and therefore cannot safely be arbitrarily shared. But an inout argument already can't be arbitrarily shared, since they can't be escaped. And it actually is okay to share an inout reference across threads as long as the accesses are still well-ordered, which interestingly is true of async calls. That is, if you have the standard exclusivity guarantee on an inout argument in an async function — that nobody else is touching that argument — there's no reason you shouldn't be able to forward that reference (or something derived from it) down to another async call. And if exclusivity can say that an inout argument is statically exclusive for the duration of a call, that's still going to be safe for async calls. So I think we just need a stricter exclusivity rule here for async calls (which seems appropriate, given that we've never claimed that dynamic exclusivity was sufficient for thread-safety), plus a restriction that the underlying type conform to ConcurrentValue.