ReviewOS

zig-utils/zig-gc

GC: run embedder post-sweep work after collector locks release

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

Required by zig-utils/zig-js#324 and zig-utils/zig-js#325.

Problem

Binding.finalize runs inside the sweep loop. In parallel mode that loop holds alloc_lock, so an embedder cannot safely invoke reentrant external-owner callbacks from a cell finalizer. The collector already documents that finalizers must not allocate from the heap, but it has no generic point for draining work after the sweep lock is released.

Scope

  • Add an optional binding post-sweep hook that runs after alloc_lock is released for full, nursery, concurrent, and parallel concurrent sweeps.
  • Guarantee the hook observes completed finalization/accounting and never runs under alloc_lock or weak_lock.
  • Do not invoke it for aborted cycles or heap teardown; embedders drain teardown work explicitly after heap deinit, when the heap cannot be reentered.
  • Add focused binding tests proving ordering, lock state, exact invocation count, and no invocation on an aborted parallel finish.
  • Document the callback contract without coupling zig-gc to any particular embedder owner type.

Acceptance

  • Existing collector tests remain green.
  • Focused tests cover ordinary collection, nursery collection, parallel concurrent success, and abort.
  • zig-js can queue external string/buffer releases during Binding.finalize and drain them from the hook without reentering collector locks.
9 days ago

Implemented and pushed in 579f683. The optional Binding.afterSweep hook now runs exactly once after every successful full, nursery, incremental/concurrent, or parallel-concurrent sweep, after alloc_lock is released and finalizer/accounting work is complete. Aborted parallel cycles do not invoke it. The focused ordering/lock/abort coverage and the complete zig-gc test suite pass. zig-utils/zig-js#324 will use this hook to drain queued external-owner releases; teardown remains an explicit post-heap-deinit embedder drain.

9 days ago

Reopened for a follow-up ordering correction found during zig-js integration: the hook must run after sharded publication is reopened as well as after alloc_lock is released, so callback reentry sees a fully restored heap state.

9 days ago

Ordering correction pushed in 9f9e852: afterSweep now runs only after sharded publication has reopened, in addition to running outside alloc_lock. The focused probe asserts both invariants across nursery, full, and parallel-concurrent success; abort remains non-invoking. The full collector suite is green.

9 days ago

Correction: the ordering-fix commit is 05f9b6e (the previous close comment contained a mistyped short hash).

Sign in to comment on this issue.