ReviewOS

zig-utils/zig-js

Compiler: keep for-of iterator temporaries activation-local

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

Parent: #465. Related: #476, #752, and #812 (discovered while reconciling that issue's CI evidence).

Linux CI run 33440235057 failed the ReleaseSafe no-GIL corpus shard in jit/spawned-thread-butterfly-stress.js with TypeError: undefined is not an object: https://github.com/zig-utils/zig-js/actions/runs/33440235057/job/99646369453

The exact ReleaseSafe case reproduces locally and intermittently, including with native JIT disabled. A diagnostic preserving the original for (const o of own) loop showed the owning thread's packed Array iterator yielding undefined at an index whose direct own[index] read still returned the expected object.

Root cause: compileForOf creates its iterator, captured next, result, and close-state bookkeeping with raw freshTemp() names but never registers those names as function locals. emitDefine/emitLoad therefore resolve the synthesized names through the shared Environment. Concurrent invocations of the same compiled function overwrite each other's Iterator Records and results. This is also wrong for recursive/reentrant execution. compileYieldStar uses the same raw temporary pattern and should use the same activation-owned abstraction.

Implement the fix at the compiler storage boundary: use frame-local bindings for ordinary functions, private activation bindings for suspendable functions, and Exec-owned scratch for program chunks. Do not serialize execution, add sleeps, weaken the corpus assertion, or special-case the test. Add structural compiler coverage plus required-bytecode parallel runtime coverage, then verify the exact ReleaseSafe no-GIL case repeatedly, unsuppressed TSan, focused/full units, and the relevant corpus gate.

9 days ago

Landed on main in 7d1ce5c5.

The compiler now keeps for-of and yield-star protocol state in execution-owned storage: frame locals for ordinary functions, Exec scratch slots for program chunks, and the private variable environment for suspendable activations.

Validation:

  • exact ReleaseSafe reproducer: 100/100 passes with JIT and 100/100 with no-jit
  • exact unsuppressed TSan run: pass, no sanitizer report
  • ReleaseSafe no-GIL corpus shard 2: 63/63
  • affected test262 subtrees: 3,528 -> 3,528, 0 failures and 0 flips
  • focused compiler/runtime witnesses and the affected generator, async, and yield-star tests: pass
  • two corrected full unit runs each accounted for all 2,233 tests; each had one distinct unrelated load-only shared-GC witness flake, now tracked in #839, and both witnesses passed 3/3 in isolation

This closes the shared-activation iterator corruption without weakening synchronization or adding timing workarounds.

Sign in to comment on this issue.