Confused why this doesn't cause a memory leak

Using Swift
1

I noticed the other day my iOS app is leaking memory like a sieve (I wrote some automated tests that did long run thrashing and then I noticed the app just grows in size until it dies).

I had long suspected, that I was going to get more disciplined about closure variables. I come from a Smalltalk background where Closures Just Worked(tm). So I've spent the day trying to figure out where my app is leaking memory. The Xcode tools so far haven't been much help. When I do the Debug Memory Graph thing, it tells me I have lots of Arrays of various UIElements, that seem to correspond to outlet collections, but doesn't really make it clear why it's a leak.

But I thought surely I have some leaks due to closures. A while ago I had my own Ticker (a periodic timer) object:

class Ticker {
    // MARK: - Properties Stored
    var interval:Duration = 1.seconds
    private var queue = DispatchQueue.main
    private var source: DispatchSourceTimer!
    var tick:()->() = {}

    // MARK: - accessing
    func start() {
        self.stop()
        self.source = DispatchSource.makeTimerSource(queue: self.queue)
        let nsgap = DispatchTimeInterval.microseconds(Int(self.interval.microseconds.rounded().magnitude))
        self.source.schedule(deadline: DispatchTime.now() + nsgap, repeating: nsgap, leeway: DispatchTimeInterval.seconds(0))
        self.source.setEventHandler(handler: self.tick)
        self.source.resume()
    }

    func stop() {
        if self.source != nil {
            self.source.cancel()
            self.source = nil
        }
    }
}

It should go away probably and be replaced by just direct use of GCD or the native Timer object. Then I have a UIController subclass that first has one of these as a variables:

class MyController: UIViewController {
    var scanTimer:Ticker = Ticker()
    ...

And later has a method that looks like this:

@IBAction func startScanning() {
    self.stopScanning()
    ...
    self.scanTimer = Ticker()
    self.scanTimer.interval = 500.milliseconds
    var tickCount = 0
    self.scanTimer.tick = {
        tickCount += 1
        if tickCount > 20 {
             self.stopScanning()
        }
        if self.bleBecameActive {
            self.bleBecameActive = false
            self.startBLEScan()
        }
    }
    self.scanTimer.start()
}

The way I understand things... my controller has a hard reference to the timer. The timer has a hard reference to a closure (the tick variable). And the closure has a reference to self. Shouldn't that create a cycle?

But the Debug Memory Graph doesn't show this as an issue at all. Why not? Is it a) not really an issue or b) the tool isn't doing what I think it is?

2

The code you’ve posted will create a cycle, but if stopScanning() sets scanTimer to nil, that would break the cycle.

3

It doesn't set it to nil. It can't, because I didn't make it an optional.

4

The way I’d handle this is to make the tick property optional, and have it automatically switch to nil on stop. Or keep it non-optional and switch it back to () -> void (btw () -> Void is idiomatic Swift, not () -> ()). This slightly breaks the semantics of your Ticker class — a client can’t stop and then restart the timer without also setting up tick again — but it’s pretty uncommon for that case to matter.

One way to avoid this particular edge case is to not expose tick but instead make the closure a parameter to the initialiser or to start. With no public tick property, the perceived semantics will matter match the actual semantics.

On an unrelated note, I’d make source optional and then have this sort of structure:

func start() {
    precondition(self.source == nil)
    let source = …
    self.source = source
    … configure `source` …
}

func stop() {
    if let source = self.source {
        self.source = nil
        … unconfigure `source` …
    }
}

This will cleanly crash if someone calls start twice (a clear sign of a programming error) and gracefully handle redundant calls to stop (which I’ve found to be a useful characteristic).

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple

5

Thanks Quinn. Like I said, the class should probably just go away completely. There's lots of ways I could rewrite it. But unless I can figure out how to use the tools to inform me with confidence that there is a leak cycle before hand and then see with the same tools that it goes away when I change the implementation, it's all moot.

Can you confirm that my logic about the current approach creating a cycle is correct? If so... why doesn't the Debug Memory Graph or Profile Leaks tool show that to me then?

6

But unless I can figure out how to use the tools to inform me with
confidence that there is a leak cycle before hand and then see with
the same tools that it goes away when I change the implementation,
it's all moot.

It’s definitely forming a retain cycle. I put your code into a test app and there are clearly instances of both Ticker and MyController retained in memory when they shouldn’t be. I confirmed this — and this is my preferred approach, honestly — by adding log points to their deinitialisers.

When I bring up the memory graph I can see these instances but it does not correctly render the retain cycle. I suspect that this is because the cycle involves a closure (the tick property) rather than being all objects. However, these tools aren’t really my forte, so I can’t give you a definitive answer about that.

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple