fix(runtime): root JS values retained in Rust containers across allocations (#7949)

[proggeramlug]

Closes #7949.

Raw NaN-boxed JS values were retained in ordinary Rust containers across calls that can allocate — a user callback, a [[Get]] accessor, a key coercion, a result-array build. Those Vec elements are neither runtime roots nor mutable shadow slots, so an evacuating collection could neither keep them alive nor rewrite their addresses. scripts/gc_root_dominance_check.py is structurally blind to this: it reads emitted LLVM IR, and a Rust-side container is not in the IR.

Sites

object/groupby.rs — group_by_collect returned Vec<(f64, f64)> filled across js_closure_call2, i.e. across arbitrary user JS, once per element. The audit found three more holes in the same family that the issue's summary did not name:

• the materialized input array and the closure were hoisted out of the loop and re-dereferenced on every iteration, across the callback;
• Object.groupBy's result object had no root at all — it is reachable from nothing else until it is returned, and it spanned one array build plus one key-string intern per group;
• group_by_make_array returned a freshly allocated array across rebuild_array_layout_from_slots, and the caller's boxed copy then spanned the key interning.

Also: Symbol keys were coalesced through a HashMap keyed on the symbol's address. A fresh Symbol(desc) is a GC allocation, so a symbol that moved mid-loop hashed to a new bucket and started a duplicate group. Now keyed on SymbolHeader::id, which an evacuation copies verbatim — the same reasoning #7246 used for symbol descriptions.

object/object_ops/define_properties.rs — keys: Vec<f64> was walked across js_string_coerce, js_dynamic_object_get_property (a user getter on the properties bag) and js_object_define_property, with the receiver, the bag, the own-names array and the coerced key string all bare locals in the same window.

proxy/own_keys.rs (issue item 5, found by sweeping for the same convention) — alloc_key_array grew the result array between key pushes, and proxy_enum_own_keys held the trap's key list and the accepted-so-far list across the proxy's getOwnPropertyDescriptor trap.

The reusable rule

gc::RootedValues (gc/roots/rooted_values.rs): a growable list whose elements are RuntimeHandles. The handle stack is a registered mutable root scanner, so elements are marked and their slots rewritten, and get(i) re-reads the slot. It is not a proof — Rust has no effect system for "this call may allocate" — but it removes the container as the hole and makes the correct shape the short one. The module docs record the one footgun: never push to an outer RootedValues while an inner RuntimeHandleScope is alive, because the inner scope's drop truncates the stack.

No new root holder (it borrows the existing handle stack, so gc_runtime_root_holders.py needs no verdict) and no new get_raw_{mut,const}_ptr sites (raw_handle_debt.py unmoved — it reads 3 below baseline).

How the fix is proven

Not "it didn't crash". Every assertion is gated on a collection that actually moved something.

gc/tests/rooted_container_values.rs, four tests under CopyingNurseryTestGuard with a forced collect_minor_trace:

1. rooted_values_elements_survive_a_collection_that_moved_them — asserts copied_objects > 0, that every element's address changed, and that the post-collection pointer still reads the original bytes.
2. plain_vec_of_values_is_not_a_root — the sabotage arm. The identical workload in a bare Vec<f64> keeps naming pre-collection addresses while a rooted witness in the same cycle moves. This is what makes (1) non-vacuous.
   3./4. object_group_by_… / map_group_by_… — end to end through the real #[no_mangle] entry points, callback forcing an evacuating minor on every element, groups verified by string bytes.

Sabotage verification (fix committed first): with object/groupby.rs reverted to origin/main and nothing else changed, both end-to-end tests abort with TypeError: value is not a function — the canonical late-surfacing form of this class. With the fix restored, all four pass.

Compiled witness. Two gap tests, each A/B'd against a pristine origin/main build with identical -p sets and a pinned PERRY_RUNTIME_DIR, under

PERRY_GC_SCHEDULE_SEED=1 PERRY_GC_SCHEDULE_RATE=1 \
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800

probe	pristine	this branch
test_gap_gc_container_value_rooting.ts	exit 138, [gc-fromspace-protect] FAULT: signal 10 naming a retired from-space address	exit 0, byte-exact vs node 26.5.1
test_gap_gc_define_properties_key_rooting.ts	exit 138, same	exit 0, byte-exact vs node 26.5.1

The instrument is shown live in both runs (retired_set=#N … bytes_protected=…), so a green arm is a protected arm and not a run with zero copying minors.

Why the two probes are separate programs

A groupBy arm followed by any defineProperty-with-getter work faults under the witness configuration even with this fix in — and it faults identically when Object.defineProperties is replaced by a hand-written Object.defineProperty loop, i.e. with none of the code this PR touches on the path. That is a separate pre-existing defect in the defineProperty/getter family — the wider window #6949's scope note names and defers (js_object_define_property holds obj / descriptor_value and the six raw JSValues inside DescView across its own later js_string_from_bytes calls). Keeping the programs apart means each gap test fails for its own reason. Filed separately.

Validation

check	result
cargo test -p perry-runtime --lib	2215 passed, 0 failed, 4 ignored (#7946's flakiness did not appear)
gap corpus — all 36 test_gap_gc_*.ts + 5 test_gap_{proxy,reflect}*.ts, byte-exact vs node 26.5.1, in the default config and under PERRY_GC_PROTECT_FROMSPACE=1 DEPTH=800	41/41
both new probes under PERRY_GC_VERIFY_EVACUATION=1 (+ seeded schedule)	exit 0, byte-exact
cargo fmt --check, check_file_size.sh, gc_runtime_root_holders.py, raw_handle_debt.py	clean (debt reads 3 below baseline)

Instrument liveness, because a run with zero copying minors protects nothing:

• test_gap_gc_container_value_rooting: 164 safepoints / 164 copying minors / 67,889 objects moved, 164 quarantined retired sets.
• test_gap_gc_define_properties_key_rooting: 60 / 60 / 33,302 moved, 60 retired sets.
• Across the 41-program corpus the quarantine printed a retired-set line in 20 of 41 programs (185 sets). The other 21 ran no copying minor, so for those the protected arm is a no-regression check, not a rooting witness — stated rather than glossed.

Sweep (issue item 5)

A scan for the shape — a Vec<f64>/Vec<(f64…)> accumulator pushed inside a loop that also calls something allocating or user-facing, with no RuntimeHandleScope — returns 14 functions. proxy/own_keys.rs (2) are fixed here. js_typed_array_filter is real and deliberately deferred (its kept vector spans a user callback, and ta/recv are hoisted raw locals across the same callback — the ta half is #6949 shape (a), so both want one change). The remaining 11 are argument-gathering: the Vec is filled from an already-materialized source with no allocating call between pushes, and converting them would cost a handle push per argument on the hottest dispatch paths for no demonstrated window. object/descriptors.rs — the function the issue quotes as carrying the "builder helpers follow this convention" comment — was already converted by #6943/#7341. Full table in gc-handoff/ROOTVEC-NOTES.md.

#7803

Could not be reproduced: test-files/gc-dep-corpus/main.ts does not link on this box (Undefined symbols: _perry_fn_node_modules_zod_src_v4_core_index_ts__NEVER, …__brand, … — the export * re-export set in the currently pinned zod@4.3.5), identically on a pristine origin/main build, so #7803's reproducer is not runnable as written.

What is established: the zod workload does route through js_object_define_properties (node_modules/zod/src/v4/core/util.ts:316, node_modules/zod/src/v4/classic/errors.ts:28), one of the two sites fixed here, and #7803's symptom — Cannot read properties of undefined (reading 'toString') — is exactly what a stale key string in that loop produces (the property is defined under a garbage name, the later read misses). That makes this a candidate cause of #7803, not a confirmed one. Retest once the corpus links again.

[coderabbitai]

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 60bc76a8-3401-4ad9-9492-bbddee55249b

📥 Commits

Reviewing files that changed from the base of the PR and between d78efca and 844bc11.

📒 Files selected for processing (11)

• changelog.d/7962-root-container-values.md
• crates/perry-runtime/src/gc/roots.rs
• crates/perry-runtime/src/gc/roots/rooted_values.rs
• crates/perry-runtime/src/gc/tests/mod.rs
• crates/perry-runtime/src/gc/tests/rooted_container_values.rs
• crates/perry-runtime/src/object/groupby.rs
• crates/perry-runtime/src/object/object_ops/define_properties.rs
• crates/perry-runtime/src/proxy/own_keys.rs
• gc-handoff/ROOTVEC-NOTES.md
• test-files/test_gap_gc_container_value_rooting.ts
• test-files/test_gap_gc_define_properties_key_rooting.ts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}