ReviewOS

zig-utils/zig-js

GC: distinguish collector tracing ownership from generator re-entry

#796
Closed chris opened this 9 days ago ·
9 days ago

Related: #791, #794. GC correctness prerequisite found by the full sharded unit run for #794.

The existing enable_gc concurrent (M3): generators and iterator helpers are safe under concurrent marking test failed with TypeError: generator is already running, originating at genResume's resume_mutex.tryLock() check. The workload has one JS mutator and no recursive generator resume. A focused replay passed, so this needs a deterministic ownership witness, not retries as acceptance.

Both mechanisms are already present on parent main 8d772d1:

  • Generator.tryLockTrace holds resume_mutex while a background marker traces an idle activation.
  • genResume treats any inability to acquire that mutex as JavaScript re-entry.

The collector's temporary lock ownership is not an executing generator. Separate the JS/native resume claim from the collector's short tracing lock, preserve real recursive/concurrent resume rejection, and preserve per-cell native-address pinning and root-allocation failure cleanup. Do not suppress the exception, disable concurrent tracing, retry the JS operation, or change job/queue/lifecycle drivers.

Acceptance:

  • Deterministic marker-owned-mutex witness fails on parent and passes with the fix.
  • Real nested/concurrent .next/.return/.throw re-entry still throws.
  • OOM, native moving-GC, concurrent marking, and TSan witnesses pass.
  • Full sharded units and pushed integration evidence are recorded without discarding the initial failed run.

Completed integration

CI run 33113309062 completed successfully on main commit 8a1398ffc04e9e30987f968693667c7271ca3ce1, which contains this fix. All 53 executed jobs succeeded, including all four unit and TSan-unit shards, Linux no-GIL TSan, functional/no-GIL corpus gates, fuzz profiles, ABI, Wasm, Test262 parallel, and docs. The scheduled-only nightly job was skipped, not counted as a pass. The local evidence above plus this descendant integration run completes this issue's acceptance.

9 days ago

Landed 52433f82 — fix(gc): distinguish generator tracing from resume ownership (#796).

The existing native-resume flag is now an atomic JS resume claim made before waiting for the collector's trace mutex. A real recursive/competing resume still throws; a collector-owned lock no longer does. The same claim pins the generator payload throughout the native borrow and is cleared under the mutex on completion/failure. No collector disabling, operation retry, or queue/job/lifecycle change.

The deterministic handshake fails with the old guard and passes for next, return, and throw after the change. Explicit recursive calls retain all three TypeErrors. The concurrent-marking stress case that failed in the original full run now passes in both Debug and ReleaseSafe+TSan.

Combined final validation with #794: 2011 unit passes, 1 platform skip, 0 failures/leaks; 45/45 generator-related TSan checks (including all 18 root-reservation OOM variants), 4/4 capture checks, and 1/1 actual moving-GC witness. Six exact-parent Test262 subtrees retain 1642 passes and five pre-existing failures (#795) with byte-identical diagnostics, and 1388 passing positive paired-tier witnesses are unchanged. Seeded fuzz passes 400 default + 300 verify programs. Flips 0 Test262 cases in these measured subtrees.

Initial failing full-run evidence is preserved; a passing retry was not used as a substitute for fixing the ownership protocol. Pushed integration remains pending, so the issue stays open.

Sign in to comment on this issue.