ReviewOS

zig-utils/zig-js

GC: preserve actively resumed generator ownership across moving safepoints

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

Parent: #790. Related: #786, #787, #340, #349.

The #790 actual moving-GC witness reaches a native numeric safepoint from inside a synchronously resumed generator. The next resume then throws TypeError: generator is already running. The candidate unit transcript points to genResume resume_mutex.tryLock. Declaration-only tests pass.

Code audit: Generator embeds its resume mutex and Exec; genResume retains the Generator pointer and saved caller Environment/Values in native locals across execLoop. The relocation binding currently permits every young cell to move. Moving an actively borrowed generator copies the held mutex/inline Exec, while native cleanup and gc_execs can still name the old payload. The native caller restoration copies also lack precise rewrite slots.

Required fix: give the active native resume lifetime an explicit per-cell relocation/ownership ruling, preserve precise saved caller roots, and keep dormant generators relocatable. Do not disable moving GC, suppress the error, clear a copied mutex, or weaken the test. Verify actual movement of the surrounding graph, repeated next/throw/return and reentrancy, failure cleanup, and TSan. Audit the corresponding async native-borrow boundary separately; Promise/job implementation work remains with its owner.

9 days ago

Implementation update (not closure): the synchronous resume now publishes an atomic native-address borrow for the Generator payload/inline Exec/mutex. Both young and full relocation policies retain that individual payload only while borrowed/running; its graph remains rewritable and dormant generators remain eligible to move. Saved caller environment and Values use precise temporary roots, registered before suspended state changes. The actual moving-safepoint regression now passes and proves a surrounding object changes address. Added fault injection for all three resume kinds before first entry and after suspension; full integration and focused ReleaseSafe+TSan are running. The async native-borrow audit remains a separate unfinished boundary; no Promise/job implementation was changed.

9 days ago

Landed a4c035b7 on main. The precise caller-root and native-address borrow implementation includes 18 injected failures across all resume kinds and initial/suspended states. Combined integration with #790/#793: 2006 passes, 1 platform skip, 0 failures/leaks; 700 seeded fuzz programs pass. The actual moving-GC generator witness is in 8d772d15. The async native-borrow/caller-root audit and required pushed CI remain open.

9 days ago

Follow-up #796 landed in 52433f82: synchronous resume now claims native ownership before waiting for the collector trace mutex, preserving real reentry errors and address pinning. Deterministic next/return/throw handshakes, all 18 root-allocation failures, concurrent tracing, and actual moving GC pass. Combined final validation with #794: 2011 unit passes plus one platform skip; 45 generator-related TSan checks, four capture checks, and one moving-GC check; 700 seeded fuzz programs. Async native-borrow/caller-root auditing remains outside this change, and pushed integration is pending.

9 days ago

Pushed integration is now verified: CI run 33113309062, on descendant main 8a1398f, completed with all 53 executed jobs successful and only the scheduled-only nightly job skipped. This includes the synchronous native-borrow implementation and #796's collector/resume ownership correction. The async native-borrow/caller-root audit is still unfinished, so this issue remains open; green CI is not evidence that the unaudited boundary is safe.

9 days ago

Completed the remaining async boundary in 4f02bbd4, now on main.

The implementation adds nested active-activation roots, precise caller restoration roots, reference-counted async address borrows, synchronized/relocatable async-disposal completion state, and an owner-side per-generation parallel-GC publication handshake. The handshake was necessary because exposing the formerly missing peer generator roots initially caused deferred_trace to abort parallel finish; the owning mutator now shades the activation children before publishing the generation, while the collector never reads mutable native-tail state.

Validation:

  • full unit suite: 2,230 passed, 1 skipped, 0 failed, 0 leaked
  • actual moving witness covering async function, async disposal, reentrant async generator, await/throw/return: Debug, ReleaseSafe, and TSan pass
  • shared async-generator resume-claim corpus: GIL and no-GIL pass; no-GIL TSan pass
  • cumulative mid-GC threadfuzz: 1,350 programs across seeds 1-30, 0 failures
  • TSan mid-GC threadfuzz: 45 programs, 0 failures
  • six affected test262 subtrees: 2,738 -> 2,738 pass, 0 flips, identical denominators

The issue auto-closed through Fixes #791. Pushed CI is being tracked separately from this local evidence.

Sign in to comment on this issue.