ReviewOS

zig-utils/zig-js

Threadfuzz: make microtask lifecycle accounting race-free

#875
Closed chris opened this 11 days ago ·
11 days ago

Parent: #482 Related: #474, #493, #874

Root cause

The lifecycle fuzzer accumulated Promise-reaction completions for Thread.asyncJoin(), Atomics.waitAsync(...), and Lock.asyncHold(fn) with shared ordinary numeric += operations. Those reactions may run concurrently on different no-GIL settling or task-pump threads after their jobs settle. The counters therefore contained JavaScript program races, not engine synchronization witnesses.

The initially observed failures were both in the async-hold total and each lost exactly one unique marker:

  • seed 17: 923080 / 984084, delta 61004 (thread 2, iteration 4)
  • seed 26: 1804312 / 1845315, delta 41003 (thread 1, iteration 3)

Six immediate seed-17 reruns and a later instrumented 20-seed window did not reproduce the schedule collision. Source ordering nevertheless proves the assertion was invalid: runHoldJob releases the callback-form async hold before resolving the outer Promise, and its reaction performed the racy += afterward. A wider audit found the same invalid pattern in the async-join and wait-async reaction totals.

The no-function asyncHold() release total is different: each fulfillment reaction retains the live hold while updating its score and calls release() only afterward, so that ordinary total remains lock-protected.

Required correction

  • Accumulate async-join, wait-async, and callback-form async-hold sum/count pairs in a shared integer TypedArray using Atomics.add.
  • Keep validating each async-hold callback result against its unique expected marker.
  • Check every exact sum and completion count after run-loop quiescence so missing, duplicate, rejected, or wrong-value delivery remains a hard failure.
  • Preserve the ordinary release total and document why its live no-function hold provides synchronization.
  • Do not serialize engine task execution, move assertions into unrelated locks, weaken expected totals, retry failed seeds, add sleeps, or treat non-reproduction as proof.
  • Preserve the original failure evidence and link the test correction to #874, whose lifecycle gate exposed it.

Acceptance

  • Post-settlement shared totals contain no ordinary read-modify-write outside synchronization.
  • Exact atomic sum/count pairs detect missing, duplicate, and rejected async-join, wait-async, and async-hold completions; async-hold also validates every returned marker.
  • Seeds 17–36 pass as one 1,120-program lifecycle window.
  • Seeds 17 and 26 pass under suppression-free TSan.
  • The full 22,400-program lifecycle profile passes with zero functional failures.
  • The change is test-only and makes no runtime throughput or semantic claim.
11 days ago

Diagnosis update: both observed deficits equal one unique callback marker (61004 at seed 17 and 41003 at seed 26). Source ordering confirms runHoldJob implicitly releases the Lock before resolving the outer Promise, so the old .then-side ordinary += was outside synchronization and could race across task-pump threads.

The corrected witness keeps the per-callback v === cbMarker check and uses a dedicated shared Int32Array with atomic sum and count. Seeds 17–36 pass as one 1,120-program normal lifecycle run; seeds 17 and 26 each pass as 56-program suppression-free TSan runs. The full 22,400-program lifecycle gate is now running as one uninterrupted job; no terminal claim will be made until it completes.

11 days ago

Broader audit complete: Thread.asyncJoin() and Atomics.waitAsync(...) reaction totals had the same invalid ordinary shared += pattern as callback-form Lock.asyncHold(fn). The permanent witness now uses one bounded shared score vector with exact atomic sum/count pairs for all three post-settlement paths. The no-function asyncHold() release score remains ordinary because that fulfillment reaction owns the live hold until it calls release().

Focused evidence for this complete correction is green: seeds 17–36 passed as 1,120 normal lifecycle programs; seeds 17 and 26 each passed 56 suppression-free TSan programs. A fresh full 22,400-program lifecycle profile is running as one uninterrupted serialized job.

11 days ago

Final witness hardening: each asyncJoin() fulfillment is now checked against its own closed-over expected marker before its atomic sum/count contribution. This prevents even compensating wrong values from satisfying aggregate totals. The tightened version passed the complete seeds 17–36 normal window (1,120/1,120) and both suppression-free TSan witnesses (seed 17: 56/56; seed 26: 56/56). The definitive 22,400-program lifecycle profile has been restarted from this exact source as one isolated job.

11 days ago

The definitive profile did not pass. It failed closed at seed 3: one isolated lifecycle subcase exceeded the 120,000 ms watchdog, had no active Context available for cooperative interruption, then failed to tear down within the additional 30,000 ms and forced exit 125. The saved run is status 1; no partial success is being counted.

Concurrent user-owned Home/pantry unit/build processes were present on the host, so starvation is a plausible but unproven contributor. I am not dismissing or retrying around the result: the next step is a serialized focused seed-3 reproduction after those unit processes clear, beginning with the changed microtask-churn case and then identifying the exact lifecycle subcase if needed.

11 days ago

Current-state follow-up after the opaque seed-3 timeout:

  • rebased cleanly onto current main at 9ce52ff8, composing #876’s .fromOwnedSlice migration with the pending rejection-queue GC fixture;
  • every lifecycle dispatch now gives the watchdog a stable compile-time subcase name while preserving the exact 56-function order and per-case timing boundary;
  • lifecycle 1 3 now passes all 56/56 subcases in 15.376 s.

This rules out an immediate deterministic seed-3 deadlock but does not erase the prior exit-125 result. The next full run will retain an exact subcase name if a timeout recurs. It is waiting for the currently active unrelated pantry compiler to clear so the corpus is not knowingly launched into competing build load.

11 days ago

Landed as test(threadfuzz): make lifecycle accounting race-free (#875). Final gate: threadfuzz lifecycle 400 1 passed all 22,400 programs from seeds 1–400 with 0 failures in 5,130,929 ms. Earlier gates also passed seeds 17–36 (1,120/1,120) and suppression-free TSan for seeds 17 and 26 (56/56 each).

Sign in to comment on this issue.