ReviewOS

zig-utils/zig-gc

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

#34
Closed chris opened this 10 days ago ·
10 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.
10 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.

10 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.

10 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.

10 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.