[Pitch #2] Async/await

The operand of an await expression must contain at least one potential suspension point, although more than one potential suspension point is allowed within an await 's operand.

This sounds stricter than the current try. You can use try without throws and get away relatively scot-free with just a warning. Is this intentional, or is it just the wording?

The pitch allows for (and demonstrates) async methods on non-actor types including classes, so interleaving issues with instance state apply there too. I expect that bugs caused by interleaving on global state will be relatively common too, although probably less so than with instance state.

Having louder suspension points would lessen the concern that interleaving will slip unnoticed. But await is loud enough for me. What isn't loud enough is the warning when introducing the concept. At the very least, the proposal should have an example of something where interleaving can be problematic. Currently, this is only discussed in the abstract and easily dismissed.

The proposal says to be careful with mutexes and blocking functions, but it offers no alternative. It says: "Ongoing library work to provide abstractions that allow programs to avoid these pitfalls will be required." It's vague and subtle though. I think the proposal should expand a bit on that, perhaps in Future Directions, so people can have some idea of how this is expected to work in an async world.

This sounds like a fine starting point, but I think it is worth making TLC in scripts and playgrounds be implicitly async-compatible, just like it is implicitly throw-compatible. Among other things, this will make it much easier to teach people async code, and will make scripts much less annoying in an async filled world :slight_smile:. This can obviously be a follow-on proposal after the major details are settled.

-Chris

8 Likes

Thank you for the clarification, I think I understand what you're going for here: you don't want actor isolation to be broken by inout parameters. I've been trying to really grok how this works with the proposed model, but I don't think I've really succeeded.

There are two different but confusingly similar concerns that I'm trying to come to grips with:

  1. Actor isolation doesn't want references to state to be transferred across actor boundaries in a way that can introduce race conditions / unsafety. Getting this right is a key part of the actor model design, related to the global semantic discussions, actor sendable, etc.

  2. The "reentrancy" aspect of actors combined with the "opening of accesses in inout parameters" means that things passed as inout parameters can have their accessed opened for "a long time" and cause unexpected exclusivity failures that turn into dynamic crashes that may be difficult to test. This can cause unpredictable and difficult to debug crashes in the field.

If I understand correctly you're trying to solve both problems at the same type by preventing forming inout argument references to properties of actors, e.g. limiting inout binding to local variables.

However, I'm not sure (said differently: every time I think about this I confuse myself :) that this is either necessary or sufficient. Here's my uncertainty:

  • Insufficient: Local variables are part of the actor protected state. They can escape locally in the actor when captured by-ref in a closure. Passing it as an inout argument can cause a race condition between the actors thread/queue/context and the receiving actor's thread. I suppose this would get caught dynamically as an exclusive access violation, but it provokes the second issue above.

  • Necessary: actors can have non-actor crossing sync and async methods within their concurrency domain. It should be perfectly fine to pass actor properties as inout parameters to these intra-actor functions. For example, my understanding of your rule is that no call to a mutating async method on an actor property would be allowed.


I'm primarily concerned about the cross-actor usage of inout parameters from a memory safety perspective. I don't see any obvious way to make this work well, because inout intentionally introducing aliases of actor-protected state in the cross-actor condition.

However, I think the secondary issue is a pretty significant one in practice as well. Even if dynamic exclusivity checks catch the problem if/when the problem manifests in practice, these crashes will be rare, require specific concurrency patterns to manifest, and therefore lead to bugs in practice.

This also doesn't seem like an important case to support for expressivity, so I'd prefer to define away the foot gun and complexity entirely.


Concrete proposal / recommendation / thought:

The compiler already has to reason about cross-actor calls - this is where sync methods become async (requiring await), and the ActorSendable sorts of checks have to be performed. Would it be reasonable to just forbid passing anything inout across these actor boundaries?

This fixes the unnecessary limitation of intra-actor references, and such a limitation could be relaxed in the future if there is a safe model for it. It would also eliminate a complicated part of the actor model we'd otherwise have to nail down.

-Chris

I think the description makes a lot more sense if the local variables are not protected. Whether they should, or if we can reign them in is another story.

Actually, I don't see how async inout works with protected states (by essentially any definition) at all.

Which scenarios are you thinking about? You normally can't escape an inout reference. You need to make a copy in one way or another.

I don't see how that would work. While the method is suspending, accessing that property is totally undefined, and we need to be able to access it during the interleaves.

That is quite a problem :thinking::thinking:. Though it's about as much as escaping the same variable in two places, which is about as much as I'd expect.


I did mention it on the other thread but I have an inkling that, if we can figure out what variables can go in escaping (sync) closure, we could simultaneously solve the async inout problem.

I'm thinking about this case:

actor Foo {
  var otherActor : OtherActor

  func someActorMethod() async {
     var x = 42
     doThingWithEscapingClosure { x = 57 }

     otherActor.crossActorCall(&x)
  }
}

In this case, x is a normal local variable, but "doThingWithEscapingClosure" can be calling the closure as often as it wants, mutating it in the background even when otherActor is touching it.

That's my point, it seems like it should work. Consider these examples that have nothing to do with actors:

  extension Bool {
     mutating func asyncTwiddle() async { ... }
  }
  func otherAsyncThing(x : inout Bool) async { ... }

it seems like this should be allowed:

actor Foo {
   var x : Bool

   func internalImplDetail() async {
      await x.asyncTwiddle()
      await otherAsyncThing(&x)
   }
}

But it seems to be banned by this proposal because x is being passed inout in both cases, even though this is a purely intra-actor computation.

-Chris

That shouldn't work. There's this case:

extension Bool {
  mutating func asyncTwiddle() async {
    // 2
    await ... // suspend
  }
}

actor Foo {
  var x: Bool
  func internalImplDetail() async {
    // 1
    await x.asyncTwiddle()
  }
}

If we have:

// In actor A
await someFoo.internalImplDetail()

// In actor B
await someFoo.internalImplDetail()

we have a problem when

  1. A enters (1), (2)
  2. A suspends
  3. B enters (1)
  4. B cannot enter (2) because of exclusivity

I expect that async mutating functions will not be supported, for the same reason that we can’t support async property setters or subscripts.

Special functions like deinit and storage accessors (i.e., the getters and setters for properties and subscripts) cannot be async .

Rationale : Properties and subscripts that only have a getter could potentially be async . However, properties and subscripts that also have an async setter imply the ability to pass the reference as inout and drill down into the properties of that property itself, which depends on the setter effectively being an "instantaneous" (synchronous, non-throwing) operation. Prohibiting async properties is a simpler rule than only allowing get-only async properties and subscripts.

Since this is a permanent “can’t fix” rather than a temporary limitation, I’d rather that we did support get-only async properties and subscripts, and changed the rule to forbid async setters or mutating methods.

It’s already hard enough to decide between writing a function or read-only computed property. Please don’t break the flimsy “rules of thumb” that we’ve somehow landed on :sweat_smile:

Okay, I can understand the confusion there. I think mentioning await at all here is causing confusion.

I'm attempting to address your concerns with https://github.com/apple/swift-evolution/pull/1225, to try to make it clear that we are marking potential suspension points with await and giving some reasons why one might not known whether a potential suspension point is, in fact, an actual suspension point.

To Karl's point, the async/await proposal is neither trying to handle data races nor trying to describe actors. Suspension points moving a task to a different thread breaks some atomicity assumptions; whether actors do that by default or not, and what kind of facilities help prevent accidental data races here, are intentionally left to the other, later proposals. This document shouldn't try to describe those in detail.

I think this is far enough out of scope for the async/await proposal that we shouldn't try to put it into Future Directions at all (and I've removed the "Ongoing library work..." sentence that hints at it). We might be able to provide some power-user APIs (say, as part of the Task library) to cope with them, but we shouldn't guess at what they would look like, and it's more important for this proposal to stay focused.

Doug

This is unnecessarily stricter than try, for no reason. And the implementation was emitting a warning anyway. I'll update the proposal to match the implementation and try.

Doug

1 Like

It's not a "problem" in the memory safety sense, but it is a footgun. This is why I was/am concerned about getting the interaction between actors and inout exclusivity right. That said, this is an actor level concern, not an async/await concern, so it seems best to move it to that level of the stack when it comes up.

1 Like

@lihansey please note that you are commenting and reading an outdated thread.

The proposal has been updated multiple times since the Pitch #2, and here is the latest version which was accepted: [Accepted with Modification] SE-0296: async/await

May be worth closing this thread?

Terms of Service

Privacy Policy

Cookie Policy