SE-0414: Region Based Isolation

Evolution Proposal Reviews
,
21

This is a great point; I'd had this sticking in the back of my mind as a potential problem, and you're absolutely right to drag it out as something we need to deal with.

The basic problem here can be summarized as follows:

  1. Region-based isolation adds a new capability in which non-Sendable values can be transferred between concurrency domains if they're currently in an independent region.
  2. There is an assumption in region-based isolation that non-isolated functions that have access to a non-Sendable value are limited in the effect they can have on that value's region. Basically, we assume that these functions can merge the value's region with the regions of other non-Sendable parameters (e.g. by making references from one to the other), but we assume they cannot merge the value's region with other regions (e.g. by creating a reference to the value from actor-isolated state).
  3. assumeIsolated does not currently limit the flow of non-Sendable values between the formally non-isolated caller and the formally isolated body of the function it's passed, and this effectively allows the arbitrary merger of regions with the actor's region.

One of these points needs to change to eliminate this soundness hole. (1) is really the whole point of region-based isolation, so we don't want to change that. We could remove the assumptions in (2), but that would make region-based isolation pretty useless without a ton of annotations. So ideally we find a way to change (3).

Changing (3) is theoretically source-breaking, since we don't currently limit the flow of non-Sendable values at all with this function. However:

  • developers have generally been informed that we're still closing holes with some of these concurrency APIs, so we do have some room to change the rules retroactively, and
  • it's not a hard source break because we're still only enforcing concurrency with warnings; at worst we're talking about people seeing extra warnings around assumeIsolated, not a completely broken build.

The only thing that's a little grating is that the current behavior of assumeIsolated is actually perfectly sound in the current sendability model — it's just these extra rules of region-based isolation that make it unsound. But it's still a sort of synchronization/isolation primitive, making it something of an edge case that hopefully isn't getting written afresh all the time. I don't think it's unreasonable for us to rebase our understanding of sendability around the goal of enabling the region-transfer capability, especially if that just turns into adding a carefully-chosen attribute to the callback parameter.

It's interesting to note that we get basically the same situation with locks. In an API like OSAllocatedLock.withLock, the callback is not actually sent — it's going to run synchronously with the current context — but it still needs a certain amount of Sendable enforcement to prevent values from being used concurrently. In particular, the lock-protected state is a "rigid" region that cannot be merged with other rigid regions, such as those of parameters from the surrounding context. Similarly, with assumeIsolated, the isolated region of the actor is a rigid region that cannot be unified with other rigid regions. We can merge non-rigid regions into these rigid regions, e.g. if we just created a non-Sendable value and it's currently independent, but we can't do true peer-merges of them.

Now, withLock doesn't have this hole because we require the function passed in to be Sendable. This is a conservatively correct way of closing this hole, because non-Sendable values can't flow either in or out of a Sendable closure at all.[1] We could do the same thing to assumeIsolated by making its parameter function Sendable.

That fix would be quite a bit more conservative than necessary. I think we can start with it, but if we find it's restricting code too much, we may need to add some ability to mark a function as some kind of Sendable barrier without actually making it Sendable. That would also probably be useful for withLock.

  1. Technically, we can allow values in independent regions to be captured by a Sendable closure as a transfer of the region. However, since we don't know how many times the function will be invoked, and in particular if it might be invoked concurrently, we wouldn't actually be able to use these captured values in the closure. We would need @calledOnce / @calledSequentially annotations to make this useful. ↩︎

12 Likes
22

I have similar concerns if you have a BoundToThreadExecutor and ThreadLocals, but I thiiiink you can prevent that by making it so that NotRubyVM can only be constructed by referencing the actor it's on. It can even throw away that information, but just passing the actor as a parameter results in the constructed value being in the same isolation domain as the actor, because the compiler's analysis is function-local and therefore it will conservatively assume that, post-construction, one may reference the other.

3 Likes
23

To make my review official,

What is your evaluation of the proposal?

Skeptical overall.

Definitely needs revision to address at least (based on previous posts in the thread):

  • Interactions with KeyPath
  • Interactions with assumeIsolated, and third-party functions which may have been written to produce similar effects
  • Interactions with third-party abstractions that may make transfer unsafe (eg. ThreadBoundExecutor + @ThreadLocal)

(my "spidey sense" is that there's probably more out there that we haven't found yet.)

It's also a "partial solution": Most of the cases we're likely to care about will rely on a future proposal to replace @Sendable closures in Task/Task.detached/etc. with closures that allow transfers. Without that proposal implemented and the ability to really try this out, it's quite hard to evaluate whether this will actually solve real-world problems.

I also have minor concerns that eventually Swift will want to solve the same problems as Rust's Pin/Unpin, and that this proposal will interact with that need in difficult-to-predict ways.

Is the problem being addressed significant enough to warrant a change to Swift?

Unconvinced.

This is not a problem I generally have. Usually, things can (and should) just be made Sendable.

When I do have it, it's unclear that this proposal will actually help me (eg. will transferring an AsyncIterator be legal? Probably not unless the underlying AsyncSequence is also transferred, at which point I could probably just have required Sendable sequences and created the iterator "on the other side" of the transfer.

Does this proposal fit well with the feel and direction of Swift?

Unconvinced. I don't like the way that this can be legal

let x = something()
actor.whatever(x)

And then this is illegal:

let x = something()
actor.whatever(x)
print(x)

Or that this can be legal:

let x = a.something()
actor.whatever(x)

And this can be illegal

let x = b.something()
actor.whatever(x)

Even when a and b have the same type, depending on the exact nature and lifetime of a compared to b.

Generally, I'd prefer that my language features have less "spooky action at a distance". I run afoul of this kind of thing enough in result builders, I'm not keen on adding another angle where code can't be copy-pasted between places that "look" identical.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I'm familiar with Rust, and they didn't feel the need to add something like this, relying on Send and Sync to provide concurrency-safety. I don't recall ever needing something like this when programming Rust either, but I've done less concurrency-related stuff there than in Swift.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Somewhere in the middle. More than a quick read, less than an in-depth study.

2 Likes
24

Thanks all for participating in this review discussion! The proposal has been returned for revision.

Holly Borla
Review Manager