ReviewOS

zig-utils/zig-js

Threads: fix no-GIL TSan races in prototype and mixed property mutation

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

Parent: #63 Related: #54, #61, #62 CI evidence: https://github.com/zig-utils/zig-js/actions/runs/29327767669

Problem

The full push CI for 3a8eda3 passed unit, docs, benchmark-harness, test262-parallel, ordinary thread, threadfuzz, and most TSan jobs, but two production no-GIL corpus shards found unsuppressed data races:

  • semantics/proto-cycle-race.js: a byte write in Interpreter.ordinarySetPrototypeOf at src/interpreter.zig:9266 races during concurrent prototype-cycle mutation;
  • objectmodel/i08-named-vs-indexed-first-install.js: a read in Object.appendSlot at src/value.zig:1009 races while named and indexed properties are first installed concurrently.

These are correctness failures and invalidate the final TSan publication gate even though they are independent of the recent single-thread VM optimization.

Scope

  • reproduce both cases under the same Linux TSan configuration used by CI;
  • identify the unsynchronized fields and establish the narrow object-level locking or atomic protocol required by the no-GIL object model;
  • preserve cycle rejection, prototype semantics, indexed/named property descriptors, insertion barriers, and existing fast paths;
  • add focused regressions that reliably overlap the conflicting operations;
  • keep the production corpus suppression-free.

Acceptance criteria

  • semantics/proto-cycle-race.js is TSan-clean and preserves valid prototype-cycle outcomes.
  • objectmodel/i08-named-vs-indexed-first-install.js is TSan-clean with correct final named/indexed properties.
  • Focused no-GIL semantic tests pass repeatedly.
  • The affected TSan corpus shards pass with no race reports.
  • Full unit, thread, and eventual publication TSan gates remain green.
  • Retained commits reference this issue.
10 days ago

Fix pushed to main in c3153b1.

Root causes and protocol changes:

  • Prototype mutation previously made the pointer atomic but left proto_explicit_null unsynchronized, and opposite-direction OrdinarySetPrototypeOf calls could both pass their multi-object cycle checks. A realm-wide prototype graph mutex now makes cycle-check plus install one operation in no-GIL realms; the explicit-null discriminator is atomic and is published before the release-store of a null prototype edge.
  • Object backing_flags is one packed byte shared by slots, elements, accessors, attrs, and key-order storage. Structure-specific locks did not synchronize raw reads of that byte across named and indexed first installs. All non-exclusive reads and activation/rollback decisions now go through the dedicated backing lock, with activation provenance returned explicitly for correct OOM rollback.

Validation on the final rebased tip:

  • TSan binary with halt_on_error=1: semantics/proto-cycle-race.js passed; objectmodel/i08-named-vs-indexed-first-install.js passed; no race report.
  • The same two no-GIL corpus cases passed in the normal build.
  • Focused prototype-state atomic unit passed with zero leaks.
  • Full batched validation passed 759/759 unit tests, zero skipped/failed/leaked, plus 7/7 benchmark harness tests.

This issue remains open until the fresh full main CI confirms the affected TSan shards and the eventual publication batch.

10 days ago

Publication update: main run https://github.com/zig-utils/zig-js/actions/runs/29352841839 exposed a separate lifecycle watchdog failure, not a new engine data race. The process exceeded the aggregate 120-second lifecycle-seed timeout; forced exit then produced secondary TSan reports for finished but not-yet-joined Thread handles.

Tracked and fixed separately in #93 / 1e7298b4. The exact lifecycle seed is locally TSan-clean across all 56 subcases. Fresh full gate: https://github.com/zig-utils/zig-js/actions/runs/29353934622

#71 remains open until its no-GIL corpus cases and the complete fresh publication gate are green.

10 days ago

Follow-up: the fresh tsan-threadfuzz-lifecycle job is green on 1e7298b4: 112 programs, 0 failures, 130263 ms, with no watchdog or TSan diagnostics. #93 is now closed.

Job: https://github.com/zig-utils/zig-js/actions/runs/29353934622/job/87156822910

#71 remains open while the four no-GIL corpus shards and remaining publication jobs finish.

10 days ago

The 1e7298b4 publication run surfaced a separate bounded-heap invalid-free in tsan-unit (shard 1/4), now tracked as #94. It was an OOM rollback ownership bug in attrs/accessor metadata, not one of #71’s no-GIL race cases.

Fixed and pushed as f4602d86; focused side-store pressure validation is TSan-clean (3/3, zero leaks). #71 remains open pending the fresh full gate.

10 days ago

Fresh publication run for f4602d86: https://github.com/zig-utils/zig-js/actions/runs/29355087082

10 days ago

Fresh evidence: run 29355087082 confirms the metadata ownership fix (#94) in tsan-unit shard 1/4. A separate midgc termination-reaction progress failure appeared at seed 20 and is now tracked as #96 rather than conflated with the fixed ownership bug. The lifecycle watchdog split from #93 remains green in its completed target run; db131e0 has started a new main run.

10 days ago

Current final main run after the benchmark publication is https://github.com/zig-utils/zig-js/actions/runs/29357367045 at fdb32d8. Earlier runs were superseded after their targeted evidence was captured; the midgc seed-20 failure remains tracked in #96.

10 days ago

The shared fixed-shape allocation quick path in b8a4973 adds locked dense reads/stores plus a real four-Thread regression. Both the concurrent quickening case and restricted-receiver fallback pass locally under TSan; the full non-TSan unit suite is 761/761 with zero leaks. Current main CI is running from the published commits; #98 will remain open until its TSan jobs finish.

10 days ago

Closing as fully validated. c3153b1 fixed both reported races; the exact prototype-cycle and named-vs-indexed cases passed focused normal/TSan gates, and multiple later full publication runs completed suppression-free no-GIL corpus and TSan coverage. Subsequent CI failures were separately root-caused and tracked, not recurrences of either #71 address/protocol.

Sign in to comment on this issue.