`async let` and scoped suspension points

Evolution Pitches
61

There's some interesting work ongoing by @Michael_Gottesman to optionally surface the locations of retains/releases and other runtime calls after optimization using diagnostic remarks: [assembly-vision] Finish rebranding of opt remark gen to Assembly Vision Remark and add @_assemblyVision Attr by gottesmm · Pull Request #37895 · apple/swift · GitHub. It would be interesting to try adapting this approach to identify potential suspension points for display in an editor, maybe as a faint highlight or horizontal rule (or anything less obtrusive than a regular error/warning). It would probably need to be an unofficial extension of the LSP spec though...

2 Likes
62

Thanks for raising this Joe, I agree that this is a key question that we need to nail down. The "marked suspension + interruptability" of actors is the biggest bet, and programmers being able to reason about suspend points is the most important part of making this fly.

When I take a step back and look at this, we have three goals:

  1. Reduce bugs introduced by actor suspensions by making it clear where the problematic suspends happen.

  2. Make sure we have a principled model that can scale to other situations, as you say.

  3. Make sure we don't overwhelm the code with noise.

I intentionally wrote the first bullet that way: our goal isn't to have an await on every suspension point, our goal is to reduce bugs by marking the problematic ones. As such, I personally think that situations like this:

func f() async throws {
  async let x = foo()

  try somethingThatMightThrow()

  use(await x)
}

Should be fine without extra syntax: the caller of f will be "awaiting" already, and there is no logic that can execute in the actor between the throw and the await, so there is no opportunity for a bug. If you generalize this, this gives us a model that says "marking is required on suspension points that can come before access to actor internal state".

Generalizing this implies that a hybrid model could work:

  1. We can make examples above work as a simply application of the rule.
  2. We can use dataflow to eliminate marking requirements on cases where the value is unconditionally read.
  3. We can use explicit annotations in the few cases that remain.

There are a few tricky things to consider, and your defer example is one example of this: leaving a scope does cause logic to run that can touch the actor state, so we need to decide "what side" of the suspension they happen on. Notably, the deinits of classes get run when exiting a scope. If you have:

func f() async throws {
  let class = SomeClass(self) // it's deinit can reference our actor.
  async let x = foo()

  try somethingThatMightThrow()

  use(await x)
}

Then we should make sure that the deinit for the class happens before the implicit await for the task group on the throw edge.

-Chris

5 Likes
63

Do deinits always run exactly when exiting the scope? I thought the rule was more like "deinits run sometime after the last use of the instance and the end of the scope".

3 Likes
64

Yes, there was a whole session at WWDC this year telling us not to rely on deinit timings.

65

That is a pretty overly-detailed rule. People will most likely let the compiler warns them and put down await to shut up the compile error, which is not good sign. We should also keep an eye out for a sweet spot somewhere between that and annotate all suspension points.

I do agree, and I think, in particular, we can omit a suspension marker when:

  • It is at the end of a closure/function body, or
  • It is at the end of a loop with suspension in the continuation condition, i.e., for await ... in ... {} and while await ... { }.

Would that really be a problem? All access on the clean up path are defer and deinit, which is non-async. They should only be able to access non-isolated members of the actor.

Since I'm already being pedantic, I think this should also apply to non-trivial @Sendable in general, esp. those with manual locking/queuing and those that are non-trivial and non-Cow.

1 Like
66

This last bullet I'm not so sure about. If the await is part of the loop condition, you'd tend to believe it awaits when evaluating the condition. As long as the condition is evaluated each time the loop scope ends that'll do. But if you break from the middle of the loop body I think it's unintuitive that you'll go back to the await in the condition, and then exit the loop without evaluating the condition.

1 Like
67

I don't think those missed suspension points are the "problematic" ones. You already need to assume suspensions somewhere between the entire loop structure, i.e., between pre-loop, the loop body, and the post-loop*. The same could be said about closure omission, that people could assume there is no suspension due to the lack of ceremony when there actually is one.

* This is also the reason I lean toward awaiting the entire control flow (await if ... {} above)

68

That's an interesting rule to consider. However, deinits should not be able to cause problems of this sort, because although a class deinit can reference the actor, deinits are (at least currently) never async. Therefore the deinit cannot cause the current line of execution to give up control of the actor, so the deinit cannot interleave any accesses to the current actor's isolated state, because it would have to have been able to await its turn in the actor's queue to do so.

3 Likes
69

While the deinit won't introduce a suspension point, whether the deinit happens before or after the implicit suspension point from async let is important. In the example, I strongly believe the deinit should happen before the implicit suspension point so the implicit await is truly outside of the function's logic and within the call boundary awaited by the caller.

70

From the perspective of where interleaved accesses to actor state can happen, when the deinit occurs doesn't matter. Also keep in mind that, in a Swift 6 world where data races are statically prevented, the scope of unprotected global state that a deinit could potentially modify, without being able to await on a message to another actor, will be much smaller than it is today.

1 Like
71

Here's where I think we stand right now. There are basically two alternatives that seem palatable.

  1. We say that statements like async let have to be understood to have implicit suspension points on exit from their scopes. There is no need to either mark edges out of these scopes with await or mark the scopes themselves.

  2. We say that edges out these scopes must be marked with await, but we carve out big exceptions so that code isn't drowning in awaits: they aren't necessary on function-exiting edges, and they aren't necessary if all the async lets in the scope have already been awaited.

I think marking the blocks that contain async let is intuitively attractive but doesn't actually work. The most natural thing here is for the mark to be at the top of the block, but then it blends in with the statement that leads into the block. In code like the following, why would someone interpret the await as being somehow tied to the declaration of x and exits from the block?

if let x = foo() await {
  ...
  async let x = baz()
  ...
}

This doesn't make any sense, and it's just going to confuse people about what await means. Furthermore, if we can expect people to scan up to find something like await and understand that there are implicit suspensions on exits from the block, why can't we expect them to scan up to find an async let?

And I think forcing await to be marked on every edge out of an async let scope, even things like a return or an unhandled try, introduces a rather high burden in order to make a rather pedantic point. Again, it risks overwhelming the code with keywords, and it makes await pretty meaningless as a marker. To salvage this idea, we have to lower that burden and make the await feel more meaningful. Achieving that can be quite complicated, though; let me sketch out the rules I think we'd need.

Assume that a statement like async let is in scope. Then:

  • Running out of such a scope must be marked with a standalone await statement unless it leads immediately to an exit from the function:
    func run1() async {
  async let a = <expression>
  // falling out is an immediate function exit,
  // so a standalone await is not required
}

    func run2() async {
  if <condition> {
    async let a = <expression>
    // falling out leads to an immediate function exit,
    // so a standalone await is not required
  }
}

    func run3() async {
  if <condition> {
    async let a = <expression>
    <code>
    // falling out does not lead to an immediate function
    // exit, so a standalone await is required
    await
  }
  <code>
}

    func run4() async {
  switch <expression> {
  case .a:
    async let a = <expression>
    // falling out does not lead to an immediate function
    // exit, so a standalone await is required
    await
  }
  <code>
}
  • A return that exits such a scope is always function-exiting and never needs to be additionally marked:
    func run5() async -> Int {
  if <condition> {
    async let a = <expression>
    // return is an immediate function exit, so return await
    // is not required unless the return operand can suspend
    return <expression>
  }
  <code>
}
  • A continue that exits such a scope must be marked await continue:
    func run6() async {
  for e in list {
    async let a = <expression>
    guard <condition> else {
      // continue does not lead to an immediate function exit,
      // so await continue is required
      await continue
    }
    <code>
  }
}
  • A break that exits such a scope must be marked await break unless it leads to a function exit:
    func run7() async {
  for e in list {
    async let a = foo(e)
    guard <condition> else {
      // break does not lead to an immediate function exit,
      // so await break is required
      await break
    }
    ...
  }
  foo()
}
    but
    func run8() async {
  for e in list {
    async let a = foo(e)
    guard <condition> else {
      // break leads to an immediate function exit, so
      // await break is allowed but not required
      break
    }
  }
}
  • A throw that exits such a scope must be marked with await if it can be caught within the function:
    func run9() async {
  async let a = foo(e)
  // throw immediately exits function, so throw await
  // is not required unless if the throw operand can suspend
  throw await <expression>
}

    func run10() async {
  do {
    async let a = <expression>
    // throw does not immediately exit function, so
    // throw await is required even if the throw
    // operand cannot suspend
    throw await <expression>
  } catch {
    // presence of code here does not affect decision
    <code>
  }
}

    func run11() async {
  async let a = <expression>
  do {
    // throw does not exit scope of async let, so await
    // is not required unless throw operand can suspend
    throw <expression>
  } catch {
  }
  // falling out of function is an immediate function exit,
  // standalone await is not required
}

    func run12() async throws {
  async let a = <expression>
  do {
    // throw may exit scope of async let, but if so, it leads
    // to an immediate function exit, so await is not required
    throw <expression>
  } catch (let x as MyError) {
  }
  // falling out of function is an immediate function exit,
  // standalone await is not required
}
  • An exception edge that exits such a scope must be marked with await if it can be caught within the function. Note that such an edge must already be marked with try, so we can think of this as being part of the try:
    func run13() async throws {
  async let a = <expression>
  // exception edge is an immediate function exit,
  // so try await is not required
  try <expression>
}

    func run14() async throws {
  do {
    async let a = <expression>
    // exception edge may not be an immediate function exit,
    // so try await is required
    try await <expression>
  } catch {
    // presence of code here does not affect decision
    <code>
  }
}

    func run15() async {
  do {
    async let a = <expression>
    // try? and try! do not introduce exception edges, so
    // so try await is not required unless the expression
    // itself can suspend
    let v = try? <expression>
  } catch {
    <code>
  }
}
  • If every async let in a scope has already been awaited on all paths leading to an edge, no await is required on the edge.
    func run16() async {
  if <condition> {
    async let a = <expression>
    <code>
    await a
    // falling out does not lead to an immediate function
    // exit, so a standalone await would be required,
    // except that the async let has already been awaited
    // on all paths leading to this point
  }
  <code>
}

Unfortunately, I think it's fair to argue that the complexity of these rules creates its own sort of burden. You don't have to memorize them all — you can just remember that all edges that exit an unawaited async let but don't exit the function must have an await — but still, it's a lot.

My personal inclination is still towards accepting that statements like async let can have implicit suspensions on scope exit, but I wanted to make a good-faith effort towards developing the alternative.

22 Likes
72

I agree that we probably do not want that many rules. Also because they will likely interfere with simple code refactoring, so you have to maintain all those awaits too.

As I see it, we only need this for loops as their behavior could be very unexpected to beginners. @michelf has made several great suggestions there.

I could live with implicit suspensions, but I do not feel it serves Swift well to have such unexpected behavior.

1 Like
73

Could it make sense to have two different types of async let’s declarations with different rules?

Let’s say any async let a = <expression> must be followed by an await a statement in the current scope. Not doing so is a compiler error.

In addition, we could have something like a defer async let a = <expression> syntax. Awaiting this value is fine, but the compiler would insert an implicit await a at the end of the current scope.

This approach would still have implicit suspension points, but it would be easier to identify code scopes where implicit awaits might be an issue. This would encourage developers to understand the difference between the two in addition to building on the existing defer keyword.

Putting it differently you could say that defer async let a = <expression> would be syntax sugar for

async let a = <expression>
defer { await a }
1 Like
74

What would this really give you, though? Is it really reasonable to ask developers to remember 2 kinds of async lets, and to also remember that one of them can involve implicit awaits? Is it really a meaningful improvement over remembering one thing: that async let awaits on exiting its scope?

Less rules means an overall easier system to work with. If you have the foresight to use async let vs. defer async let, you likely have the foresight to use explicit awaits (and to use tools which verify it, like a linter or such).

75

TBH, I don't find your proposed rules to be very confusing (with one exception which I'll get to). My rationale is that I don't think we actually have to remember anything — we can let the compiler do the work for us, since it can tell right from wrong.

I'm thinking about this from up-thread:

I agree with this sentiment in general, but in this special case I think perhaps it's exactly right to let the compiler give errors for which we just use fixits to shut the compiler up! That is to say, the compiler knows what I mean, and I'll just add some annotations to make sure that future-me and other readers of the code can comprehend the flow more easily.

Similarly, I'd expect the compiler to warn about unnecessary awaits when the code changes, so that I could remove them, again by fixits. Maybe this would be a code maintenance headache for some code bases (as @JJJ suggests), but I don't see this as obviously so.

The one case I don't think works is try await <expression>, when the await is not relevant to the expression, but just marks unawaited lets. I'd prefer:

    await
    try <expression>

I think await try <expression> would work too, but it's probably too subtle.

[Edit: Similarly for throw await <expression>.]

I realize you'd prefer that we preferred your other option, but I can't help it if your "bad" ideas are still pretty good.

4 Likes
76

I agree with @QuinceyMorris that this seems to me to be a reasonable rule (though I was already on the side of preferring suspensions to be marked), and I really appreciate the effort you put into making a fair case for a position you didn’t necessarily support!

+1 to this. Requiring the suspension points to be marked might create a burden for the author of the code initially, but even if the original author doesn’t have a good understanding of why the errors are produced or what the fix-its mean, the fixed code still ends up better for future readers.

6 Likes
77

To me, the basic idea that async let's task must be strictly scoped within each scope in a function is a flawed one. I think the objectives regarding async let should be:

  1. The task must terminate before the function returns to the caller. If you don't explicitly await on it, I don't think it matters much when the implicitly-cancelled task finishes, as long as it does not linger beyond the function itself.

  2. You can't create an explosion of tasks (loops creating an unbounded number of tasks all running at the same time). Those must be throttled somehow.

Awaiting unawaited tasks at every scope surely accomplish the above goals, but it creates the following problem:

  • each end of scope can become a suspension point, either an implicit one or one that must be annotated

I think we need to reduce the places where those suspension points occur. The more suspension points you have (either implicit or annotated), the harder it make the code to read and understand when reviewing (for each you need to consider the impact of the actor state changing).

I made two suggestions earlier with that perspective of reducing the awaits for unawaited async let tasks:

  1. one about only awaiting when exiting a loop scope.
  2. one where you have to await to start an async let task within a loop.

I find the second one in particular is very easy to explain and reason about since it does not require following every path of the control flow: all you need to know is whether the async let is inside of a loop. My preference would go to this second solution, but sadly no one commented on it.

2 Likes
78

I think moving the await to the end of the function is still the problem. Not very ideal when you’re dealing with concurrent code/reasoning.

79

Can you come up with an example of a problematic refactoring?

80

Would it be reasonable to only require this more explicit await marking when in an actor-isolated context? I think this is the place where an unexpected suspension is potentially going to cause unexpected behaviour.

As others have mentioned, I think it’s OK for people to rely on the compiler knowing these rules. I think I’d would find them useful when refactoring to ensure I’ve either not accidentally introduced a suspension point, or at least acknowledged when one has been introduced. This would also apply to moving async let code from a non-isolated context to an isolated one.

In terms of the language evolution, it would be source-breaking to add the requirement for the extra await annotations, so it might be preferable to err on the side of caution and have them, with the option to relax this requirement in the future if it’s too onerous.

2 Likes