Unit 24 · Reeler

AI checkpoint: the leaking image loader

Note

Unit 24 · Project 4, AI checkpoint. This is Pair mode's core skill: you asked Claude for an image loader with a completion callback, and it produced the code below. It compiles. It runs. It even passes a quick manual test. Your job is to review the diff before accepting it — because there is a real leak hiding in plain sight.

You prompted: "Write a small image loader class that downloads a poster and calls a completion handler when done, so a cell can refresh itself. Keep the completion so the cell can be told to reload later." Here is what came back. Read it as a reviewer, not a reader.

The AI-generated code (review this)

import UIKit

final class PosterLoader {
    private(set) var image: UIImage?
    var onReload: (() -> Void)?

    private let url: URL
    private var cache: [URL: UIImage] = [:]

    init(url: URL) {
        self.url = url
        // Store a reload closure the cell can call to force a fresh download.
        self.onReload = {
            self.image = nil
            self.cache[self.url] = nil
            Task { await self.load() }
        }
    }

    func load() async {
        if let cached = cache[url] {
            image = cached
            onReload?()
            return
        }
        guard let (data, _) = try? await URLSession.shared.data(from: url),
              let downloaded = UIImage(data: data) else { return }
        cache[url] = downloaded
        image = downloaded
        onReload?()
    }
}

It looks reasonable. The loader downloads, caches, exposes the image, and stores a reload closure the cell can invoke. Ship it?

Guiding questions

Work through these before reading the explanation. Write down your answers — the discipline of articulating the flaw is the skill Pair mode is training.

PosterLoader stores onReload, and onReload's body refers to self three times. What holds a strong reference to what, in a full circle?
When a scrolling list creates and discards hundreds of PosterLoaders, what happens to the discarded ones? Would a breakpoint in a deinit ever fire?
Why do the tests still pass and the app still work, despite the bug? What kind of failure is this — a crash, a wrong result, or something quieter?
What one change to the init breaks the circle without changing behavior? What happens to the self references inside the closure after that change?

Explanation

The flaw is a retain cycle. PosterLoader holds onReload (a stored property → a strong reference to the closure). The closure's body captures self strongly by default (Swift closures capture reference-type self strongly unless told otherwise). So:

PosterLoader ──(strong, via onReload)──▶ closure ──(strong, via self)──▶ PosterLoader

Each keeps the other's reference count above zero. When the cell that created the loader goes away, ARC decrements — but the count never reaches zero, because the two objects are propping each other up. The PosterLoader and its cached UIImage are leaked: never deallocated, never freed. A deinit { print("gone") } would never print. In a scrolling grid that mints a loader per cell, memory climbs for the whole session and never comes back down.

Why it still "works." This is the insidious part, and why review matters. A leak is not a crash and not a wrong answer — the image still loads, the callback still fires, a unit test that checks "did image get set" still passes. Nothing observable is broken in the moment. The cost is invisible until you profile: Instruments' Leaks/Allocations shows the count of live PosterLoaders only ever going up. This is exactly the class of bug that survives a casual read and a green test suite — and exactly why you review AI-proposed code with a reviewer's eye.

The fix: a capture list. Break the strong link from the closure back to self with [weak self], then handle the now-optional self:

init(url: URL) {
    self.url = url
    self.onReload = { [weak self] in
        guard let self else { return }
        self.image = nil
        self.cache[self.url] = nil
        Task { await self.load() }
    }
}

[weak self] makes the closure hold self weakly — it no longer contributes to the retain count, so the circle is broken and PosterLoader deallocates the moment the cell releases it. Inside the closure, self is now Optional; guard let self else { return } unwraps it (and bails harmlessly if the loader is already gone by the time the closure fires). Behavior is identical when the loader is alive; the only change is that it can now die.

Tip

The rule. When an object stores a closure that refers back to that same object — a completion handler, an event callback, a Combine sink, a Task retained on the instance — capture self weakly. If the closure is called exactly once and then released (many URLSession completions), the cycle is temporary and often tolerable; but a stored closure that outlives the call is a permanent leak. When in doubt on a stored closure: [weak self].

Note

Don't over-correct. [weak self] is not a reflex to sprinkle everywhere. A closure passed to withTaskGroup or a one-shot Task that self does not store creates no cycle — self isn't holding the closure. Adding [weak self] there just introduces needless Optional unwrapping and can drop work you wanted to finish. Weak-capture where there is an actual cycle: self stores the closure, and the closure references self.

Your turn

The exercise below is the same shape as the bug you just fixed, reduced to something the runner can grade: a class that stores a callback which, when invoked, must mutate the class's own state. Wire the stored closure up correctly (capturing the object weakly, as in the fix above) so that firing events increments the counter. The tests check the behavior; the capture list is the habit.

Wire a stored callback without a retain cycle

Edit the code on the right, then run the hidden tests.

Knowledge check

Q: Why does the original PosterLoader never deallocate? It stores onReload, and onReload captures self strongly. The object holds the closure and the closure holds the object — a reference cycle whose counts never reach zero, so ARC never frees either.

Q: Why isn't this caught by a passing test or normal use? A leak produces no wrong value and no crash — the image still loads and callbacks still fire. It only shows up as ever-growing memory under profiling. That silence is why reviewing AI-proposed stored closures for capture lists is a required habit, not an optional one.