`async let` and scoped suspension points

This whole dilemma is still just about the situation of async lets that aren't awaited on every path, right? My question is how common will this situation be? If not very common, then a more verbose/naggy solution could be tolerable. Otherwise I'm of two minds.

2 Likes

Maybe I’m misunderstanding/oversimplifying the issue, but it seems to me like Swift already has the tools to express whether something should be explicitly awaited:

  • If you name the variable (async let a = ...) you obviously care about the results, and so it makes sense that you need to await the variable along every edge.
  • If you don’t name the variable (async let _ = ...) you obviously don’t care about the results, and so it makes sense that you don’t need to await the variable.

If on some edges you care about the results, and on other edges you don’t care about the results, explicitly writing _ = await a to call out that you’re awaiting but throwing away the results seems to be exactly the type of thing that I as a reader of the code would want.

This also makes implicit awaits a style question. Projects that are stricter about making awaits explicit can disallow async let _ in their style guides, and lint for it automatically.

3 Likes

I think moving the await to the end of the function is not a requirement for @michelf's proposal. It could just be:

  1. Implicit suspension points are added for async lets without awaits
  2. Inside loops you have to spell it await async let, and this will be a suspension point as it will await any task created by a previous iteration of the loop
  3. An implicit suspension point is added when exiting the loop, to await any remaining tasks

So e.g.:

for x in y {
  if predicate(x) {
    await async let a = f(x) // LINE 3
    // possibly use a here
  } else {
    await async let b = g(x) // LINE 6
    // possibly use b here
  }
}

If the first iteration creates a task at line 3, and the second iteration creates a task at line 6, then the first task is cancelled at the end of the first loop (or as soon as we realize we do not care about its result?), but we do not await on it until either

  1. An iteration wants to create another task at line 3, or
  2. Exiting the loop

I guess I could also live with this simple rule set:

  1. Implicit suspension points are added for async let s without explicit awaits
  2. Inside loops the async let shorthand is not allowed (Edit: or should we just require that it's explicitly awaited on all paths inside loops?)

That's a good point: how common will it be to have a non-awaited async let? Hard to say for sure, but certainly a minority of cases. Which makes it more likely people may not realize the presence of a suspension point when the control flow creates one. So it really ought to be explicit when it happens.

3 Likes

Agreed, non-awaited async let will be pretty rare, since people will just use the Task { } initialiser for those kind of "fire-and-forget" operations. So, I think its preferable that the compiler be strict on these rules since they won't practically affect programmers as much.

And these programmers will definitely be glad the compiler is so strict when playing around with an async function inside an actor. Reentrancy is one of the big risks taken in Swift's actor implementation, so I think it's reasonable we help programmers as much as possible.

1 Like

Do you mean if you’re in a loop the task is somehow created but not started, then only if it is awaited before exiting the scope will it be started? I looked at the Task documentation and I don’t understand how that would work.

Maybe I should have explained (2) better. I'll try with an example:

func test() async {
   for i in 0..<10 {
      await async let x = work()
      if Bool.random() {
         print(await x)
      }
   }
}

This would become semantically equivalent to this:

func test() async {
   // task for async let is stored outside of the loop:
   var _task: Task<Result, Never>? = nil

   for i in 0..<10 {
       // async let: awaits on the task from previous iteration
       _ = await _task?.value
       _task = Task { await work() }

       if Bool.random() {
           await print(_task!.value)
       } else {
           // unused on this path: implicit cancellation
           _task!.cancel() 
       }
   }

   // end of function: implicit await
   _ = await _task?.value
}

Of course, async let does not need to be implemented like this in term of Task by the compiler. In particular, the task can allocated on the stack since it is scoped to the function. But the semantics should be similar.

2 Likes

What I was worried about the task bounding to the function scope is that the scope of the task is enlarged when you're folding the callee into the caller. This means the task can live for much longer than expected, potentially getting cancelled (which could be bad should the task be effectful) when it wasn't originally.

func callee() async {
  async let x = ...
  // No cancellation, x always finished
}
func caller1() async {
  if ... {
    callee()
    // x is always completed
    ...
  }

  doSomethingThatCurrentTask()
}

func caller2() async {
  if ... {
    async let x = ...
    ...
  }

  doSomethingThatCurrentTask()
  // x may be cancelled
}

or you'd be adding a new suspension point when someone refactors code into a function:

@Actor1
func caller1() {
  doSomethingOnActor1()
  if ... {
    async let x = ...
  }
  doSomethingElseOnActor1()

  // suspension point
}

func caller2() {
  doSomethingOnActor1()
  if ... {
    await callee() // new suspension point
  }
  doSomethingElseOnActor1() // Now called after suspension point
}

There a new await is added, but at this level of refactoring, it'd be hard-press to expect someone to check again if the invariance breaks across the new suspension point.


Now, TBF, this isn't exactly unique to the awaiting async let at the end of the function scope, you could also move the suspension points by moving only the async let portion of the same scope:

func caller1() {
  if ... {
    doSomething()
    async let x = ...
    doSomethingElse()
    // suspension point
  }
}

func caller2() {
  if ... {
    doSomething()
    await callee()
    // suspension point
    doSomethingElse()
  }
}

but async let applying to the end of the scope would amplify how easy it is for such thing to happen.

Here is a crazy idea: (still working my way through the thread, so apologies if it has been suggested)

What about adding a defer await _ statement that can be used to signify that you want to await either a particular value (defer await x) or all unawaited values (defer await _) before exit. The compiler would prompt you to add it if there are paths for a async variable which aren't awaited.

Just wanted to add it to the pile of ideas

1 Like

My understanding of a function like this one is that the task would get cancelled immediately, since you never await explicitly on it. And the compiler should complain of no path is awaiting x. If you want x to complete without being cancelled, you need to explicitly await somewhere. Or did I misunderstood the plan?

Once you explicitly await, it's pretty clear what happens when refactoring. So if it's unacceptable that implicitly cancelled tasks are awaited at less predictable times, we should make this await for cancelled task explicit then. That'd also help with the issue of changing the control flow within the same function. But I'm not sure it's an issue given it only applies to cancelled tasks.

Correct — what you describe @michelf, i.e. the cancellation behavior, that not using a variable is bound to produce a warning, and that such warning would be improved to mention that such task is going to be cancelled is all pieces I believe we’ve been pretty aligned on. At least as far as all discussions I’m personally aware of, in and off thread.

1 Like

Ehh, would that also apply to taskGroup? I hope it does, I really do :pleading_face:. I've always been a big fan of consistency if you can't tell :stuck_out_tongue_closed_eyes:.


In that case, though we could still add new suspension points (yes, with explicit await) from refactoring, it would have been much less of a problem than I originally thought.

1 Like

I'm not worried about the deinits suspending. I'm worried about logical actor invariant holding. The entire purpose of an actor as an "island of single threadedness" is that they maintain logical invariants between suspension/reentrance points. A class deinit can touch general actor state, and if it is implicitly run after a suspension point, then the invariants it expects to holds may be broken.

Swift today doesn't have strong RAII features because we intentionally give the compiler the ability to move deinit calls early, but it does have weak RAII -- and class deinits are the only way to affect certain patterns, and just saying "don't use deinits if you want correctness in actors" seems like it would significantly erode the correctness goals of actors in general.

Furthermore, when/if we we add first-class ownership support, this issue will rear its head in a much bigger way, because structs will get deinits, and we'll probably want to have more predictable lifetime for them. The interaction here seems like a really fundamental aspect of the async let design that needs to be figured out, I don't think we can just brush it away.

I think the "fix" in this case is straightforward: we take advantage of Swift's flexible rules for deinit running to make sure they run before any async let implicit suspensions happen. This ensures that the deinit logic sees the actor state with the expected invariants. We just need to /define/ that behavior and make sure it is implemented predictably.

-Chris

6 Likes

Agreed, I think those are the only plausible answers.

Agreed.

I don't understand your concern. You go on to enumerate the things that can exit a scope in Swift. You can summarize the rules as "anything that exits an async let scope needs to be marked await"... if the async let hasn't already been awaited.

That said, this isn't the experience a Swift developer would have. I think we the vastly most common things code will do is:

  1. actually use the value of an async let, and all exits dominated by the uses of the async let won't need marking.
  2. Use throwing functions that exit all the way out of the current function. I don't think these exits ever need to be marked, because the caller already has an await and thus is prepared for suspension (though we do need to define the deinit rule as mentioned above).

My belief is that if you cover these two cases, the remaining situations (e.g. conditional uses of the async let) will be uncommon. Furthermore, the Swift compiler will tell the developer where (and how) to insert the marking with error messages and fixit hints. If the required marks are pervasive, then the compiler will effectively be putting backpressure on the developer to refactor their code to make the logic simpler: this seems like a win to me.

The benefit of this is the same as any marking with try/await: a reader of the code later will see the few marks in the code, and will understand that suspensions are possible. This allows them to reason about the logic of the code, and make sure the high level invariants in the actor are being held.

Has anyone done any study of how many marks would actually be required with the above two rules?

-Chris

9 Likes

I agree, if we give people the right amount of example code and documentation showing that an un-awaited async let should be a Task { }, practically speaking un-awaited async lets will be rare and can even be considered an anti-pattern (at least I do).

Afterwards, rarely will a function have more than two or three try's / returns / breaks before an await for an async let. If it does, developers should seriously consider refactoring their code.

I think I prefer an exhaustive model which guides/nudges developers towards the right patterns than a so-called "incomplete" one.

1 Like

If on some edges you care about the results, and on other edges you don’t care about the results, explicitly writing _ = await a to call out that you’re awaiting but throwing away the results seems to be exactly the type of thing that I as a reader of the code would want.

This also makes implicit awaits a style question. Projects that are stricter about making awaits explicit can disallow async let _ in their style guides, and lint for it automatically.

I've been reading through this thread and thinking the same thing the entire time. This seems like the best suggestion I've read so far. Am I missing something, are there any issues with it?