fix(gc): allocate-black births were never traced — children reachable only through them were swept live - #6494
Merged
proggeramlug merged 2 commits intoJul 17, 2026
Conversation
📝 WalkthroughWalkthroughArena and malloc GC allocation paths now record black births. Barrier birth flags are cleared at sweep entry, while barrier-seeded work is drained before the sweep snapshot. New tests cover born-black tracing and allocations during the finalize-to-sweep gap. ChangesIncremental GC barrier lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant gc_malloc
participant gc_note_black_birth
participant step_atomic_finalize_current_subphase
participant step_sweep
gc_malloc->>gc_note_black_birth: Initialize and note marked GcHeader
step_atomic_finalize_current_subphase->>step_atomic_finalize_current_subphase: Drain barrier-seeded work
step_sweep->>step_sweep: Drain remaining barrier seeds
step_sweep->>step_sweep: Disable barrier while creating sweep snapshot
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/gc/tests/cycle_state.rs`:
- Around line 718-788: Prevent allocation-time trace seeds from masking both GC
regressions: in crates/perry-runtime/src/gc/tests/cycle_state.rs lines 718-788,
allocate the white child before starting the cycle so only tracing through the
born-black parent preserves it; in lines 790-847, use a hook-free
codegen-equivalent allocation or remove the child’s birth seed before the
barriered store. Update the affected regression tests without changing their
intended assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f49ef04-9023-4409-86ff-f5b5f63305c9
📒 Files selected for processing (6)
crates/perry-runtime/src/arena/allocators.rscrates/perry-runtime/src/gc/barrier.rscrates/perry-runtime/src/gc/cycle.rscrates/perry-runtime/src/gc/malloc.rscrates/perry-runtime/src/gc/tests/cycle_state.rscrates/perry-runtime/src/object/mod.rs
Contributor
Author
|
CI triage for the red checks — none are caused by this PR:
On this branch, |
added 2 commits
July 17, 2026 09:26
… only through them were swept live A budgeted cycle sets allocate-black birth flags at cycle construction, but the insertion barrier only engages once BuildValidPointerSet completes. An object born in a build-phase mutator window is therefore MARKED at birth while its subsequent child stores are unshaded — and marking treats MARKED as "already visited", so the object is never traced. A child linked into it before barrier-enable and reachable through nothing else stays white for the whole cycle and is swept live. Observed in a large compiled TUI app: React creates work-in-progress fibers in exactly that window and cross-links them (`wip.alternate = current`). After a commit swap the old fiber tree is reachable ONLY through those `alternate` back-edges, so an entire live tree was freed one phase later — its objects' payloads stayed readable while their side tables (overflow fields, Set/Map buffers) were cleared, surfacing as intermittent `undefined` reads of constructor-initialized fields (`alternate`, `lanes`, `dependencies`) and `Cannot read properties of undefined (reading 'slice')` crashes at ~50% repro on a scripted PTY run. Three coordinated changes: 1. `gc_note_black_birth`: every born-black runtime allocation (6 arena allocator sites + 2 malloc sites) is also pushed as a mark seed, so the trace drains descend into it and mark its children. This preserves the PerryTS#6224 born-marked contract (raw-installed runtime buffers held only in Rust locals still survive) while restoring the tracing the mark bit was suppressing. 2. The insertion barrier now stays enabled across the AtomicFinalize→Sweep boundary and is disabled inside the first step_sweep slice, right before the sweep state's block-fill snapshot is built — previously the barrier dropped at finalize-end while the snapshot was taken one or more mutator windows later, so a gap-born object sat white INSIDE the snapshot and was freed live. The first sweep slice drains the gap's shaded seeds, disables the barrier, and snapshots atomically. 3. Allocate-black now ends when the barrier disables (sweep entry) instead of at outcome finalization. Sweep-phase births cannot be reached by the in-flight sweep anyway (the arena cursor and the malloc sweep are both snapshot-bounded), while a birth mark they carried was never cleared by that sweep (post-snapshot objects are exactly the ones it never visits) and leaked into the next cycle as "already traced" — re-creating the untraced-black-birth hole one cycle later. Three regression tests, each validated failing-then-passing: - born_black_build_phase_object_is_traced — unrooted born-black object's only-child must survive (fails without the mark seed). - gap_born_child_stored_between_finalize_and_sweep_survives — white child linked from a live parent in the finalize→sweep gap must survive (fails with the early barrier disable). - full_atomic_finalize_slices_barrier_seed_drain_with_tiny_budget — updated contract: barrier active across the finalize→sweep boundary, off after the first sweep slice. Verified end-to-end on the compiled TUI reproducer: 6/6 scripted PTY runs with zero visible errors and zero sweep-then-read incidents (previously 5-6/6 failing runs with 16-20 live fibers freed per incident); perry-runtime suite 1323/0 single-threaded. Claude-Session: https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
…st self-contained Post-CI refinements to the allocate-black tracing fix: - `gc_note_black_birth` skips leaf types (`gc_type_is_pointer_free`): they carry no child edges, so the birth mark alone protects them. Without the skip, a lazy-init burst mid-cycle (the globalThis builtins table populating: thousands of interned strings) turned into pure seed-drain traffic — enough to blow the test harness's 100k single-unit step cap in `full_cycle_bound_prototype_method_cache_after_root_scan_marks_new_value` when run in isolation. - The born-black regression test reclaims its filler blocks with a trailing full collection so later bounded-step tests don't walk seven extra blocks of dead strings, and cargo fmt over the touched files (the lint failure). Claude-Session: https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
proggeramlug
force-pushed
the
fix/allocate-black-untraced-births
branch
from
July 17, 2026 07:30
7ff8d47 to
1d005ac
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/perry-runtime/src/gc/barrier.rs (2)
781-787: 📐 Maintainability & Code Quality | 🔵 TrivialRebuild the static wrapper crates.
As per coding guidelines, when changing runtime or stdlib code, rebuild the corresponding static wrapper crates (
perry-runtime-staticandperry-stdlib-static) because the runtime and stdlib crates themselves emit only rlibs and otherwise may leave stale archives linked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/gc/barrier.rs` around lines 781 - 787, Rebuild the corresponding static wrapper crates per the repository’s standard procedure: perry-runtime-static and perry-stdlib-static. Ensure their generated static archives reflect the runtime change near GC_BIRTH_EXTRA_FLAGS and are included in the resulting changes.Source: Coding guidelines
808-812: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRead the header flag directly to avoid a thread-local storage lookup.
Since all callers assign
gc_birth_extra_flags()to(*header).gc_flagsimmediately before callinggc_note_black_birth, you can check the header's flags directly. This avoids a redundant thread-local storage access on the allocation hot path.♻️ Proposed fix
#[inline] pub(crate) fn gc_note_black_birth(header: *mut GcHeader) { - if GC_BIRTH_EXTRA_FLAGS.with(|cell| cell.get()) & GC_FLAG_MARKED == 0 { + if unsafe { (*header).gc_flags & GC_FLAG_MARKED } == 0 { return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/gc/barrier.rs` around lines 808 - 812, Update gc_note_black_birth to inspect (*header).gc_flags for GC_FLAG_MARKED instead of reading GC_BIRTH_EXTRA_FLAGS through thread-local storage. Preserve the early return when the header is not marked, relying on callers having already assigned gc_birth_extra_flags() to the header.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-runtime/src/gc/barrier.rs`:
- Around line 781-787: Rebuild the corresponding static wrapper crates per the
repository’s standard procedure: perry-runtime-static and perry-stdlib-static.
Ensure their generated static archives reflect the runtime change near
GC_BIRTH_EXTRA_FLAGS and are included in the resulting changes.
- Around line 808-812: Update gc_note_black_birth to inspect (*header).gc_flags
for GC_FLAG_MARKED instead of reading GC_BIRTH_EXTRA_FLAGS through thread-local
storage. Preserve the early return when the header is not marked, relying on
callers having already assigned gc_birth_extra_flags() to the header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0700a12-5f75-4670-ba8a-6a416680f223
📒 Files selected for processing (6)
crates/perry-runtime/src/arena/allocators.rscrates/perry-runtime/src/gc/barrier.rscrates/perry-runtime/src/gc/cycle.rscrates/perry-runtime/src/gc/malloc.rscrates/perry-runtime/src/gc/tests/cycle_state.rscrates/perry-runtime/src/object/mod.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/perry-runtime/src/object/mod.rs
- crates/perry-runtime/src/arena/allocators.rs
- crates/perry-runtime/src/gc/cycle.rs
- crates/perry-runtime/src/gc/tests/cycle_state.rs
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Jul 17, 2026
…em (PerryTS#6495) `visit_overflow_field_slots_mut` (mark-phase trace + rewrite walks) and `scan_overflow_fields_roots_mut` consulted the per-object layout slot mask to visit only pointer-bearing overflow slots. The mask is maintained by `layout_note_slot` at store time — but not every overflow write path notes: GC owner moves merge entries via `merge_overflow_fields` with no notes, so a usable-looking SIDE_MASK can claim pointer-bearing overflow slots are pointer-free. The trace then skips them, and a child reachable only through such a slot is swept while referenced. Observed at bundle scale in a large compiled TUI app (diagnostics from the PerryTS#6494 investigation): 17 objects whose masks capped at bit 48 while slots 49..63 held live NaN-boxed heap pointers — ~3,200 skipped pointer-slot visits per run. Visit every overflow slot unconditionally. The Vec's length is the live overflow region, and objects with large overflow populations sit in UNKNOWN layout state in practice (every dynamic-shape store degrades the layout), so the mask fast path bought little exactly where it would cost. The inline-slot region keeps its layout-driven visit — its stores all funnel through the noting choke points. Regression test `overflow_slots_beyond_layout_mask_are_traced`: a rooted object with a SIDE_MASK claiming slots 0..=48 and the only reference to a live string sitting in overflow slot 50 (seeded without notes, the merge_overflow_fields shape); asserts the string is MARKED at sweep entry. Validated failing-then-passing against the old fast path (block-persistence window aged out so the recent-block resurrection pass cannot mask the miss). perry-runtime suite: 1322/0 single-threaded. Claude-Session: https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
A large esbuild-bundled TUI app (React-based) intermittently loses constructor-initialized object fields at runtime: reads of
alternate/lanes/dependencieson live React fibers returnundefined, and a cachedSet's iteration yieldsundefined(Cannot read properties of undefined (reading 'slice')). ~50% repro on a scripted PTY run, GC-mode-insensitive (default /PERRY_GC_FORCE_EVACUATE=1/PERRY_GEN_GC_EVACUATE=0identical).Diagnosed with an env-gated side-table tracer: a live object's overflow entry was cleared by the sweep (
clear_overflow_for_ptr) and then read moments later through a still-held reference — the object was swept while alive. Its payload stayed readable (inline fields fine,tagintact), which is why the failure surfaced as "some fields are undefined" rather than a crash.Root cause
Budgeted cycles set the allocate-black birth flag (
GC_BIRTH_EXTRA_FLAGS = MARKED) at cycle construction, but the insertion barrier only engages at the end of BuildValidPointerSet. An object born in a build-phase mutator window is:A child linked into such an object before barrier-enable — and reachable through nothing else — stays white for the entire cycle and is swept live.
React hits this shape deterministically: work-in-progress fibers are created in bursts (runtime construct path → born black) and immediately cross-linked (
wip.alternate = current). After a commit swap, the old fiber tree is reachable only via thosealternateback-edges, so a whole live tree was freed one phase later (reverse-reference scan confirmed: victims' only marked referrer held them via an overflow slot, and that referrer had zero trace events — born-black, never traced).Two adjacent windows had the same consequence and are fixed together:
step_sweepslice — one or more mutator windows later. Gap-born objects sat white inside the snapshot.Fix
gc_note_black_birth— every born-black runtime allocation (6 arena allocator sites + 2 malloc sites) is also pushed as a mark seed; the trace drains (which already absorb seeds every step) descend into it and mark its children. Preserves the GC: incremental mutator assists use a fixed budget — allocation outruns the collector, RSS grows unbounded (6-22x measured) #6224 born-marked contract for raw-installed runtime buffers.step_sweepslice drains the gap's shaded seeds, disables the barrier, and builds the block snapshot atomically (no mutator window between barrier-off and snapshot).Tests
Three regression tests, each validated failing-then-passing by temporarily reverting the corresponding change:
born_black_build_phase_object_is_traced— an unrooted born-black build-phase object's only-child must survive the cycle (fails without the mark seed; block-persistence window aged out to unmask the sweep).gap_born_child_stored_between_finalize_and_sweep_survives— a white child linked from a live parent in the finalize→sweep gap must survive (fails with the early barrier disable).full_atomic_finalize_slices_barrier_seed_drain_with_tiny_budget— contract updated: barrier active across the finalize→sweep boundary, off after the first sweep slice.perry-runtimesuite: 1323 passed / 0 failed (--test-threads=1).End-to-end on the TUI reproducer: 6/6 scripted PTY runs with zero visible errors and zero sweep-then-read incidents (previously 5–6/6 failing, 16–20 live fibers freed per incident, deterministic at the same sweep).
Cost
One
Vecpush per runtime allocation while a budgeted cycle is active (seeds are deduplicated by the born-marked bit and drained incrementally); born-black floating garbage now also gets traced, keeping its children one extra cycle — same retention class as allocate-black itself.https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
Summary by CodeRabbit