|
| 1 | +# ADR-0175 — Worker registration is a GC safepoint (spawn-to-register window) |
| 2 | + |
| 3 | +- **Status**: **ACCEPTED 2026-07-29.** |
| 4 | +- **Deciders**: autonomous loop + one Devil's-advocate fork (general-purpose, |
| 5 | + fresh context, F-006/F-002/F-011/F-009 envelope; output reflected verbatim |
| 6 | + in Alternatives considered). |
| 7 | +- **Supersedes / relates**: ADR-0090 (the STW safepoint rendezvous this |
| 8 | + extends), ADR-0150 + am1 (fabrication region / park semantics — sibling |
| 9 | + park-site rule), D-244 #4, D-548 (the load-flake ledger this explains a leg |
| 10 | + of), D-418/D-559 (the two previously-fixed agent-path GC races). |
| 11 | + |
| 12 | +## Context |
| 13 | + |
| 14 | +The scheduled full-gate CI on x86_64-linux (run 30333558512, 2026-07-28) failed |
| 15 | +`e2e_phase16_gc_torture` with SIGABRT (exit 134) in the alloc-torture agent |
| 16 | +block (`agent_alloc 'drain'`), on a commit whose previous six nightly runs were |
| 17 | +green — an intermittent race, not a regression. Reproduced on a 4-vCPU-pinned |
| 18 | +x86_64 host at ~1.5%/run; a `-Dprofile` build captured the crash four times |
| 19 | +with one identical signature: |
| 20 | + |
| 21 | +> General protection exception in `root_set.nextThreadRoots` |
| 22 | +> (the thread-roots walk), on the MAIN thread, inside the STW collect injected |
| 23 | +> by `agent.tortureCollectInWindow` during `send`. |
| 24 | +
|
| 25 | +Root cause: **`safepoint.stopWorld` computes its park-rendezvous target from |
| 26 | +`registered_count`, but a worker thread that has been SPAWNED and not yet |
| 27 | +REGISTERED is invisible to it.** The agent drain case churns drainers (each |
| 28 | +queue-empty exit + next send spawns a fresh one) while the alloc-torture |
| 29 | +injects a collect at every enqueue, so this interleaving fires readily on a |
| 30 | +slow/loaded runner: |
| 31 | + |
| 32 | +1. `send` #k: queue empty → spawn drainer D. `std.Thread.spawn` returns; main |
| 33 | + continues while D is still starting up (OS scheduling delay — the reason |
| 34 | + this is a 4-vCPU-Linux flake and a 10-core-Mac never). |
| 35 | +2. `send` #k+1: the injected collect runs `stopWorld` → `registered_count` is |
| 36 | + 0 (D not yet registered) → target 0 → returns immediately → the collector |
| 37 | + begins the root walk + sweep. |
| 38 | +3. D comes to life, `registerThread` succeeds (it never looked at |
| 39 | + `gc_requested`), D pops an action and runs it — pushing binding/eval frames |
| 40 | + that the collector's `nextThreadRoots` is concurrently reading through the |
| 41 | + published TLS slot pointers (torn frame chain → GP fault, the observed |
| 42 | + crash), while the popped action itself has left the queue's traceGc range |
| 43 | + (swept-in-use → the drainer-side UAF variant, also observed as a heap-address |
| 44 | + SIGSEGV). |
| 45 | + |
| 46 | +The walk's stated safety justification — "the safepoint guarantees no |
| 47 | +concurrent register/unregister during collect" — was false for exactly this |
| 48 | +window. Every prior fix in this family (D-418 enqueue-window rooting, D-559 |
| 49 | +park-honors-fabrication) assumed the rendezvous itself was sound; this is the |
| 50 | +rendezvous's own hole. |
| 51 | + |
| 52 | +## Decision |
| 53 | + |
| 54 | +**Registration is itself a GC safepoint, and the registration handshake is |
| 55 | +seq_cst.** Concretely (all landed in this arc): |
| 56 | + |
| 57 | +1. **`registerThread` parks the newborn** (root_set.zig): after publishing its |
| 58 | + slot + count (under `registry_mutex`) and releasing the mutex, the worker |
| 59 | + loads `gc_requested` (seq_cst) and calls `safepoint.park()` if set — so a |
| 60 | + worker born while a collect is arming/running quiesces before its first |
| 61 | + mutator step. A freshly registered ctx publishes only empty TLS roots, so a |
| 62 | + collector already mid-walk reading its slot sees an empty contribution. |
| 63 | +2. **Seq_cst Dekker pairing**: `stopWorld`'s `gc_requested.store(true)` + its |
| 64 | + `registeredCountLockFree()` read, and `registerThread`'s |
| 65 | + `registered_count.fetchAdd` + its `gc_requested` load, are all seq_cst. |
| 66 | + The count add sits under `registry_mutex` and the collector's count read |
| 67 | + under `sp_mutex` — no common lock, so without seq_cst the both-sides-miss |
| 68 | + interleaving (store-buffer/Dekker) is real. Under seq_cst either the |
| 69 | + collector waits for the newborn's park, or the newborn parks at |
| 70 | + registration — never both-miss. **Do not relax these four orderings** — |
| 71 | + both sides carry comments naming the pair. |
| 72 | +3. **Atomic registry-slot access**: `threadContextAt` / `markRegisteredTxs` |
| 73 | + read slots with `@atomicLoad(.acquire)`, register/unregister write with |
| 74 | + `@atomicStore(.release)`. A newborn may legally publish its slot mid-walk |
| 75 | + (it then parks); mid-collect slots only transition null→ctx (unregistration |
| 76 | + is impossible mid-collect — every already-registered worker is parked or |
| 77 | + blocked-counted for the rendezvous to have completed). |
| 78 | +4. **The `TooManyThreads` fallthrough is closed** (the DA fork's finding — the |
| 79 | + same corruption class, deterministically reachable at the 64-worker cap, |
| 80 | + e.g. `(dotimes [i 100] (future …))`): `future.worker` and `thread.worker` |
| 81 | + previously ran their thunk UNREGISTERED when registration failed. Now: a |
| 82 | + future realises as `realised_error` with nil cached (no allocation — an |
| 83 | + unregistered thread may not touch the GC heap; `errorValue`'s new nil-guard |
| 84 | + routes `deref` to the generic `future_thunk_failed` raise), and a Thread |
| 85 | + reports one non-allocating stderr line + falls through to its done-epilogue. |
| 86 | + The agent drainer already bailed correctly (releases the drainer slot; the |
| 87 | + queued actions wait for the next send-spawned drainer). |
| 88 | +5. **Shared worker-context builder**: `root_set.workerContext(tx_slot)` builds |
| 89 | + the standard 5-slot ctx from the calling thread's own TLS; the three worker |
| 90 | + families (agent drainer / future / Thread) use it, so a future worker family |
| 91 | + cannot silently drop a slot. |
| 92 | + |
| 93 | +### Invariants this ADR makes explicit (previously unwritten) |
| 94 | + |
| 95 | +- **Pre-registration code touches only spawner-pinned objects + pure bit-ops.** |
| 96 | + Every spawn site `gc.pin`s the object the worker will address before |
| 97 | + `Thread.spawn` (agent/future/thread all do); under F-006 non-moving GC that |
| 98 | + makes the newborn's pre-register reads safe. A new spawn site must keep this. |
| 99 | +- **No lock may be held at a `registerThread` call site** (the registration |
| 100 | + park would strand it). True at all sites today; documented as a contract on |
| 101 | + `registerThread`. |
| 102 | +- **On `TooManyThreads` the worker must NOT run its mutator body**, and its |
| 103 | + failure report must not allocate on the GC heap. |
| 104 | + |
| 105 | +## Verification |
| 106 | + |
| 107 | +- New deterministic unit test (safepoint.zig, "registerThread during an active |
| 108 | + STW collect parks the newborn until resumeWorld"): red pre-fix (the newborn |
| 109 | + observably runs between stopWorld-return and resumeWorld), green post-fix. |
| 110 | +- The seq_cst half is not deterministically testable; the statistical guard is |
| 111 | + the e2e repro: pre-fix `agent_alloc 'drain'` crashed 3/221 and 4/600 (two |
| 112 | + campaigns) on a 4-vCPU-pinned x86_64 host; post-fix the same harness must run |
| 113 | + clean (recorded in D-548 on landing). |
| 114 | +- The existing `phase16_gc_torture` agent block remains the standing e2e guard |
| 115 | + (it is what caught this on CI). |
| 116 | + |
| 117 | +## Alternatives considered (DA fork output, condensed faithfully) |
| 118 | + |
| 119 | +The fork's leading finding: **no alternative avoids the core mechanism** — every |
| 120 | +shape ends up needing the newborn to check `gc_requested` at its first |
| 121 | +GC-visible act; the alternatives only vary the fence spelling and walk |
| 122 | +protection. No F-NNN conflict exists in any shape. |
| 123 | + |
| 124 | +1. **Smallest-diff — collector-held `registry_mutex` + rendezvous |
| 125 | + re-validation**: collector re-reads the count under `registry_mutex` after |
| 126 | + the rendezvous, re-waits if it grew, and holds the mutex across walk+sweep; |
| 127 | + registration simply blocks. Better: one synchronization vocabulary (no |
| 128 | + seq_cst reasoning), the old quiescence comments become true again. Breaks: |
| 129 | + the re-validation loop bounces between `sp_mutex` and `registry_mutex` |
| 130 | + (choreography arguably worse), creates a latent hold-order hazard against |
| 131 | + `noteWorkerLeft`, and does nothing for the `TooManyThreads` hole. The naive |
| 132 | + no-re-validation variant deadlocks (a tiny-action worker blocks in |
| 133 | + `unregisterThread` on the held lock) — rejected. |
| 134 | +2. **Finished-form-clean (ADOPTED)**: the mechanism above + fallthrough |
| 135 | + closure + shared ctx builder + documented invariants. The fork also |
| 136 | + recommended a safe-build assert in `gc_heap.alloc` ("allocating thread is |
| 137 | + main or registered") — deferred to **D-566**: the survey found in-tree unit |
| 138 | + tests (e.g. lazy_seq's force-race workers) that alloc from unregistered |
| 139 | + test threads, so the assert needs a test-audit sweep first; nrepl/http are |
| 140 | + serial on the calling thread, so production is assert-clean today. |
| 141 | +3. **Wildcard — spawner-side accounting**: (a) a pending-registration counter |
| 142 | + `stopWorld` waits on self-deadlocks when the collector IS the spawner |
| 143 | + inside the window (exactly the reproduced topology: the injected collect |
| 144 | + fires between counter-increment and `Thread.spawn`); (b) register-before- |
| 145 | + spawn is structurally impossible (the ctx holds pointers to the newborn's |
| 146 | + threadlocals) and its placeholder-adopt repair re-derives the registration |
| 147 | + park plus extra machinery. Both rejected; the salvageable boilerplate- |
| 148 | + collapse instinct became the shared ctx builder. |
| 149 | + |
| 150 | +## Consequences |
| 151 | + |
| 152 | +- STW pause acquires no new waits: the collector never waits for unborn |
| 153 | + threads (they cannot touch the heap); it only refuses to let them START |
| 154 | + mid-collect. |
| 155 | +- `registeredCountRelaxed` renamed `registeredCountLockFree` (it is seq_cst |
| 156 | + now; the old name lied). |
| 157 | +- D-548 exposure (a) — the future/promise SIGABRT residual — was a plausible |
| 158 | + beneficiary, but the post-fix protocol run (4 rounds of 8× parallel |
| 159 | + `phase14_future_promise_delay` on 8-core linux) still reproduced 6/32 at |
| 160 | + the pre-fix baseline rate: **(a) is a distinct race and stays open**, |
| 161 | + signature unchanged (recorded in the row). |
| 162 | +- Deferred hardening: **D-566** (alloc-side unregistered-mutator assert after |
| 163 | + a test-audit sweep). The dormant worker-initiated-collect gap (main thread |
| 164 | + unregistered — root_set.zig `is_registered_worker` doc) is pre-existing and |
| 165 | + out of scope here. |
| 166 | + |
| 167 | +## Affected files |
| 168 | + |
| 169 | +- `src/runtime/gc/root_set.zig` — registration safepoint + contract docs + |
| 170 | + `workerContext` + atomic slots + walk-safety comment rewrite + rename. |
| 171 | +- `src/runtime/concurrency/safepoint.zig` — seq_cst arm + doc + the new |
| 172 | + deterministic unit test. |
| 173 | +- `src/runtime/future.zig` — fallthrough closure + `errorValue` nil-guard + |
| 174 | + ctx builder adoption. |
| 175 | +- `src/runtime/thread.zig` — fallthrough closure + ctx builder adoption. |
| 176 | +- `src/runtime/agent.zig` — ctx builder adoption. |
| 177 | +- `.dev/gc_rooting.md` — E3/E4 rows note the registration safepoint. |
| 178 | +- `.dev/debt.yaml` — D-548 recurrence note; new D-566. |
0 commit comments