Capturing self inside a Task launched from an @MainActor method can cause retain cycles or threading issues

Using Swift
5

I understand why linters suggest the weak-self dance, but my experience is that in a lot of cases it’s the wrong choice. Ignoring Swift concurrency for the moment, let’s focus on closures.

The weak-self dance is necessary:

  • If object A references a closure which references A [1].
  • There’s no explicit mechanism to break that cycle.

That second point is key. There are three types of closure-based APIs:

  • Those that take a closure, perform an operation, and then release the closure.
  • Those that take a closure and hold on to it indefinitely, only releasing the closure in response to some explicit action on your part.
  • Those that take a closure and hold on to indefinitely, only releasing the closure when you release the parent object.

IMO the weak-self dance is only relevant for the last one. Lemme explain each in turn.

Many APIs that take a closure only hold on to that closure while some operation is in flight. The canonical example of this is the URLSession convenience methods. Consider this code:

class MyHTTPRunner {

    func start(request: URLRequest) {
        URLSession.shared.dataTask(with: request) { dataQ, responseQ, errorQ in
            self.handleResults(data: dataQ)
        }
    }
    
    func handleResults(data: Data?) {
        … process the results …
    }
}

Note In my examples I’ve left off various Swift concurrency decorations, just so you can focus on the core code.

There’s little point doing the weak-self dance in the closure because:

  1. URLSession requests are guaranteed to run to completion [2].
  2. When the request completes, it releases the completion handler closure.
  3. Which breaks any potential retain cycle.

The second type of API is one with an explicit mechanism to break the retain cycle. The canonical example of this is Timer. This forms a retain cycle with every run loop on which it’s scheduled, and you have to call invalidate() to break that cycle.

Consider this code:

class MyViewController: NSViewController {

    override func viewDidLoad() {
        super.viewDidLoad()
        Timer.scheduledTimer(withTimeInterval: 5.0, repeats: true) { _ in
            self.updateCounter()
        }
    }
    
    func updateCounter() {
        // …
    }
}

Doing the weak-self dance in the closure is actively bad. It breaks the retain cycle involving MyViewController, but it fails to break the retain cycle between the timer and the main thread’s run loop. That takes an easy-to-debug retain cycle, one involving your MyViewController type, and turns it into a much more obscure one, involving only Foundation types )-:

And that brings us to the third type of API, the ones where the weak-self dance makes some degree of sense. The canonical example of this is the closure-based NotificationCenter API. For example:

[Note: I edited this example to fix a problem. See downthread for details. Thanks tera!]

class MyTimeZoneWatcher {

    init() {
        self.observation = NotificationCenter.default.addObserver(forName: .NSSystemTimeZoneDidChange, object: self, queue: .main) { [weak self] note in
            self?.timeZoneDidChange()
        }
    }
    
    var observation: AnyObject? = nil
    
    deinit {
        if let observation {
            NotificationCenter.default.removeObserver(observation)
            self.observation = nil
        }
    }
    
    func timeZoneDidChange() {
        … act on the notification …
    }
}

The deinitialiser can’t run until the notification centre releases the closure, and the closure can’t run until the deinitialiser has removed the observation.

Of course, there are other ways to break this retain cycle. You can explicitly call removeObserver(_:), just like you explicitly call invalidate() on Timer. Which option is best is a matter of style, at least IMO, but I’ll accept that the weak-self dance isn’t actively bad in this case.

So, with the basics out of the way, let’s return to Task. Or not, actually, because Task has pretty much the same rules as any other API that takes a closure: You only have to do the weak-self dance if the task can run indefinitely.

Oh, and re-reading your question I see there’s one part that’s not related to the weak-self dance:

Since Task is implicitly isolated to the @MainActor, and
the Task block is started within an @MainActor function,
are there any threading concerns … ?

No.

In your MyOtherViewController example, literally all the code is isolated to the main actor, and thus I’m not sure what sort of threading issues you’re concerned about.

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple

[1] Either directly or indirectly, for example, A could reference B which references C which references a closure which references A.

[2] Assuming the operation completes in some reasonable amount of time. Now, you could argue that URLSession requests can take a long time to complete, and thus holding on to a ‘heavy’ object, like a view controller, for that length of time is problematic. Of course I’d then counter that you shouldn’t be doing networking in your view controllers in the first place (-:

13 Likes