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

Seeking definitive clarification on the memory management behavior and thread safety issue when an instance method of a class marked with or confined to the @MainActor is called implicitly within a Task closure defined inside another @MainActor method.

Specifically, we observe that static analysis tools like SwiftLint's (explicit_self rule or Use weak self in closures Violation: Use weak self when create a closure.(weak_self_closure) often flag the use of self. inside the Task block as a potential, unhandled strong reference cycle, requiring the use of [weak self].

My understanding is that a strong reference cycle requires A (the class instance) to hold a strong reference to B (the closure/task) and B to hold a strong reference back to A (self). Since the Task is an ephemeral block of asynchronous work not captured or stored as a property of self, we do not believe a classic retain cycle exists, provided the Task naturally completes.

Need confirmation on whether the Swift Concurrency runtime creates any internal strong reference that could cause a memory leak if self is captured strongly here and clarify Any thread safety issue occurs ( not a retain cycle).

Retain Cycle Guarantee: Can you confirm that the strong capture of self within the Task { ... } closure does not create a strong reference cycle (i.e., a memory leak where deinit is never called) because the Task is not stored as a property?

Best Practice vs. Necessity: If a retain cycle is guaranteed not to occur, is using [weak self] purely an optional best practice to allow the Task instance to be deallocated before the asynchronous work completes, or is it necessary to prevent a genuine memory leak?

Threading Safety: Since Task is implicitly isolated to the @MainActor, and the Task block is started within an @MainActor function, are there any threading concerns regarding the implicit capture of self before the first await point, or does the runtime handle the isolation guarantee completely regardless of the capture list?

STEPS TO REPRODUCE

Here is the code snippet:

Does using self inside a fire-and-forget Task { MainActor in} closure (Not stored as a property) create a retain cycle / crash issue or any thread safety related issue ?

class MyOtherViewController: UIViewController {

  override func viewDidLoad() {

    super.viewDidLoad()
    doSomething()

  }

  func doSomething() {

    // Fire and forget , Task not stored as a property
    Task { @MainActor in
      self.updateUI()
    }
  }

  func updateUI() {

    // Example UI Update
    self.view.backgroundColor = .systemGreen
    print("UI updated!")

  }

}

If [weak self] is already captured in an outer closure, is it necessary to capture [weak self] again in nested closures ?

class MyViewController: UIViewController {

  override func viewDidLoad() {

    super.viewDidLoad()
    startProcess()

  }

  func startProcess() {

    perfromStepOne { [weak self] in

      guard let self = self else { return }

      self.performStepTwo {
        self.doSomething()  // is this safe ?
      }
    }

  }

  func performStepOne(completion: @escaping () -> Void) {

    // Simulate a step, then call completion
    print("step one complete")

    completion()

  }

  func performStepTwo(completion: @escaping () -> Void) {

    // Simulate a step, then call completion
    print("step two complete")

    completion()

  }

  func doSomething() {
    print("Do Something")

  }

}

Please use code blocks, either in the visual editor, or by using ``` above and below the code in the editor.

1 Like

Unless you're worried about keeping the instance alive as long as it takes for the work to complete, there's nothing to worry about here. You aren't storing the closure anywhere, so there's no risk of a retain cycle. Even if you stored the Task, the closure is cleared once it completes, so there's no risk of a cycle there either (IIRC).

2 Likes

The way I think about it is that a Task block starts executing (almost) immediately so a top “[weak self]” dance does not effectively do anything to prevent the task from capturing self - if it reaches that point, it will execute whatever it has to execute. This is in contrast with closures, which can be invoked at a later time - when the instance is already not in the memory, then the execution can be skipped.

Therefore I do not think the SwiftLint rules around closures are relevant to escaping closures which are executed immediately, like the Task one.

1 Like

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

If I am not mistaken, there’s also this subtlety with “[weak self]”:


let lock: NSLock

var closure: ((Void) -> ())? = { [weak self] in
  // 1
  self?.lock.lock()
  // 2 
  self?.doSomethingWhichTakesSomeTime() 
  // 3
  self?.lock.unlock()
}

if whoever owns the closure above gets deallocated after 1 and before 3, i think the lock will end up in an inconsistent state
this is also relevant whenever transaction-like behaviour is needed i.e not only around locks

and it can be fixed with:

let lock: NSLock

var closure: ((Void) -> ())? = { [weak self] in
  guard let self else { return }
  self.lock.lock()
  self.doSomethingWhichTakesSomeTime() 
  self.lock.unlock()
}

here self will stay alive for the duration of the closure, just like in the URLSession.shared.dataTask example from the message above.

1 Like

If I am not mistaken, there’s also this subtlety with [weak self]:

Yeah, that’s deeply un-fun. In Objective-C there’s a specific warning for that and I’m not sure why Swift never got the same.

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple

2 Likes

I believe you meant to say calling removeObserver(...), no?
And, self.observation = nil in deinit is also redundant, is it not?
Otherwise I 100% agree with your post (excellent as always), and I also saw this weak dance more often than needed.
What's your opinion on using unowned in those cases, that currently use weak to break the cycle, assuming that weak usage is legit. (oh, no, I had some valid unowned cases in mind but the above example is not one of those).

Thx for detailed response!

I would just like to give my two cents on this specifically:

I never thought about the easy-to-debug retain cycle and that seems like an actual benefit!

However I usually find myself using weak self dance in the Timer because what I usually do is store a reference to the timer in the view controller itself (to be able to invalidate it). And the point where I usually invalidate the timer is in the deinit and because of that the reference to View controller inside of a timer needs to be weak to not cause retain cycle.

I find it most convenient to invalidate timer in the deinit but I guess there are other options as well like invalidating it in viewWillDisappear for example I suppose? but then you need to setup timer again in viewWillAppear.

I believe you meant to say calling removeObserver(...), no?

D’oh! I was confusing NotificationCenter with Combine, where you just have to release the token. Thanks for keeping my honest.

I’ve gone back and fixed the post, because I want to be able to reference it in the future.

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple

ps While testing this I hit an interesting deadlock. If you’re curious…

Explanation of a ‘fun’ deadlock

Consider this program:

import Foundation

class Test: NSObject {

    var observation: AnyObject? = nil
    
    func start() {
        assert(self.observation == nil)
        self.observation = NotificationCenter.default.addObserver(forName: Test.myNote, object: nil, queue: .main) { _ in
            print("was notified")
        }
    }
    
    static let myNote = NSNotification.Name(rawValue: "Test.myNote")
}

func main() {
    let t = Test()
    t.start()
    print("will post 1")
    NotificationCenter.default.post(name: Test.myNote, object: nil)
    print("did post 1")
    DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) {
        print("will post 2")
        NotificationCenter.default.post(name: Test.myNote, object: nil)
        print("did post 2")
    }
    withExtendedLifetime(t) {
        dispatchMain()
    }
}

main()

It prints:

will post 1
was notified
did post 1
will post 2

and then stalls.

The second notification is never delivered, with the thread deadlocking inside post(name:object). I believe this is caused by a main thread vs main queue mixup within NotificationCenter, but I haven’t looked at it in too much detail. Certainly, switching to a run loop got me past the deadlock.

1 Like

Great, thank you!

I also typically not bother with deinit in this case... If you remove deinit, the only behavioural change is that notification centre will still be calling the retained closure (so there are still a few bytes on the heap left allocated and a few CPU cycles wasted upon notification positing) but as the main MyTimeZoneWatcher object is already deallocated the guard self (if any) will short-circuit or, equivalently, the self?.timeZoneDidChange() will do nothing.

In those regards I like the behaviour of the older selector-based notification observation machinery

as it does notification unregistering automatically upon deinit (since some now ancient OS version). I remember implementing my own closure based notification observer wrapper type that has this nice property (it was removing observation inside its deinit but from the outer usage perspective that aspect was hidden and done automatically). Pseudo code from memory:

class MyTimeZoneWatcher {

    init() {
        self.observation = NotificationCenter.default.myAddObserver(forName: .NSSystemTimeZoneDidChange, object: self, queue: .main) { [weak self] note in
            self?.timeZoneDidChange()
        }
    }
    
    var observation: MyNotificationObserver? = nil
    
    deinit { /* nothing here */ } // or don't have it at all
}

class MyNotificationObserver {
    private var observer: AnyObject?
    
    init(observer: AnObject?) {
        self.observer = observer
    }
    deinit {
        if let observer {
            NotificationCenter.default.removeObserver(observer)
        }
    }
}
extension NotificationCenter {
    func myAddObserver(forName name: Notification.Name, object: Any?, queue: DispatchQueue?, closure: @escaping () -> Void) {
        let observer = addObserver(forName: name, object: object, queue: queue, closure: closure)
        return MyObserver(observer: observer)
    }
}