ReviewOS

zig-utils/zig-js

GC: keep Environment ownership metadata immutable during reuse

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

Parent: #476 Related: #478, #693

Problem

The full TSan matrix after #693 exposed a race in the existing reusable Environment path. Environment.gc_managed is documented as immutable after initialization, but acquireBlockEnvironment and acquireCallEnvironment rewrite it while a concurrent marker can read the same cell through another Environment parent edge. The writer holds the reused cell binding lock; the marker holds the child Environment lock, so those operations have no happens-before relation. Rewriting the same boolean value is still an engine-state race.

Observed witness: C-API JSValueProtect roots survive mid-script parallel GC. TSan reports acquireBlockEnvironment at interpreter.zig:3383 racing markManaged at gc.zig:154. The JavaScript assertion itself passes; the sanitizer correctly exits nonzero.

Scope

  • Preserve GC ownership metadata as a true allocation-time invariant across block and call Environment reuse.
  • Remove reuse-time ownership writes rather than adding cross-object locking.
  • Assert that every reusable side-storage Environment retains the allocation mode required by its cache.
  • Cover both block and call caches while a concurrent marker traces parent chains.
  • Preserve parent-edge barriers, side-storage ownership, cache publication, capture semantics, and arena-only Environment behavior.

No-workaround rules

  • No TSan suppression, relaxed sanitizer exit, race allowlist, disabled reuse, disabled concurrent marking, broad heap-membership scan, or redundant atomic metadata.
  • No stacks, queues, jobs, Promise, Worker, or Map/Set iterator changes.

Acceptance

  • gc_managed is assigned exactly once at Environment initialization and never mutated during reuse.
  • Focused block/call reuse witnesses pass under Debug and TSan with concurrent marking.
  • The observed C-API parallel-GC witness is TSan-clean.
  • Full Debug and full TSan units pass with zero failures, leaks, or sanitizer warnings.
9 days ago

Completed on main in fix(gc): preserve Environment ownership on reuse (#694).

Environment.gc_managed is now assigned only at allocation/initialization and is read-only during both block and call cache reuse. Debug assertions bind the cached cell to the Interpreter GC mode. Parent-edge barriers, cache publication, bindings, capture behavior, and arena-only paths are unchanged; no atomic metadata or suppression was added.

Validation:

  • exact former race witness: 1/1 Debug, 1/1 TSan, no warning/leak
  • Environment controls: 10/10 Debug, 10/10 TSan
  • full Debug: 1791 passed, 1 skipped, 0 failed, 0 leaked
  • full TSan: 1790 passed, 2 skipped, 0 failed, 0 leaked, zero ThreadSanitizer reports
  • applicable Test262 cases: 0; this is a C-API parallel-GC engine-state race

The prior nonzero TSan exit is resolved without changing sanitizer policy.

Sign in to comment on this issue.