Home

The actor reentrancy bug I shipped

I converted a cache to an actor, the compiler went quiet, and I assumed the concurrency problems were over. Two weeks later we were making the same network request four times for the same URL.

Actors prevent data races. They do not prevent reentrancy, and reentrancy is the bug I did not know to look for.

The code

actor ImageCache {
    private var cache: [URL: Image] = [:]

    func image(for url: URL) async throws -> Image {
        if let cached = cache[url] {
            return cached
        }

        let image = try await downloader.download(url)   // suspension point
        cache[url] = image
        return image
    }
}

This looks airtight. The state is protected by the actor, the check happens before the download, the result is stored after. There is no data race and the compiler agrees.

What actually happens

Four views appear at once, all asking for the same image.

  1. Call A checks cache[url] — empty. Starts downloading, suspends at the await.
  2. While A is suspended, the actor is free. Call B enters, checks cache[url] — still empty, because A has not stored anything yet. Starts its own download. Suspends.
  3. Same for C and D.
  4. All four downloads complete and each writes to cache[url].

Four network requests, four decoded images, three of them thrown away. On a screen with a grid of thumbnails, this multiplied badly.

The critical fact: an actor only guarantees exclusive access between suspension points. At every await, the actor is released and other calls can enter. Your function resumes later in a world where everything it checked earlier may have changed.

Warning

The mental model that misleads people — mine included — is “an actor is a lock”. It is not. A lock held across an await would block everything; an actor deliberately does not, because blocking is what actors exist to avoid. The price of that non-blocking behaviour is that your critical section is only the code between two suspension points.

The fix: store the task, not the result

The insight is that the cache needs to record “a download is in flight”, not just “a download finished”. Caching the Task does both:

actor ImageCache {
    private enum Entry {
        case inFlight(Task<Image, Error>)
        case ready(Image)
    }

    private var cache: [URL: Entry] = [:]

    func image(for url: URL) async throws -> Image {
        if let entry = cache[url] {
            switch entry {
            case .ready(let image):
                return image
            case .inFlight(let task):
                return try await task.value       // join the existing download
            }
        }

        let task = Task {
            try await downloader.download(url)
        }
        cache[url] = .inFlight(task)              // stored BEFORE any await

        do {
            let image = try await task.value
            cache[url] = .ready(image)
            return image
        } catch {
            cache[url] = nil                      // let the next caller retry
            throw error
        }
    }
}

The line that matters is cache[url] = .inFlight(task), placed before the first await. Between entering the function and that assignment there is no suspension point, so no other call can interleave. Callers B, C and D find the in-flight task and await the same one.

One request. Three callers sharing it.

The rule I use now

Re-check your assumptions after every await. Anything you read before a suspension point may be stale when you resume.

Practically, that means:

func update(_ id: Item.ID) async {
    guard let item = items[id] else { return }
    let result = await process(item)

    // WRONG: `item` may have been removed or replaced while we were suspended
    items[id]!.result = result

    // RIGHT: re-check
    guard items[id] != nil else { return }
    items[id]?.result = result
}

And more generally: mutate state before you suspend, not after, wherever the order allows it. The in-flight entry above is that principle applied.

What to look for in existing code

Three shapes worth auditing in any actor:

  1. Check-then-act across an await. if cache[x] == nil { … await … ; cache[x] = y }. The classic, and the one I shipped.
  2. A counter or flag set before an await and read after. isLoading = true; await load(); isLoading = false — two overlapping calls and the flag is wrong.
  3. Index or reference captured before an await. An array index taken before suspending may point at a different element, or past the end, when you resume.

What actors do still give you

I do not want this to read as “actors are useless”. They eliminate the entire category of low-level data race — torn reads, corrupted dictionaries, the crashes that used to happen when two queues touched the same array. That is real and it is enforced by the compiler.

What they do not do is make your logic atomic. A sequence of operations that must happen together has to be written as one uninterrupted stretch between suspension points, and the compiler will not tell you when it is not.

That distinction is the whole lesson, and it cost me two weeks and four times the bandwidth bill to learn.