5

I have a retained cycle so my viewcontroller's deinit won't be called, and I'm trying to resolve this my adding [unowned self], but I'm not too sure where to put the unowned in my cases:

Case 1

class YADetailiViewController: UIViewController {
 var subscription: Subscription<YAEvent>?

 override func viewDidLoad() {
    super.viewDidLoad()
    if let query = self.event.subscribeQuery() {
        self.subscription = Client.shared.subscribe(query)
        self.subscription?.handle(Event.updated) {
            query, object in
            DispatchQueue.main.async {
                [unowned self] in// Put unowned here won't break the cycle, but it does compile and run 
                self.pageViewLabel.text = String(object.pageViews) + " VIEW" + ((object.pageViews > 1) ? "S" : "")
            }
        }
    }
 }
}

Case 2

 override func viewDidLoad() {
    super.viewDidLoad()
    if let query = self.event.subscribeQuery() {
        self.subscription = Client.shared.subscribe(query)
        self.subscription?.handle(Event.updated) {
            [unowned self] query, object in // Put unowned breaks the cycle, and deinit is called
                DispatchQueue.main.async { 
                    self.pageViewLabel.text = String(object.pageViews) + " VIEW" + ((object.pageViews > 1) ? "S" : "")
                }
            }
        }
    }

I'm curious what's the differences between these two scenarios and why one works but not the other

Kesong Xie
  • 1,316
  • 3
  • 15
  • 35
  • 1
    In the first one, you have omitted `in` after `[unowned self]`. Is that deliberate? – matt Feb 11 '18 at 22:19
  • I think the difference has to do with when `self` is captured. In the first one, `self` has already been captured in the outer closure by the time you declare `self` to be `unowned` in the inner closure. In the second one, you prevent that by declaring `self` to be `unowned` in the outer closure. See http://www.swiftarchive.org/Nested-functions-and-reference-capturing-td667.html which may be relevant here (though I could be wrong) – matt Feb 11 '18 at 22:23
  • @matt, good catch, just added the `in` – Kesong Xie Feb 12 '18 at 05:58
  • Apart from the issue consider that the `async` closure of GCD does not cause a retain cycle so adding `[unowned self]` there is meaningless anyway. – vadian Feb 12 '18 at 08:46

1 Answers1

1

Indeed, as correctly mentioned by @matt the problem is related to the time when self is captured. In fact int this code self is captured twice:

  • When the outer closure is passed to the handle method
  • When the inner closure is passed to the async method (during the handle closure execution)

The outer closure needs self to pass it to the inner closure, otherwise the inner closure won't be able to capture it.

The nature or retain cycle is the following: self(YADetailiViewController) -> subscription -> closure (handle parameter) -> self. To break this cycle it's enough to not retain self in that (outer) closure. That's why the second code sample works.

The second capture of self (in the inner closure) happens only when the outer closure is executed and lasts until that async block is executed - usually it's quite a short time.

In the first code sample you're not breaking the cycle. The second capture of self doesn't happen but the first one (causing thy cycle) is still present.

And if your handle closure can still be called when the view controller is already deinited/released then as suggested by @AdrianBobrowski you should use weak instead of unowned to prevent possible crash.

algrid
  • 5,600
  • 3
  • 34
  • 37