ReviewOS

zig-utils/zig-js

GC: audit Proxy metadata operands across reentrant traps

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

Parent: #465. Related: #808 and #801/#805. Caller-side follow-up: #811.

Confirmed baseline

Engine f2340f2 reproduces nine failing logical moving-GC cases among fifteen isolated ReleaseSafe tests (five process aborts, four assertion failures). A failure in the first forced execution mode stops that logical test; do not count its second mode as executed.

The gaps are continued Proxy target/handler operands across trap lookup and revocation, descriptor values across conversion getters, and retained trap/ target descriptors across nested GetOwnProperty/IsExtensible callbacks. Additional semantic controls expose nested ownKeys invariant ordering, early descriptor-return/throw ordering, accessor validation order, and inherited setters being invoked during descriptor publication.

Implementation and evidence

  • Capture and root the validated target/handler before GetMethod, including revocation during a trap getter. Root/reload result objects, target descriptors, value/getter fields, and receivers across their actual observable callbacks.
  • Own continued encoded key bytes. Apply nested internal methods, and snapshot the ownKeys nonconfigurable partition once in key order. Ordinary target storage uses attributes without allocating a JavaScript descriptor per key.
  • ToPropertyDescriptor validates a getter before reading setter; descriptor publication creates own data fields and never invokes inherited setters.
  • 23 actual-moving cases in both forced tiers (46 verified relocations per test configuration), ten semantic controls in both tiers, shared no-GIL four-lane identity checks, and two allocation-failure sweeps pass Debug, ReleaseSafe and unsuppressed Debug+TSan (26 selected tests each).
  • 33 paired local witnesses improve 21 -> 33 passing; each candidate witness explicitly passes tree-walker and required bytecode without fallback. Node v24.18.0 independently matches all 33 expected results.
  • All 4,048 upstream tests across 14 Proxy/Reflect/Object/expression subtrees remain passing with byte-identical reports: 0 upstream Test262 flips.
  • Full sharded Debug: 2,068 passed, one platform skip, zero failures/leaks, 541.0 seconds across four shards.

Landed on origin/main as fix(proxy): retain metadata operands across reentrant traps (#810), authored/committed by Chris without trailers. ReleaseSafe+TSan also passes all 26 focused tests (9.502s). Seeded Debug fuzz passes 400 default and 300 exact-value programs, both from seed 1, with zero failures.

Pushed integration run is queued. Closure still needs integration evidence; prior CI's separate parallel-collection completion failure is tracked in #812, not claimed fixed here. All completed local build caches were removed; reproducing binaries, source hashes, exact fixtures and logs remain available.

This is synchronous helper ownership. It does not redesign native stacks, queues/jobs, Promise/Worker, Map/Set iterator or native activation lifecycle. No pinning, stale-pointer recovery, disabled GC, relaxed invariant or sanitizer suppression was added. Exact fixtures, hashes and logs are retained locally in /Users/chris/.cache/zig-js-proxy.Hdh6eC.

10 days ago

Now reproduced against exact engine baseline f2340f2. Fifteen isolated ReleaseSafe moving-GC tests run both forced tiers when possible: six logical tests pass, nine fail (five process aborts and four assertion failures). Failing groups are ownKeys trap lookup/revocation, descriptor trap lookup/revocation, descriptor enumerable/writable/accessor conversion, and retained descriptors across nested target/extensibility callbacks. Each selected test must match exactly one linked test; an earlier empty linker placeholder run was rejected as non-evidence. Ten additional ordinary semantic controls also fail baseline, including a segfault on revocation during ownKeys lookup; Node matches all 25 expected control results. Implementation now targets captured handler/target ownership plus exact metadata ordering and own descriptor publication. Evidence: /Users/chris/.cache/zig-js-proxy.Hdh6eC .

10 days ago

Candidate core fix now passes all 26 focused Debug and ReleaseSafe tests: 23 actual-moving cases in both forced tiers (46 verified moves), ten descriptor/order controls in both tiers, shared no-GIL four-lane identity checks, and allocation-failure restoration. The 33 paired local witnesses improve 21 -> 33 passing (baseline includes three segfaults), with zero required-tier fallback. All 4,048 upstream tests across 14 Proxy/Reflect/Object/expression subtrees remain passing; this is 0 upstream Test262 flips. Unsuppressed TSan and full sharded Debug are running. Independent caller-side ownership gaps are tracked in #811; they are not claimed fixed by callee-local roots. Completed build caches were removed after preserving binary hashes and logs.

10 days ago

Verification update: full sharded Debug passed 2,068 tests, one platform skip, zero failures/leaks. All 26 focused tests also pass both unsuppressed Debug+TSan (814.594s) and ReleaseSafe+TSan (9.502s). Sampling confirmed Debug sanitizer time was active allocator stack-trace work; that run was allowed to finish. Seeded default/verify fuzz is the remaining local check before landing. Temporary corpus fixtures and all completed local build caches have been removed after preserving executable hashes and evidence.

10 days ago

Pushed fix(proxy): retain metadata operands across reentrant traps (#810) to origin/main as Chris, no trailers. All local checks are complete, including 700 seeded fuzz programs with zero failures. Integration run 33140348656 is queued; keeping this issue open pending pushed integration evidence. #811 tracks continued caller-side ownership; #812 tracks the separate Linux mid-GC completion assertions.

10 days ago

Post-landing CI is complete for 0ad2b5d9: run 33140348656 finished with 52 successful jobs, one intentional skip, and one failure. The sole failure was the pre-existing #722 witness cve/mc-lock-stop-vs-park.js (expected 400 but got 401), now recorded with its exact job evidence in #722. All Proxy metadata, unit, TSan, no-GIL TSan, test262-parallel, docs, WebAssembly, and threadfuzz jobs relevant to this issue passed. Closing #810; the independent caller audit continues in #811.

Sign in to comment on this issue.