Task async retain cycles

A subtlety of async functions is that the executing Task retains the function and its dependencies — this does make sense as tasks guarantee execution.

But if one is not careful, retain cycles can be easily created until the Task is cancelled, or the function completes. (AsyncSequence may not complete!)

If an object spawns a new Task executing one of its own async functions, then cancellation needs to be explicit and cannot by tied to its own lifetime;

class Worker {

  private var handle: Task.Handle<Void, Never>?

  func start() {
    precondition(handle == nil)
    handle = async {
      await doSomeWork() //no warning but cycle created
    }
  }

  func cancel() {
    handle?.cancel()
    handle = nil
  }

  func doSomeWork() async {
    for await result in someAsyncSequence {
      ...
    }
  }
  
  deinit {
    cancel() // no effect as Task retains Worker 
  }
}

The subtlety is not captured by compiler diagnostics, maybe because the common [weak self] dance will only break the cycle momentarily, as soon as doSomeWork() executes and suspends the cycle persists.

handle = async { [weak self] in
  await self?.doSomeWork() // nice attempt, but cycle will persist
}

Separating the task creation and the async breaks the cycle:

class Controller {

  private var handle: Task.Handle<Void, Never>?

  func start() {
    precondition(handle == nil)
    handle = async {
      await Worker().doSomeWork()
    }
  }
  
  deinit {
    handle?.cancel()
  }
}

class Worker {
  func doSomeWork() async {
    for await result in someAsyncSequence {
      ...
    }
  }
}

It would be nice to have better compiler support to alert developers to these pitfalls.

11 Likes

If anybody is interested in taking on diagnostics for potential reference cycles, see SR-1807 and give me a ping either on here or via direct message.

I'm not sure that this is quite the same as SR-1807, because there isn't a concrete reference cycle in cases like this, more of a logical one—we're relying on cancellation to make the task give up its reference to the object, but the only thing triggering cancellation is the deinit on the same object. Even if there were a garbage collector, it could not clean this up, because there's no formal leak as long as the task is running. Maybe we could warn specifically about trying to cancel tasks inside deinits? (Though how would you silence the warning when you know what you're doing?)

3 Likes

I second Joe’s thoughts: a retain cycle for an active task making progress is perfectly fine; it’s only when that task becomes infinite that it’s a problem. If the compiler starts complaining about this, I’d definitely want to make it easy to say “here’s why it’s not a problem”.

5 Likes

I brought up the same „problem“ with infinite sequences and retain cycles in my other post.

The ergonomics around using async and infinite task resulting in retain cycles that are only avoidable by weak self or framework lifecycle management like SwiftUI, feels a bit lacking. It would be great if we could achieve something where you can subscribe to a infinite sequence using async and letting the Task get cancelled as soon as no other reference to the object is there. Would be interesting to hear the compiler folks point of view on this.

I was initially surprised to see that async { } was not following the existing @escaping closure warnings of self, but I also appreciate that the common attempt to silence it will be the [weak self] dance which may be worse — as it leaves the developer thinking the cycle has been broken while it has not.

Completely agree when this is the intention of the developer.

The URLSession example is thrown around as a migration candidate for async — I cannot speak for all but the majority I’ve seen typically cancel when the user gives up and a ViewController is deallocated.
A naive migration will likely come across this problem retaining self and performing some unneeded work.

1 Like