swift-concurrency-review

Swift Concurrency Review

Overview

Swift 6 strict concurrency catches data races at compile time but lets a class of semantic bugs through: state-overwrite races, TOCTOU around Task { }, and cancellation that doesn't actually prevent firing. This is a reviewer's checklist for those bug classes. Apply on any diff touching actor, @MainActor, await, Task {, lock-protected state, or deadline timers.

The Five Questions

1. After every await, is state re-checked — not just the index?

When an actor method suspends at await, any other method on the actor can run to completion before it resumes. Re-finding a record by id ≠ verifying its state is still what you expected.

// BAD
try await transport.send(message)
guard let idx = outbound.firstIndex(where: { $0.id == id }) else { return }
outbound[idx].direction = .delivered   // overwrites .cancelled / .deadlineMissed

// GOOD
try await transport.send(message)
guard let idx = outbound.firstIndex(where: { $0.id == id }),
      outbound[idx].direction == .sending else { return }
outbound[idx].direction = .delivered

Invariant: terminal states are sticky. Once a record is .cancelled / .expired / .deadlineMissed, no later success path may overwrite it. The guard encodes this invariant.

2. Is Task { } body reading mutable state, or is it snapshotted at spawn?

A Task { } body runs later, on some executor. Reading clock.now, self.count, or any mutable state inside the closure is a TOCTOU — the value is read at an arbitrary later moment, not at spawn.

// BAD — clock.now read inside the Task
task = Task { [weak self] in
    let seconds = max(0, deadline.timeIntervalSince(clock.now))
    try? await clock.sleep(for: seconds)
}

// GOOD — clock.now read before Task exists, captured as constant
let seconds = max(0, deadline.timeIntervalSince(clock.now))
task = Task { [weak self] in
    try? await clock.sleep(for: seconds)
}