SE-0302: ConcurrentValue and @concurrent closures

Yeah, I completely agree, the ability to declare an unchecked retroactive conformance is a mandatory feature here.

1 Like

I can see the value of Synchronized as effectively documentation that a particular class is concurrency-safe because it provides implicit synchronization, which is distinct from being concurrency-safe because the class is immutable. However, Synchronized feels like something that could be checked, and I hesitate to introduce the name and implied semantics without also having some checking in place, because it would be harder to add the checking later.

There is considerable overlap between this attribute and nonisolated(unsafe) in the actors pitch: both allow specific declarations to opt out of the checking that would normally be done to prevent data races. The primary use case for actors is to take a specific stored instance property and allow access to it from code not protected by the actor's mailbox, meaning you need to handle the synchronization yourself. It's very nearly what we're looking for here, but we would need a more general term to describe it. It's more than "sendable" and it's not specific to actors.

What if we call it @concurrent(unsafe)? It would mean "turn off safety checking related to concurrency", which means both allowing non-ConcurrentValue types (the @sendable(unsafe) behavior) as well as making a stored instance property in an actor not part of the actor's isolated state (subsuming nonisolated(unsafe)). We are already proposing @concurrent for function types, where @concurrent(unsafe) also makes sense: it should disable the checking that captures conform to ConcurrentValue.

Doug

Something like this is definitely possible, but I'd really prefer to consider it in a separate proposal. This proposal is already pretty large in scope, and is very fundamental. Expanding the scope will make it more difficult to land this piece of the puzzle.

-Chris

I specifically would not like to use the name synchronized for anything around this because I think it's possible we'll want to make synchronized types (or at least variables) that use fine-grained locking, for which there's ample prior art under that keyword. Even if we don't actually do that, the name is taken.

5 Likes

The -Value suffix could be dropped from protocol name to align with @concurrent function attribute - which means the function require [Unsafe]Concurrent protocol constrained args.

[Unsafe]Concurrent <-> @concurrent

Or the other way:
[Unsafe]ConcurrentValue <-> @concurrentFunction.

Agreed. Retroactive conformance to UnsafeConcurrentValue is going to be critical for folks adopting concurrency before the libraries they depend on have annotated types with ConcurrentValue. Having the per-member @sendable(unsafe) or (my favorite spelling) @concurrent(unsafe) is gravy---it narrows the scope of ConcurrentValue---but doesn't obviate the need for UnsafeConcurrentValue.

Doug

3 Likes

This looks like a solid proposal, and I'm glad these features are coming to Swift.

I did want to question some of the general statements made, which don't quite mesh with my understanding.

Swift has a strong emphasis on types with value semantics, which are safe to transfer across concurrent boundaries.

Structs, enums and tuples are the primary mode for composition of values in Swift. These are all safe to transfer across concurrency domains — so long as the data they contain is itself safe to transfer.

Is this true? Are value types safe to share between concurrency domains?

I had a long discussion back in 2019 and learned that value types which are shared between threads are just as dangerous as reference types. You can get race conditions — of course — but you can even get inconsistent states where the value is completely invalid (eg. half the struct is from a write on one thread, and half the struct from a write on another). See Demonstrates how a Swift value constant can mutate when using Copy-on-Write (CoW) and multi-threading. · GitHub

In the end several members of the Swift team convinced me that shared data — even shared value types — are not safe in any way.

Swift’s Copy on Write approach means that collections can be transferred without proactive data copying of their representations — an extremely powerful fact that I believe will make the Swift concurrency model more efficient than other systems in practice.

Unfortunately, the way CoW is generally implemented — with an embedded reference type — means most CoW types are not safe. Again, see that gist. Demonstrates how a Swift value constant can mutate when using Copy-on-Write (CoW) and multi-threading. · GitHub

@concurrent(unsafe) could obviate the need for UnsafeConcurrentValue if it was also applicable to an extension:

@concurrent(unsafe)
extension SomeType: ConcurrentValue {}

Might also be applicable to a type as a whole:

@concurrent(unsafe)
struct SomeType: ConcurrentValue {
    var str1: NSString
    var str2: NSString
    var str3: NSString
    var str4: NSString
    var str5: NSString
}

So you don't have to repeat @concurrent(unsafe) for each variable.

1 Like

A very nice read, huge +1.

Just need one clarification. In the proposal, it says

... can only be made to conform to ConcurrentValue within the same source file in which the type was defined.

No matter it's a struct, an enum or a class.

But in the proposal, it also mentions "retroactive conformance" several times, for example

... and retroactive conformance makes it easy for users to work with older libraries that haven’t been updated to know about the Swift Concurrency model yet.

Does it mean we can make a type conform to UnsafeConcurrentValue outside of its defining source file?

Combined @concurrent(unsafe) (for member, extesnion or Self ) and Concurrent protocol would lead to confusion for swifters; what exactly the concurrency model is this type? Safe(checked) or Unsafe(unchecked)?

It's ok to have both [Unsafe]Concurrent protocols for the whole type.

If @concurrent(unsafe) is selected there is NO need to conform to Concurrent protocol any more.

@concurrent(unsafe)
struct/extension SomeType {
.. class members..
}

above looks simple and reasonable. But it will lose conditional conformance capability for generics.

About the thread-safety of value types, I wrote some rules-of-thumb here: Understanding Swift's value type thread safety - #14 by JJJ

Basically lets are thread-safe, vars are not. If you start a closure by assigning a shared var to a local let your closure still access a non-local var and is therefore not thread-safe. If you capture the shared var as a let up front, then all is good :smiley: :+1:

2 Likes

Right. The key point is that value types are perfectly concurrency-safe when you're talking about concurrent accesses to different variables, even if one is created by copying the other. Value types do not make concurrent read/write or write/write accesses to the same variable safe, but if you expect that, you don't really understand concurrency.

All of this applies equally to value types implemented with a copy-on-write reference type behind the scenes. The only subtlety is that, when you are concurrently modifying two copies of the same original value, you can sometimes end up with both threads copying the referenced object instead of one of them getting to take it over.

2 Likes

Yes. You can make a type conform to UnsafeConcurrentValue outside its defining source file. It's unsafe because the compiler can't do the checking to ensure that the stored properties/associated values are all ConcurrentValue.

Doug

3 Likes

Very interesting gist, thank you for sharing it. I believe that this is safe in the new concurrency system being designed here, and that ConcurrentValue with the rules proposed will be safe as well.

The key thing going on in your example is that you have two threads poking at the same copy of the data, so the refcount is 1 because the closure is capturing crazyOperator by reference. The way the actor and structured concurrency features work is to share mutable things across concurrency domains by copying them first. This means that the refcount of the underlying mutable value is guaranteed to be >= the number of concurrency domains that are referencing it.

The only situation where that is violated is when the value/ConcurrentValue is immutable. The proposed model allows let's to be shared by multiple concurrency domains (e.g. as members of an immutable class, where the class is declared as a ConcurrentValue). I believe this is also fine, because you have to copy a let value before mutating it, increasing refcounts to at least 2 predictably.

Thanks for raising this case though, it is interesting and not one I had considered. This is even more of a reason for legacy APIs like "async" on GCD queues to be marked as @concurrent closures. Doing so would define away the race condition in your code, because you'd have to declare crazyOperator as being captured by value:

DispatchQueue.global(qos: .background).async { 
  [crazyOperator] in ...
}

You can test this with your gist by manually capturing crazyOperator by value. That should eliminate the race as well. This proposal forces you to write the correct code (assuming GCD's async method adopts the attribute).

-Chris

6 Likes

Evaluation of the proposal:

This seems quite heavy handed. The requirements for existing swift frameworks might be quite high. From my understanding the proposal as stated requires every structural type to adopt a new protocol. Applying this to a relatively large framework project (that is already built to be thread safe) is quite daunting. I think it would be better to require the annotation to any type that contains unsafe types such as (non actor) reference types but not require structural types (such as struct or enum etc) that only contain inhabitants that are already either implicitly a concurrent value or explicitly a concurrent value.

In general I think it is quite uncommon for folks to build their own CoW types so requiring that is quite reasonable. For a distinct example, I think it is perfectly reasonable to ask Foundation structural types to adopt this but I don't think that asking this to be littered on types that are composed of things like Int and String and Dictionary are reasonable to ask them to have to consider if the types they use are thread safe or not (which they would be perfectly safe due to CoW or being PoD types).

Is the problem being addressed significant enough to warrant a change to Swift? The problem itself seems reasonable to consider but the approach given is less than ideal and quite a learning burden as well as a maintenance burden.

I think this as it is proposed (from my understanding) does not meet the bar of making swift approachable nor does it make concurrency easier - in fact this seems harder than the concept of atomic properties in objc or ref counting in my opinion - even though the task is difficult for the compiler to reason it really seems like more heavy lifting there should be done.

I have not worked extensively with any similar language features shy of atomic properties or @synchronized / using locks in c derived languages; but I have taken the time to review what it would be like to adapt this to Foundation's types and it is a huge audit with some cases that are quite unfortunate for having to use the "unsafe" annotation on types that are known to be structurally safe. Additionally I have taken a look at some other projects I work on and it would be a huge ask for little visible benefit. I am sure this will pose an adoption issue and will likely leave many developers out there not being able to use a good number of frameworks with concurrent closures.

One additional note: being that task cancellation is a concurrent callback this will pose a HUGE adoption problem with a number of use cases.

Summary a big -1 as it currently stands; unless I misunderstood the proposal.

2 Likes

My initial impression was to be concerned about the requirement for explicit conformance as well, but Swift requires this for fundamental protocols such as Hashable and the relatively more recently added Codable and Identifiable. All of these are necessary for fundamental features of the language or key frameworks, and I'm not aware that this has been much of an issue. Therefore, I have to conclude that not just existing design precedent but also our empiric experience working with these designs would favor continuing with such a design here over magical auto-conformance. The latter is not free of drawbacks, one prime drawback being that innocuous-looking changes to a type could silently cause it to lose a conformance.

4 Likes

I'm curious, all these fundamental protocols are single word naming with -able suffix, such as Equatable and Comparable, according to this convention Concurrency model protocol should be named like Concurrentable or just Concurrent paired with @concurrent attribute, but why it's called Concurrent·Value?

Everybody knows it applies to value, and we didn't have HashableValue CodableValue ComparableValue EquatableValue ... protocols as for now.

If it is a basic/core/fundamental protocol it would be better to name as a single short word.

E.g. FundamentalValue protocol doesn't look like it's a fundamental core protocol at all, right? :innocent:

2 Likes

One option (and I'm not sure if this has been already raised) might be to infer the conformance on non-public types, and only require it to be explicitly stated when a type is made public. Then the argument is about consistency between public and internal types vs. ease-of-use within a module.

1 Like

What prevents me from writing this struct? If the answer is nothing, then why is there distinction between ConcurrentValue and UnsafeConcurrentValue when both are unsafe?

var global = 0
public struct Foo: ConcurrentValue {
  public func f() {
    global += 1
  }
}

I'm not sure how much the big picture has changed as these proposals have been iterated on, but at one point the answer to your question was that long-term (i.e., Swift 6), we'd require mutable globals like that to be annotated with a global actor, so it could not be directly mutated from arbitrary code like Foo.f.