ReviewOS

zig-utils/zig-js

GC: release collection acceleration-index backing stores

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

Parent: #134

GitHub Actions run 29764466248 exposed two allocator leaks in Map/Set stress coverage. Object.collIndexPut activates the coll_index backing flag, but hasObjectBacking did not include that flag and finalizeObjectBacking never deinitialized the AutoHashMapUnmanaged. Reachable collection teardown therefore leaked its acceleration table even though every other cold backing store was finalized.

Acceptance

  • Include coll_index in the object-backing finalization predicate.
  • Deinitialize and reset the collection index before the cold sidecar is destroyed.
  • Keep live backing-store accounting exact.
  • Preserve the real 256-byte cold-sidecar allocation-class invariant without a compiler-layout-sensitive raw-size cap.
  • Prove both previously leaking GC stress tests and the size-class test in Debug and ReleaseSafe.
  • Record the fixing CI evidence and close this issue.
9 days ago

Implemented and pushed in 0eb0600f.

Local evidence:

  • Debug focused contended Map.set test: 3/3 passed, 0 leaked.
  • Debug focused long-lived incremental collections test: passed with 0 leaks.
  • Debug cold-sidecar allocation-class test: passed.
  • ReleaseSafe focused batch: passed.

The production fix now deinitializes and resets ObjectColdState.coll_index, includes the flag in finalization eligibility, and decrements backing-store accounting through the existing released path. The compiler-sensitive raw 224-byte assertion is gone; the allocator’s real <=256-byte class remains enforced. Keeping this open until the fixing GitHub Actions run supplies Linux CI evidence.

9 days ago

Fixing CI recheck is now https://github.com/zig-utils/zig-js/actions/runs/29766523096. The earlier superseded run already showed shard 2 green after 0eb0600f; shard 0 was cancelled by the next direct-main push, so the issue remains open pending the current complete pair.

9 days ago

Fixing CI recheck is now https://github.com/zig-utils/zig-js/actions/runs/29766523096. The earlier superseded run already showed shard 2 green after 0eb0600f; shard 0 was cancelled by the next direct-main push, so the issue remains open pending the current complete pair.

9 days ago

Follow-up d1d73de5 restores 32 bytes of ObjectColdState headroom (192-byte payload / 224-byte raw cell) by moving collection-only indexes behind one tracked state allocation and moving WebAssembly owner trace triplets into native owner storage. Debug unit shards 0 and 1 pass; the exact slab invariant passes in Debug and ReleaseSafe. Awaiting the fixing main CI before closure.

9 days ago

Completed on main through 0eb0600f and d1d73de5. Collection-index backing stores now retire correctly, the cold sidecar remains in its 256-byte class, unit shard 0 is green, and the GC/thread teardown paths pass the fixing threadfuzz-lifecycle job.

Sign in to comment on this issue.