Pingline checkpoint — the reentrancy bug

You asked the assistant to make the outbox assign a monotonically increasing sequence number to each queued message so the server can order them, and to guarantee a message is only sent once. Here is the diff it proposed. Read it as you would in a real review.

The AI-generated code

actor Outbox {
    private var pending: [Message] = []
    private var nextSequence = 0
    private let sender: MessageSender

    init(sender: MessageSender) { self.sender = sender }

    func enqueue(_ message: Message) {
        pending.append(message)
    }

    // Flush the next queued message with the next sequence number.
    func flushNext() async {
        guard let message = pending.first else { return }   // 1. check

        let sequence = nextSequence                          // 2. read counter
        let stamped = message.withSequence(sequence)         // stamp it

        await sender.send(stamped)                           // 3. AWAIT — network send

        nextSequence += 1                                    // 4. advance counter
        pending.removeFirst()                                // 5. remove the sent one
    }
}

It looks disciplined: one method, a guard, a clear five-step flow. And if you call flushNext once at a time it is perfect. The bug only appears when the reconnect logic does what reconnect logic does — fires several flushes at once.

Guiding questions

Work these before scrolling to the explanation.

1. Outbox is an actor, so only one task runs its code at a time. But what does the await on line 3 do to that guarantee for the rest of flushNext?
2. Suppose two tasks call flushNext() nearly together while pending has one message. Walk the interleaving: task A reaches the await and suspends. What is the state of pending and nextSequence when task B enters?
3. How many times does that single queued message get sent? What sequence number does each copy carry?
4. Which lines form the "critical section" that must not be interrupted, and which line splits it?

Explanation

Actors are reentrant: when an isolated method hits await and suspends, the actor is free to run other queued work before the method resumes. flushNext checks pending.first, reads nextSequence, and then awaits the network send — and that suspension is a hole. Nothing has been removed from pending yet, and the counter has not advanced.

Here is the interleaving under a burst of flushes:

• Task A: pending.first is M, reads nextSequence == 0, stamps M#0, await sender.send → suspends.
• Task B runs (the actor is free): pending.first is still M, reads nextSequence == 0 (A hasn't advanced it), stamps M#0, await sender.send → suspends.
• Both sends complete. Each task advances the counter and calls removeFirst().

M was sent twice, both times with sequence 0, and removeFirst() runs twice — dropping a different, innocent message that was queued behind it. This is the classic check-then-act race: the state you validated before the await was mutated by another task during the suspension. It compiles cleanly, survives single-threaded tests, and corrupts your outbox the first time reconnect fires two flushes.

The fix: mutate before you suspend

Do all the state changes — take the message, advance the counter, remove it from the queue — synchronously, before the await. Then the network send happens against a value you already own, and no other task can see the same message or the same sequence number.

func flushNext() async {
    // Critical section: claim the work atomically, no await inside.
    guard !pending.isEmpty else { return }
    let message = pending.removeFirst()
    let sequence = nextSequence
    nextSequence += 1
    let stamped = message.withSequence(sequence)

    // Only now suspend. If send fails, requeue deliberately.
    do {
        try await sender.send(stamped)
    } catch {
        pending.insert(stamped, at: 0)   // put it back for the next flush
        nextSequence -= 1
    }
}

Now the claim is atomic: between removeFirst() and the next task entering, the message is gone from pending and the counter has moved. A second concurrent flushNext sees the next message (or an empty queue) and a fresh sequence number. The only state that survives the await is the local stamped, which no other task can touch. The alternative fix — re-validate after the await — works too, but "keep the check-then-act synchronous" is the habit to build: if a method reads state, decides, and mutates, do not put an await in the middle.

Your turn

The exercise below is the distilled core of this bug, made server-gradable: an actor that must hand out unique ids under 1,000 concurrent callers. Get the critical section right — no await between reading the counter and advancing it — and every id comes back exactly once.

Knowledge check

Q: Why does making Outbox an actor not, by itself, prevent the double-send? Actor isolation guarantees one task runs its code at a time, but it is reentrant: at an await the method suspends and another task can run. The atomicity is only for the synchronous span between suspension points, so a check-then-await-then-mutate sequence can still race.

Q: What is the general rule the fix encodes? Keep a check-then-act critical section synchronous — claim and mutate the shared state before any await. If you must suspend mid-operation, re-validate the state after the suspension instead of assuming it is unchanged.