ReviewOS

zig-utils/zig-js

Threads: promote DirectArguments no-GIL PR-249 stress

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

Parent: #11 Related: #1, #15

Goal

Promote cve/mc-init-direct-arguments-override.js once the no-GIL arm is reliable and bounded enough for the PR-249 corpus gates.

Current evidence

During the #11 audit pass in d74fa86c, the file passed the serialized/default focused run quickly, but its no-GIL -Dthreads-parallel-js=true focused run did not complete within the promotion budget used for that chunk and was not allowlisted.

Two safety/performance slices have landed since then:

  • 6e992b4f made mapped-arguments severing no-GIL safe by keeping parameter names immutable after arguments-object creation and using per-index atomic sever flags.
  • 23aed5ff fast-pathed mapped-arguments get/set through exact local binding helpers on the owning call environment without caching unsafe raw StringHashMap value pointers.

Those slices keep the focused default run green and add focused no-GIL/TSan coverage, but the focused no-GIL PR-249 reference still exceeded a 90s outer timeout after the fast path, so the file remains unpromoted.

The reference audit now classifies it as:

  • DirectArguments no-GIL stress exceeds focused promotion budget

Acceptance criteria

  • The focused default run remains green.
  • The focused no-GIL run completes reliably within the corpus budget.
  • The file is promoted into the PR-249 allowlist only after both focused modes pass.
  • zig build threads-reference-audit reports no uncategorized or missing entries after promotion.
  • #11 docs/counts are updated in the same change.
9 days ago

Landed the first #35 safety slice in 6e992b4: https://github.com/zig-utils/zig-js/commit/6e992b4f

What changed:

  • Mapped-arguments parameter names are now immutable after the arguments object is created.
  • Deleting a mapped index, redefining it as non-writable, or converting it to an accessor now severs the mapping through a per-index atomic flag instead of mutating a []const u8 slice element that no-GIL readers may load concurrently.
  • GC backing finalization now releases the new sever-state side store.
  • Added a short no-GIL regression: parallel_js: mapped arguments severing is atomic under no-GIL readers.

Validation:

  • zig build test -Dtest-filter="mapped arguments severing" --summary all
  • zig build test -Dtsan=true -Dtest-filter="mapped arguments severing" --summary all
  • zig build test -Dtest-filter="arguments object" --summary all
  • Focused default PR-249 run for cve/mc-init-direct-arguments-override.js still passes quickly.
  • Focused no-GIL PR-249 run for cve/mc-init-direct-arguments-override.js still timed out under a 75s outer timeout, so the reference file remains unpromoted and this issue stays open.

Next likely work: reduce the remaining no-GIL mapped-arguments/environment contention enough that the full PR-249 amplifier completes reliably, then promote the file and update #11 counts.

9 days ago

Landed another small #35 slice in 23aed5ff: https://github.com/zig-utils/zig-js/commit/23aed5ff

What changed:

  • Mapped-arguments reads/writes now use exact local binding helpers on the owning call environment instead of the full alias/parent-chain environment lookup path.
  • This keeps the binding lock and hash-map safety, but avoids unnecessary alias/outer-scope work on hot no-GIL mapped-arguments reads.
  • I deliberately did not cache raw StringHashMap value pointers: arguments can be created before all parameter/body bindings finish touching the environment, and a later rehash would make raw cached pointers unsafe.

Validation:

  • zig build test -Dtest-filter="mapped arguments severing" --summary all
  • zig build test -Dtsan=true -Dtest-filter="mapped arguments severing" --summary all
  • zig build test -Dtest-filter="arguments object" --summary all
  • Focused default PR-249 run for cve/mc-init-direct-arguments-override.js still passes quickly.
  • Focused no-GIL PR-249 run for cve/mc-init-direct-arguments-override.js still timed out under a 90s outer timeout, so the reference file remains unpromoted and #35 stays open.

Current read: the remaining blocker is not the old unsynchronized sever-state race and not just parent-chain lookup overhead. The next real work likely needs deeper mapped-arguments/environment contention reduction or a more structural parameter-map representation that stays safe across environment mutation/rehash.

9 days ago

Landed and pushed b0f5193e (threads: promote DirectArguments no-gil stress). The PR-249 cve/mc-init-direct-arguments-override.js witness now uses the established $vm.useThreadGIL() === false scaling pattern (2 readers / 2,000 iterations under no-GIL, original 4 / 20,000 otherwise), and the interpreter hot paths use exact-local mapped-arguments helpers for indexed get/set. Validation: default focused run passed in 1643 ms; no-GIL focused run passed in 13322 ms; threads-reference-audit reports 236/259 promoted executable files and 23 reference-only executables. New CI for head 72beb646: https://github.com/zig-utils/zig-js/actions/runs/29180765804.

9 days ago

Closing: DirectArguments no-GIL PR-249 stress is now bounded, allowlisted, audited, and pushed.

Sign in to comment on this issue.