The changes to the first reviewed proposal include:
Non-Sendable captured values of isolatedclosures are merged into the actor's region. This prevents parameter regions of nonisolated synchronous functions from being transferred into actor-isolated regions through APIs like Actor.assumeIsolated.
Non-Sendable closures that call global actor isolated code are formally isolated to the global actor.
Key-path literal values with isolated path components are part of the actor's region.
This review is scoped to the above revisions to the proposal.
Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by email or DM. When contacting the review manager directly, please put "SE-0414" in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
Could not find this in the proposal text. Are arguments of the nonisolated synchronous function assumed to be in a disconnected region or an unknown connect region?
The latter is more conservative, but the former might cause issues:
@MainActor var global: NonSendable?
func f(_ ns: NonSendable) {
// [{(ns)}, {(global), @MainActor}]
global = ns
// [{(ns, global), @MainActor}]
}
func g() {
let ns = NonSendable()
// [{(ns)}]
f(ns)
// no information that region of ns was merged with MainActor
// [{(ns)}]
let a = MyActor()
// [{(ns)}, {(), actor}]
a.field = ns
// [{(ns), actor}]
}
+1 on the proposal. It is solving a big problem with the current Sendable checking after we fixed all the remaining holes. At this point, the Sendable checking is overly restrictive and I have personally used an struct UnsafeTransfer<T>: @unchecked Sendable box way too often.
I also appreciate the very detailed reasoning and examples throughout the proposal!
I have a potentially similar question to @Nickolas_Pohilets but in a slightly different example. In the following code we assume that after await value that x is disconnected. How can we be sure that is true inside the nonIsolatedCallee function we might have transferred the value to another region like @Nickolas_Pohilets showed above to a global actor but it might be to just another actor or task region. Especially considering that the nonIsolatedCallee might be in a separate module and we have no visibility into what it is doing to x.
func nonIsolatedCallee(_ x: NonSendable) async -> Int { 5 }
actor MyActor {
func example() async {
// Regions: [{(), self}]
let x = NonSendable()
// Regions: [(x), {(), self}]
async let value = nonIsolatedCallee(x) + x.integerField
// Regions: [{(x), Task}, {(), self}]
useValue(x) // Error! Illegal to use x here.
await value
// Regions: [(x), {(), self}]
useValue(x) // Ok! x is disconnected again so it can be used...
await transferToMainActor(x) // and even transferred to another actor.
}
}
Doesn't this mean that any synchronous or non-isolated asynchronous function must assume that it's parameters are bound to a Task or actor region so they must prohibit transferring the value into another region since the caller assumes that value is in its region again after the call returned. Ultimately this would mean that all parameters have strong transfer semantics currently.
Could we add a section for TaskGroups as well similar to async let. I commonly want to create an AsyncSequence's iterator wait for one element and then transfer the iterator into a child task. This ought to be safe since I am not using the iterator in the parent task after having it transferred to the child task.
On another note, is it possible to let the compiler show its understanding of the regions as remarks via some compiler flag?
I have a question about Actor Isolated Regions. It is written: "Since the region is tied to an actor's isolation domain, the values of the region can never be transferred into another isolation domain since that would cause the non-Sendable value to be used by code both inside and outside the actor's isolation domain allowing for races"
Seems that can never be transferred is too restrictive.
Is the following code valid and possible?
actor Actor {
var nonSendable: NonSendable? = .some(NonSendable())
func foo() -> NonSendable? {
let copy = self.nonSendable
self.nonSendable = nil
return copy
}
}
@MainActor func actorRegionExample() async {
let a = Actor()
let nonSendable = await a.foo()
await transferToMainActor(nonSendable) // cunsuming NonSendable
}
It's not just closure captures we need to worry about, right? My original example of a potential soundness hole doesn't involve closure captures at all – instead the cross-actor violation was caused by a nonisolated function returning an actor-isolated value.
Example
class NonSendableType {
...
}
actor Actor1 {
var x: NonSendableType
nonisolated func get() -> NonSendableType {
self.assumeIsolated { isolatedSelf in
return isolatedSelf.x
}
}
}
actor Actor2 {
var x: NonSendableType
func take(_ x: NonSendableType) {
self.x = x
}
}
func unsound(actor1: Actor1, actor2: Actor2) async {
// Regions: []
let nonSendable = actor1.get()
// Regions: [(nonSendable)]
// Not [{(nonSendable), actor1}], because `get()` is nonisolated
await actor2.take(nonSendable)
// Regions: [{(nonSendable), actor2}]
// `nonSendable` crosses actor boundaries!
}
Yes, you're right. That the closure result value is part of the actor's region along with the parameters and captures would naturally fall out of the other region merging rules, except that the implementation of assumeIsolated unsafe casts away the isolation in order to call the closure. I think this is a problem with the API, and it needs to be changed to require a Sendable result value. If we pursue the "transferring result type" future direction of this proposal, we could add an overload of assumeIsolated that accepts a closure returning a (strawman syntax) transferring T result type where T is not required to conform to Sendable.
In this case, one would not be able to assign into the global since f is not MainActor isolated.
Arguments of a nonisolated synchronous function are considered to merge together its arguments like any other function. The reason why this is safe is that the arguments of the nonisolated synchronous function can never be transferred into another isolation domain... so we know that as long as the synchronous functions s actually nonisolated, we are ok.
In the proposal, we specify that all function parameters of nonisolated functions are Task isolated and cannot be transferred in the callee. In contrast, a parameter of an actor isolated function takes its parameters as a weakly transferred actor isolated value. The end result is that in both cases, one is unable to transfer the parameter in the callee into another isolation domain.
If one wants to be able to transfer a parameter further, one must mark it as strongly transferred (which is a different proposal).
So to apply this to your example, since nonIsolatedCallee cannot transfer x further, we know that once the async let has finished that x is safe to be treated as disconnected since it cannot be used from any other isolation domain.
Can you provide an explicit example to ease our discussion? = ).
My thought was that showing the regions as remarks is too noisy. Instead, what I was planning on doing was making it so that when the compiler emits a warning, we would explain the regions at that point. So for instance:
func foo(_ x: NonSendable) async {
let y = NonSendable()
let tuple = (x, y)
await transferToMain(y)
}
We would say something like:
func foo(_ x: NonSendable) async {
let y = NonSendable()
let tuple = (x, y) // (1)
await transferToMain(y) // (2)
}
Warning: Transferring 'y' could result in races. This would be at (2).
Note: 'y' is task isolated and thus cannot be transferred to the main actor. This would be at (2).
Note: y is task isolated due to the following region merges. This would show a list of merges that lead to x and y being in the same region. In this example, we would just show (1) since that is the only merge.
Looking at the example that you provided above, the first problem that you will run into is at (1). It is still illegal to return part of an actor outside of the actor region. So returning copy would be illegal. Now one may say, well I can see that there isn't an issue here, but from a type system perspective, there is nothing preventing it.
What you are really looking for is something that will be proposed in a subsequent proposal I am preparing: disconnected fields and transferring results.
A disconnected field of an actor is a field that is separate from the rest of the actor's region and which one can extract from the actor and turn into a disconnected region via a "swizzling" sort of operation (like you have above).
A transferring result is a value that is transferred into the caller's isolation domain. So together, one would be able to write the above code like the following (noting I am straw manning the syntax for the purpose of our discussion):
actor Actor {
disconnected var nonSendable: NonSendable? = .some(NonSendable())
func foo() -> transferring NonSendable? {
let copy = self.nonSendable
self.nonSendable = nil
return copy
}
}
@MainActor func actorRegionExample() async {
let a = Actor()
let nonSendable = await a.foo()
await transferToMainActor(nonSendable) // consuming NonSendable
}
I was actually thinking of the opposite use case, where a non-sendable value in the actor's region can be returned from assumeIsolated, but would still be statically bound to the actor's region:
assumeIsolated<T>(_ body: (isolated Self) -> T) -> @isolated(to: self) T
This would allow us to do something like this:
actor Actor1 {
var x: NonSendableType
}
func example(actor1: Actor1, actor2: Actor2) async {
// Regions: []
var x: NonSendableType = actor1.assumeIsolated { $0.x }
// Regions: [{(x), actor1}]
// This would be fine:
print(x)
// This would be an error:
await actor2.take(x)
}
But it wouldn't cover the use case where we want to return a non-sendable value that's independent of the actor, and then transfer it to another isolation domain outside of the closure. In that case, this would be better:
assumeIsolated<T>(_ body: (isolated Self) -> @returnsIsolated T) -> T
If we want to accommodate both use cases, the region of the return value of assumeIsolated would need to be dependent on the region of the returned value within the closure. It makes me wonder if someday we can allow functions to be generic over isolation. In the meantime, maybe an underscored attribute or some other special casing for assumeIsolated can achieve this, if we ever find it to be necessary.
actor MyActor {
let ns: NonSendable
init(_ ns: NonSendable) {
self.ns = ns
}
func doStuff() {
// use ns
}
}
func f(_ ns: NonSendable) {
let a = MyActor(ns)
// [{(ns, a.ns), a}]
Task {
await a.doStuff()
}
}
func g() {
let ns = NonSendable()
// [{(ns)}]
f(ns)
// no information that region of ns was merged with MainActor
// [{(ns)}]
let a = MyActor()
// [{(ns)}, {(), actor}]
a.field = ns
// [{(ns), actor}]
}
How does the merged region look like inside the callee? Is it disconnected? If connected, what is it connected to?
How is this achieved? Is this because region of the arguments is already connected to something? Or in some other way?
Does this apply both to synchronous and asynchronous functions?
A task isolated isolation region consists of values that are isolated to a specific task. This can only occur today in the form of the parameters of nonisolated asynchronous functions since unlike actors, tasks do not have non-Sendable state that can be isolated to them
This sentence made me think it does not apply to synchronous functions.
I think this is just a mistake in the proposal text; actor initializers transfer their parameters regardless of whether the initializer is sync or async.
Same here; parameters of nonisolated functions should always be considered to be in a task-isolated region regardless of whether the function is sync or async.
assumeIsolated is really only meant to be used within non-isolated synchronous functions, and this is directly relevant to your desired use case.
Non-isolated synchronous functions preserve the dynamic isolation of the caller, but they lose track of it statically; assumeIsolated re-aligns these two things. It's then safe to use a non-Sendable value from the actor's region within the callback because it's synchronous and the dynamic isolation is still preserved. We could allow it to be returned out to a different synchronous function, but that'd be pretty pointless, because we'd still have to restrict it to not escape that synchronous, dynamically-isolated context, and there's really nothing you can express that way that you can't express by just doing that same work within the assumeIsolated callback.
async functions never lose track of their formal static isolation, and non-isolated async functions do not preserve their caller's isolation dynamically. If you call assumeIsolated from an async function, you're really just asserting something about the current context, which again is pretty pointless: it's not clear to me why someone would ever need to do this instead of just declaring the function with the right isolation in the first place. If somehow you did have a situation where an async function is executing in an isolated context without knowing about it statically, that's only going to be true until the next suspension. So returning a non-Sendable value from an actor's region into an asynchronous context would have to do some sort of check that you didn't suspend before using it, which is not the sort of thing we really want to be adding to the language instead of, again, just requiring the value to be used within the synchronous callback.
Thanks for clarifying that! From some examples I got the impression that was not the case.
Sure something like the following code ought to be safe but is still producing Sendable warnings on the latest main toolchain with RegionBasedIsolation enabled.
func bar() async {
let stream = AsyncStream<Int>.makeStream().stream
var iterator = stream.makeAsyncIterator()
let first = await iterator.next()
await withTaskGroup(of: Void.self) { group in
group.addTask {
/// Iterator is transferred to the child task's region
while let next = await iterator.next() {
}
}
}
// We should be able to use the iterator here again since the above task group has terminated.
}
+1. I know the review period is over and I also don’t fully understand all of this one but I’m so excited for this stuff to work and ease the use of swift concurrency.