Skip to content

Commit 13e6334

Browse files
committed
fix(gc): live-worker teardown guard — no deinit under a running worker (ADR-0176)
Smell-audited: 3: ADR-0176 landed in the preceding commit (DA fork embedded); its error-unwind finding drove the deinit/env chokepoint guards on top of the exit barriers; joinAllNonDaemon renamed exitBarrier (the old name lied about its three duties); fpd ncpu gate removed with the canaries as the standing guard root_set.live_workers counts spawner-increment → worker-final-decrement across the whole thread lifetime (pre-register and post-unregister spans included). thread.exitBarrier half 3 waits a bounded 50ms for epilogue quiescence (the common case proceeds to a real graceful teardown), hard- exiting daemon-style only for a genuinely running thunk (AD-056: cljw exits immediately where the JVM waits). Runtime.deinit/Env.deinit guard the error-unwind paths (broken pipe mid-print etc.) by skipping all freeing when workers are live. CLJW_TORTURE_TEARDOWN_DELAY_MS widens the window for the deterministic e2e canaries. Verified: pre-fix 6/32 glibc aborts → post-fix 0/96 on the same 8x-parallel Linux harness; gc_torture drain 0/150 regression-clean; Mac smoke green.
1 parent 0e67a04 commit 13e6334

13 files changed

Lines changed: 191 additions & 18 deletions

File tree

‎src/app/builder.zig‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,9 @@ fn buildArtifact(io: std.Io, gpa: std.mem.Allocator, arena: std.mem.Allocator, o
395395
var ow = out.writer(io, &wbuf);
396396
try ow.interface.writeAll(artifact);
397397
try ow.interface.flush();
398+
// Exit barrier (ADR-0176): build-time eval may have spawned workers;
399+
// hard-exit if any is live rather than tear down under it (D-548(a)).
400+
@import("../runtime/thread.zig").exitBarrier(&rt);
398401
}
399402

400403
/// `cljw build <in.clj> -o <out>` (script mode): compile the source to a
@@ -485,6 +488,10 @@ pub fn tryRunEmbedded(io: std.Io, gpa: std.mem.Allocator, arena: std.mem.Allocat
485488
}
486489
}
487490

491+
// Exit barrier (ADR-0176): the embedded program is a full user program —
492+
// join its non-daemon Threads and hard-exit if any worker is still live
493+
// (the defers would free the heap under it, the D-548(a) teardown race).
494+
@import("../runtime/thread.zig").exitBarrier(&rt);
488495
try stdout.flush();
489496
return true;
490497
}

‎src/app/cli.zig‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ pub fn dispatch(init: std.process.Init) !void {
7373
// safepoint mode above; both armed together by `gc_torture.arm()`.
7474
if (init.environ_map.get("CLJW_GC_TORTURE_ALLOC")) |raw|
7575
gc_torture.configureAlloc(std.fmt.parseInt(u32, raw, 10) catch 1);
76+
// ADR-0176: teardown fault injection (widens the exit-teardown window so
77+
// a live-worker-guard regression aborts deterministically; test-only).
78+
if (init.environ_map.get("CLJW_TORTURE_TEARDOWN_DELAY_MS")) |raw|
79+
gc_torture.configureTeardownDelay(std.fmt.parseInt(u32, raw, 10) catch 50);
7680

7781
// Publish the process env for the runtime surface (`System/getenv`).
7882
// `init.environ_map` is already a `*Environ.Map` owned by `init`

‎src/app/nrepl.zig‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ pub fn run(
101101
const stream = server.accept(io) catch |err| {
102102
try stderr.print("nrepl: accept failed: {s}; exiting\n", .{@errorName(err)});
103103
try stderr.flush();
104+
// Exit barrier (ADR-0176): a session may have left live workers
105+
// (fire-and-forget futures/agents); hard-exit rather than let the
106+
// defers free the heap under them (the D-548(a) teardown race).
107+
// The port file is cleaned BEFORE the barrier — a hard exit skips
108+
// the defer, and a stale `.nrepl-port` misleads CIDER discovery
109+
// (the delete is idempotent, so the defer double-clean is fine).
110+
cleanupNreplPortFile(io);
111+
@import("../runtime/thread.zig").exitBarrier(&rt);
104112
return;
105113
};
106114
defer stream.close(io);

‎src/app/repl.zig‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ pub fn run(
155155
} else {
156156
try runPiped(io, &rt, &env, &macro_table, arena, stdout, stderr, &stars);
157157
}
158+
// Exit barrier (ADR-0176): join non-daemon Threads + hard-exit if any
159+
// worker (future/agent/Thread epilogue) is still live — the defers below
160+
// would free the heap under it (the D-548(a) teardown race).
161+
@import("../runtime/thread.zig").exitBarrier(&rt);
158162
}
159163

160164
/// Interactive loop driven by the raw-mode line editor.

‎src/app/runner.zig‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ pub fn runSource(
174174
// ADR-0174 D6: main waits for live non-daemon `(Thread. f)` threads,
175175
// JVM-exact — a fire-and-forget `(.start (Thread. f))` completes f.
176176
// Daemon threads die with the process (skipped: never registered).
177-
@import("../runtime/thread.zig").joinAllNonDaemon(&rt);
177+
@import("../runtime/thread.zig").exitBarrier(&rt);
178178
try stdout.flush();
179179
}
180180

@@ -227,6 +227,9 @@ pub fn runSourceCompare(
227227
try print.printValue(stdout, value);
228228
try stdout.writeByte('\n');
229229
try stdout.flush();
230+
// Exit barrier (ADR-0176): compared source may have spawned workers;
231+
// hard-exit if any is live rather than tear down under it (D-548(a)).
232+
@import("../runtime/thread.zig").exitBarrier(&rt);
230233
return;
231234
}
232235

‎src/runtime/agent.zig‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,10 @@ fn enqueueDirect(rt: *Runtime, agent_val: Value, action: Action) !void {
349349
if (need_spawn) {
350350
// Keep the agent alive for the drainer even if the caller drops it.
351351
try rt.gc.pin(agent_val);
352+
// Teardown guard (ADR-0176): account the drainer BEFORE spawn.
353+
root_set.noteWorkerSpawned();
352354
var t = std.Thread.spawn(.{}, drainer, .{a}) catch |e| {
355+
root_set.noteWorkerExited();
353356
io_default.lockMutex(&a.cell.mutex);
354357
a.cell.draining = false;
355358
io_default.unlockMutex(&a.cell.mutex);
@@ -364,6 +367,9 @@ fn enqueueDirect(rt: *Runtime, agent_val: Value, action: Action) !void {
364367
/// `ThreadGcContext` so a concurrent collect parks it at a safepoint and walks
365368
/// its operand-stack roots (the in-flight action), like `future.worker`.
366369
fn drainer(a: *Agent) void {
370+
// FIRST defer → runs LAST: the teardown guard may only see 0 once every
371+
// heap-touching cleanup below (state stores, unpin, unregister) completed.
372+
defer root_set.noteWorkerExited();
367373
const agent_val = Value.encodeHeapPtr(.agent, a);
368374
// The tx_slot publishes this drainer's STM transaction (an action may run
369375
// a `dosync`) so it is GC-rooted during a collect (#4a' in-txn-map rooting).
@@ -575,7 +581,10 @@ pub fn restart(rt: *Runtime, agent_val: Value, new_state: Value, clear_actions:
575581

576582
if (need_spawn) {
577583
try rt.gc.pin(agent_val);
584+
// Teardown guard (ADR-0176): account the drainer BEFORE spawn.
585+
root_set.noteWorkerSpawned();
578586
var t = std.Thread.spawn(.{}, drainer, .{a}) catch |e| {
587+
root_set.noteWorkerExited();
579588
io_default.lockMutex(&a.cell.mutex);
580589
a.cell.draining = false;
581590
io_default.unlockMutex(&a.cell.mutex);

‎src/runtime/env.zig‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,12 @@ pub const Env = struct {
424424
}
425425

426426
pub fn deinit(self: *Env) void {
427+
// Teardown guard (ADR-0176 / D-548(a)) — see Runtime.deinit's twin
428+
// guard: on an error-unwind with a live worker (whose eval reads this
429+
// Env's namespaces/Vars), freeing them here is the same corruption
430+
// class. This defer runs BEFORE Runtime.deinit's (LIFO), so it does
431+
// the bounded epilogue wait; a timeout = a running thunk → skip.
432+
if (!@import("gc/root_set.zig").awaitQuiescentWorkers(50 * 1_000_000)) return;
427433
// Clear any process-global slot that points INTO this Env's
428434
// Vars before they are freed, so the slot can never dangle.
429435
// Currently the sole consumer is `runtime/error/context.zig`'s

‎src/runtime/future.zig‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ pub fn alloc(rt: *Runtime, env: *Env, thunk: Value, loc: SourceLocation) !Value
122122
const fut_val = Value.encodeHeapPtr(.future, f);
123123
// Pin so the worker's write target survives even when no deref'er holds it.
124124
try rt.gc.pin(fut_val);
125+
// Teardown guard (ADR-0176): account the worker BEFORE spawn so the exit
126+
// boundary sees it even while the thread is still starting up.
127+
root_set.noteWorkerSpawned();
125128
var t = std.Thread.spawn(.{}, worker, .{f}) catch |e| {
129+
root_set.noteWorkerExited();
126130
_ = rt.gc.unpin(fut_val);
127131
return e;
128132
};
@@ -134,6 +138,9 @@ pub fn alloc(rt: *Runtime, env: *Env, thunk: Value, loc: SourceLocation) !Value
134138
/// + wake deref'ers, unpin. Runs on a fresh thread with its own threadlocal GC
135139
/// slots (no conveyed dynamic bindings yet — binding conveyance is a follow-up).
136140
fn worker(f: *Future) void {
141+
// FIRST defer → runs LAST: the teardown guard may only see 0 once every
142+
// heap-touching cleanup below (result store, unpin, unregister) completed.
143+
defer root_set.noteWorkerExited();
137144
const fut_val = Value.encodeHeapPtr(.future, f);
138145
// The tx_slot publishes this worker's STM transaction so a `dosync` in the
139146
// thunk is GC-rooted during a collect (#4a' in-txn-map rooting).

‎src/runtime/gc/gc_torture.zig‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ var pending_alloc_period: u32 = 0;
5555
pub var alloc_period: u32 = 0;
5656
threadlocal var alloc_counter: u32 = 0;
5757

58+
/// Teardown fault injection (ADR-0176 / D-548(a) red test):
59+
/// `CLJW_TORTURE_TEARDOWN_DELAY_MS=N` makes `Runtime.deinit` sleep N ms AFTER
60+
/// its live-worker guard and BEFORE freeing — so a guard regression turns the
61+
/// teardown-vs-worker race back into a (near-)deterministic glibc abort under
62+
/// a fire-and-forget allocating thunk. Inert (0) unless the env var is set;
63+
/// read once at CLI startup, no arming stage (it fires at most once, at exit).
64+
pub var teardown_delay_ns: u64 = 0;
65+
66+
/// Record the requested teardown delay (`CLJW_TORTURE_TEARDOWN_DELAY_MS`).
67+
pub fn configureTeardownDelay(ms: u32) void {
68+
teardown_delay_ns = @as(u64, ms) * 1_000_000;
69+
}
70+
5871
/// Record the requested torture period from the parsed `CLJW_GC_TORTURE`
5972
/// value. Does NOT activate — `arm()` does, post-bootstrap. `n == 0` is a
6073
/// no-op (mode stays off).

‎src/runtime/gc/root_set.zig‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,59 @@ var registered_count: std.atomic.Value(u32) = .init(0);
351351
/// dormant D-244 #4 path, validated separately under user awareness.
352352
pub threadlocal var is_registered_worker: bool = false;
353353

354+
/// Live worker-thread accounting for the PROCESS-EXIT teardown guard
355+
/// (D-548(a) / ADR-0176). The spawner increments BEFORE `Thread.spawn`
356+
/// (decrementing on spawn failure); the worker decrements as its very LAST
357+
/// action (its first-declared defer), AFTER every heap-touching cleanup
358+
/// (result store, `gc.unpin`, `unregisterThread`). Distinct from
359+
/// `registered_count`: this covers the whole thread lifetime including the
360+
/// pre-register and post-unregister spans. While nonzero, graceful Runtime
361+
/// teardown is FORBIDDEN — the app exit boundary hard-exits instead
362+
/// (`thread.exitBarrier`): a detached worker's thunk/epilogue reads the
363+
/// GC heap and mutates gpa-backed structures, and `rt.deinit` freeing them
364+
/// underneath produced the D-548(a) glibc heap-corruption aborts
365+
/// (`malloc_consolidate(): unaligned fastbin chunk detected`). The worker's
366+
/// seq_cst decrement pairs with the boundary's seq_cst read: count==0 proves
367+
/// every epilogue completed, so teardown is safe.
368+
var live_workers: std.atomic.Value(u32) = .init(0);
369+
370+
/// Spawner-side: account a worker about to be spawned (call BEFORE
371+
/// `Thread.spawn`; call `noteWorkerExited` to roll back on spawn failure).
372+
pub fn noteWorkerSpawned() void {
373+
_ = live_workers.fetchAdd(1, .seq_cst);
374+
}
375+
376+
/// Worker-side final act (or spawner-side rollback): the thread's heap access
377+
/// is complete. Must be the worker fn's FIRST-declared defer.
378+
pub fn noteWorkerExited() void {
379+
_ = live_workers.fetchSub(1, .seq_cst);
380+
}
381+
382+
/// Snapshot for the exit-boundary teardown guard.
383+
pub fn liveWorkerCount() u32 {
384+
return live_workers.load(.seq_cst);
385+
}
386+
387+
/// Bounded wait for worker quiescence (ADR-0176). Returns true iff
388+
/// `live_workers` reached 0 within `max_wait_ns` — then graceful teardown is
389+
/// safe. Worker EPILOGUES (result store → unpin → unregister → final
390+
/// decrement) never block, so a completed thunk's worker quiesces in µs; a
391+
/// timeout therefore means a thunk is genuinely still RUNNING, and the caller
392+
/// must abandon teardown (hard exit / skip-freeing) instead. Polling sleep, no
393+
/// condvar: this runs at most twice per process (exit barrier + deinit guard),
394+
/// never on a hot path.
395+
pub fn awaitQuiescentWorkers(max_wait_ns: u64) bool {
396+
if (liveWorkerCount() == 0) return true;
397+
const io_def = @import("../concurrency/io_default.zig");
398+
var waited: u64 = 0;
399+
while (waited < max_wait_ns) {
400+
io_def.sleep(1_000_000); // 1ms
401+
waited += 1_000_000;
402+
if (liveWorkerCount() == 0) return true;
403+
}
404+
return false;
405+
}
406+
354407
/// Build the CALLING thread's standard worker context: all four TLS root
355408
/// slots plus the caller-supplied opaque `tx_slot` (opaque because root_set
356409
/// must not import lock_tx — see `ThreadGcContext.tx_slot`). Must run ON the

0 commit comments

Comments
 (0)