ReviewOS

zig-utils/zig-js

GC: publish provisional VM activation roots failure-atomically

#787
Closed chris opened this 10 days ago ·
10 days ago

Parent: #786 Related: #477, #493

Audit finding

Interpreter.pushExecRoot currently discards gc_execs.append allocation errors. A failed root-list growth can therefore run bytecode without registering its precise roots. Separately, buildActivation installs callee state and performs argument/rest/parameter/debugger allocations before execLoop or runDriver registers its Exec. Saved caller state and newly prepared frame slots need precise ownership during that provisional window too.

This is a pre-existing publication/lifecycle issue found while fixing the stale saved caller copies in #786; it is not solved by moving those copies into Exec alone.

Scope and acceptance

  • Root registration propagates allocation failure before executing an unregistered activation.
  • Provisional activation preparation publishes all live managed caller/callee state before any allocation-recovery or observable call boundary.
  • Every error path restores the caller and unregisters/recycles exactly the records it owns, including tail replacement and driver-list growth failures.
  • Deterministic allocation-failure sweeps and moving-GC witnesses cover initial, inline, nested-driver, and tail-call entries.
  • Normal/TSan/OOM and required integration gates pass with no ignored allocation errors, pinning, disabled GC, or fallback.

Coordinate any activation-driver lifecycle changes with the actor handling stack/queue/job-native work; this issue must not change Promise, Worker, or iterator behavior.

10 days ago

Follow-up source audit against b023626a identifies the concrete failure-injection boundaries for this lifecycle work:

  • buildActivation debugger preparation: the environment allocation and binding puts use bare try after callee state installation. Unlike surrounding argument/rest/parameter failures, these exits do not restore/recycle the activation.
  • runDriver: initial list growth has explicit cleanup, but nested list growth, debugger synchronization, native-entry errors, and return-value delivery can leave through bare try while owned activations remain registered. The handler-delivery appends in unwindThrow require the same audit.
  • execLoop root entry: scratch/binding storage preparation precedes root registration, and registration currently cannot report failure. The inline caller has a restoration defer once buildActivation succeeds; the ownership contract must cover the earlier preparation window too.

A coherent fix should define one provisional-to-running ownership handoff, then inject allocation failure at each entry/append/sync/handler/return boundary and assert caller state, logical depth, precise-root list, pending activation, and reusable activation state are restored exactly. Actual moving-GC probes should force collection during preparation, not only inside the eventual instruction loop.

This is read-only audit evidence, not a claimed repro or implemented fix. I have left activation-driver/stack lifecycle implementation untouched for coordination with the reserved native stack/queue/job work.

10 days ago

Implemented and pushed in be776fef (fix(vm): make activation rooting failure-atomic (#787)).

The VM now reserves precise-root capacity before it exposes an activation, publishes each prepared activation exactly once, and treats that root as explicit ownership transferred to inline execution or the heap driver. One cleanup path restores/unregisters/recycles every live activation on debugger-sync, native-entry, handler-delivery, activation-list growth, depth rejection, tail replacement, return delivery, and ordinary error exits. Root registration OOM now propagates before the first instruction instead of being discarded. Arguments and eager rest arrays are published directly in the registered frame, eliminating the now-redundant temporary-root allocations.

Regression evidence:

  • deterministic allocation-index sweeps cover initial, inline, deep nested/list-growth, tail, handler, and debugger entries and assert caller state, depth, root lists, pending activation flags, and activation-pool ownership after every induced failure;
  • a real moving nursery collection runs after callee preparation and before its first instruction, relocates a managed argument, and resumes from the rewritten frame;
  • all 12 existing parameter/field activation-allocation tests pass;
  • focused new regressions pass normally and under TSan;
  • zig build test-parallel: 2,223 passed, 1 skipped, 0 failed, 0 leaked.

No GC disabling, pinning, fallback, or ignored allocation error was introduced.

Sign in to comment on this issue.