Swift MacOX - Popover segue creates multiple view controller instances without destroying them when they are dismissed

Question

I'm creating a popover style view controller in storyboard like this

Then I click the button, the view controller shows and when I click anywhere outside, the view controller is "dismissed".

However, when I click the button again, a new instance of the view controller is lunched and the previous one is still running. I have tried deinit but it's not getting called when the view controller is "dismissed".

How can I either destroy the view controller instance when clicking outside, or "show" the already created instance?

My code in the view controller:

class FileTransViewController: NSViewController {
    override func viewDidLoad() {
        super.viewDidLoad()
        // Do view setup here.
        timer = Timer.scheduledTimer(timeInterval: 0.25, target: self, selector: #selector(updateProgress), userInfo: nil, repeats: true)
        //print(123)
        print("\(self)")
    }

    deinit {
        print("destroyed")
        if let timer = timer {
            timer.invalidate()
        }
    }

    @objc func updateProgress() {
        print("updating progress")
    }

}

matt · Accepted Answer · 2019-04-03T15:05:41.737

3

The problem has nothing to do with popovers. You are leaking because you retain the timer while the timer retains you — a classic retain cycle.

To break the cycle, you must invalidate the timer. You cannot do this in deinit, because by definition it cannot be called until after you break the cycle. NSPopover.willCloseNotification might be a good opportunity.

edited Apr 03 '19 at 15:05

answered Apr 03 '19 at 14:50

matt

515,959
87
875
1,141

Gotcha! Thanks for the hint! I was able to break the circle during `NSPopover.willCloseNotification` – Fangming Apr 03 '19 at 14:56

Robert Dresler · Answer 2 · 2019-04-03T15:28:08.513

1

As @matt says, you have problem with retain cycle. You can avoid it using Timer with block where you can declare weak reference for self

timer = Timer.scheduledTimer(withTimeInterval: 0.25, repeats: true) { [weak self] timer in
    guard let self = self else {
        timer.invalidate()
        return
    }
    print("updating progress")
}

You also don't need deinit in this case since you're invalidating timer inside guard's else block and you also don't need variable timer and you can just write Timer if you don't want to invalidate timer manually somewhere else

override func viewDidLoad() {
    super.viewDidLoad()
    Timer.scheduledTimer(...) { ... }
}

edited Apr 03 '19 at 15:28

answered Apr 03 '19 at 15:07

Robert Dresler

10,580
2
22
40

Thanks! I like this answer too! – Fangming Apr 03 '19 at 15:24
@Fangming you also don't need `deinit` in this case since you're invalidating it inside `guard else` block. You also don't need variable for `timer` and you can just write `Timer` if you don't want to invalidate timer manually somewhere else – Robert Dresler Apr 03 '19 at 15:26
1

Yup. Thanks! A lot cleaner now – Fangming Apr 03 '19 at 15:27
Yes, this approach is illustrated by my example project https://github.com/mattneub/Programming-iOS-Book-Examples/blob/master/bk1ch12p498timerLeaker2/ch12p325NotificationLeaker/FlipsideViewController.swift. The other approach is illustrated by https://github.com/mattneub/Programming-iOS-Book-Examples/blob/master/bk1ch12p498timerLeaker/ch12p325NotificationLeaker/FlipsideViewController.swift. – matt Apr 03 '19 at 17:38
@matt yes, thank you for this comment. Anyway, I like with my approach that you don't need variable for `Timer` and you can simply invalidate it if `self` gets deinitialized and you can also get rid of declaring `deinit` – Robert Dresler Apr 03 '19 at 18:35

Swift MacOX - Popover segue creates multiple view controller instances without destroying them when they are dismissed

2 Answers2