Deep-dive: three reviews, three races — making the crash supervisor safe against its own retry flow

[debpalash]

Part of a series of engineering deep-dives from VoiceStudio's reliability work. This one is about the crash supervisor — and the three successive races review found in it.

The setup

VoiceStudio's desktop shell supervises the Python backend: if the process dies unexpectedly, the supervisor records a crash marker (exit code + stderr tail + uptime), then respawns it — with escalating backoff (0s → 5s → 15s) and a budget of 3 restarts per 10 minutes, so a genuine crash loop surfaces as an actionable error instead of a silent resource churn (#1548).

Death is detected only via confirmed process exit (try_wait) — never a slow health probe — so a busy-but-alive backend is never killed by its own babysitter.

The hard part isn't the happy path. It's that the supervisor isn't the only thing that kills and spawns backends: the user's Retry / Clean & Retry flow deliberately kills the child and spawns its own replacement. Two owners, one process slot, one port. Review found three races here, each fix revealing the next.

Race 1: the flag can be missed

The retry flow sets a kill_intended flag so the supervisor knows the death was deliberate. But the completed retry then clears the flag when it tracks its replacement child — and both the set and the clear can happen between two supervisor samples. The supervisor wakes, sees a dead child, sees no flag, and "restarts" a backend the retry flow already replaced — two backends fighting over one port.

Race 2: the generation counter

The flag is transient; what's needed is a durable tell. Fix: a monotonic spawn generation —

static BACKEND_SPAWN_GENERATION: AtomicU64 = AtomicU64::new(0);
// bumped in track_backend_child(), never un-bumped

The supervisor snapshots it, and any change means someone else tracked a replacement — observed even if that replacement has itself already exited by the time we look. The backoff pause polls it every 500ms and yields promptly, so the retry's own flow claims the supervisor slot and we never free_port() a live replacement out from under its owner.

Race 3: snapshot placement (the TOCTOU)

Third review pass: where you take the snapshot matters. Originally it was sampled after observing the exit:

let exit = backend_child_exit(app)?;          // gap ↓
let observed = BACKEND_SPAWN_GENERATION.load(SeqCst);  // replacement tracked in
                                                       // the gap is baked into
                                                       // the snapshot — missed

A replacement tracked in the gap between try_wait and the load gets baked into the snapshot, and the transfer is invisible. Sampled before observing the exit, any tracking from that point on — even one whose child we're about to see exit — reads as a generation change and yields:

let observed = BACKEND_SPAWN_GENERATION.load(SeqCst);  // FIRST
let exit = backend_child_exit(app)?;                   // then observe

Plus one last check immediately before touching the port — covering the zero-backoff first respawn (which never enters the pause loop) and the tail of the pause:

if BACKEND_SPAWN_GENERATION.load(SeqCst) != observed { return; } // yield
if port_in_use(port) && !free_port_or_report(port) { /* fail loudly */ }

Why backoff at all

The first respawn is immediate — a one-off crash should self-heal fast. Repeat deaths within the window get 5s then 15s of breathing room, because back-to-back torch-import storms are exactly what pushes an already memory-pressured machine over the edge again, burning the whole restart budget in seconds and helping nobody. During the pause the frontend already shows a "reconnecting" banner, so the wait reads as reconnecting, not a hang.

Proving it

All of this is now exercised end-to-end by the fault-injection harness (#1551): scripted fake backends that crash-loop, die by signal, or get deliberately killed, run against the real supervisor code on all 3 OSes in CI — asserting the restart events, the crash markers, the give-up message, and that a deliberate kill produces no restart.

Lesson we keep re-learning: in a two-owner process-lifecycle design, every flag is a race until it's a monotonic counter — and every counter snapshot is a race until you've proven where it's taken.

PRs: #1548 (supervisor), #1551 (harness).