From 777e6edc16a388ec18186f0d68fe86b8771708c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 20:51:12 +0200 Subject: [PATCH 1/2] test(runtime): dead_owner_side_tables was measuring the globalThis bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_dead_arguments_object_entry_pruned_on_full_gc` and `test_dead_owner_descriptor_entries_pruned_on_full_gc` fail 200/200 when they are the first test in the process and 0/200 when any sibling runs first — 10 in 200 `--test-threads=10` runs of the module. It is not a race: every test in the module already takes the same global isolation mutex. Both reach the runtime through an API that resolves the process-global memoized `Object.prototype` address (`array::prototype_addr`), and a miss runs the whole lazy `globalThis` bootstrap — ~1.15 MB allocated, ~410 KB live — inside the caller. Arena block reset is all-or-nothing, so `mark_block_persisting_arena_objects` then force-marks every object in the block, the test's unrooted owner included; the death prune correctly declines to drop an entry whose address cannot be recycled. `GcTestIsolationGuard::with_realm_bootstrapped()` runs the bootstrap inside the isolation lock but before the scanner registry is taken and the roots are reset, so the realm graph is outside the measured window. The mark is cleared again by the sweep, so a test had no way to state that premise. `gc::trace::block_persist_force_mark_count()` is an always-on, O(1)-per-pass census of block-persistence force-marks (same rationale as `gc::scan_fallback`, #7148), recorded by both the whole-cycle and the budgeted arm. `full_gc_with_no_block_persistence()` fails as a premise instead of letting the subject assertion mis-name it, and `test_block_persistence_census_moves_when_a_block_has_a_live_tenant` plants the confounder's exact shape so the census is sabotage-tested rather than merely read. Closes #7975 --- crates/perry-runtime/src/gc/cycle.rs | 1 + .../src/gc/tests/dead_owner_side_tables.rs | 115 +++++++++++++++++- crates/perry-runtime/src/gc/tests/support.rs | 57 +++++++++ crates/perry-runtime/src/gc/trace.rs | 41 +++++++ 4 files changed, 210 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index 2ca0d073f7..4bc22252fa 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -165,6 +165,7 @@ impl BlockPersistCycleState { } self.stats.marked_objects = self.stats.marked_objects.saturating_add(self.newly_marked); + super::trace::note_block_persist_force_marks(self.newly_marked); if self.newly_marked == 0 { self.subphase = BlockPersistSubphase::Done; return true; diff --git a/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs b/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs index e8f53d0a1a..0edf805647 100644 --- a/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs +++ b/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs @@ -6,6 +6,25 @@ //! and assert the entry is gone — plus the two safety inverses (a LIVE owner's //! entries survive; a TENURED owner's entries survive a MINOR trace, which //! never marks the old generation and therefore proves nothing about it). +//! +//! ★ #7975 — THE PREMISE EVERY "IT IS PRUNED" CASE HERE DEPENDS ON. Arena +//! block reset is all-or-nothing, so `gc::trace` force-MARKS every object in a +//! block that still holds one reachable object +//! (`mark_block_persisting_arena_objects`, `BLOCK_PERSIST_WINDOW`). An owner +//! sharing its block with ANY live object is therefore not dead, and the prune +//! *correctly* keeps its entry — a persisting block cannot recycle the +//! address, so the entry is not stale. A case that lets a live tenant into its +//! block does not test the prune; it fails on it. +//! +//! The tenant that actually turned up is the lazy `globalThis` realm bootstrap +//! (~1.15 MB allocated, ~410 KB live), reached through the **process-global** +//! memoized `Object.prototype` address that `set_property_attrs` and +//! `js_object_set_field_by_name` resolve. It misses once per PROCESS, so which +//! libtest thread pays for it is a scheduling accident: two cases here failed +//! 200/200 alone and 0/200 behind a sibling. Use +//! [`GcTestIsolationGuard::with_realm_bootstrapped`] in any case that reaches a +//! realm-resolving API, and collect through [`full_gc_with_no_block_persistence`] +//! so the premise fails as a premise instead of as the subject. use super::super::*; use super::support::*; @@ -70,6 +89,39 @@ unsafe fn alloc_malloc_test_object() -> *mut crate::object::ObjectHeader { obj } +/// PREMISE GUARD for the "dead owner ⇒ entry pruned" cases (#7975). +/// +/// "The entry is gone" only says something about the death prune when the owner +/// was genuinely DEAD. Arena block reset is all-or-nothing, so +/// `gc::trace::mark_block_persisting_arena_objects` force-MARKS every object in +/// a block that still holds one reachable object — and a force-marked owner is +/// *correctly* left registered, because a block that persists cannot recycle +/// its address, so the entry is not stale. A case whose block picked up an +/// unrelated live tenant therefore fails on the prune assertion and blames the +/// prune for something the prune got right. +/// +/// The mark itself is cleared again by the sweep, so nothing about it survives +/// the collection for a test to read. `gc::trace`'s force-mark census is the +/// one quantity that does, and it has exactly one satisfying path: it moves iff +/// some block in the persist window held a reachable object. +/// +/// Wrap the collection so the premise fails as a premise, before the subject +/// assertion gets a chance to mis-name it. +fn full_gc_with_no_block_persistence() { + let force_marks_before = crate::gc::block_persist_force_mark_count(); + full_gc(); + assert_eq!( + crate::gc::block_persist_force_mark_count(), + force_marks_before, + "test premise: block persistence force-marked objects during this \ + collection, so an unrooted owner in one of those blocks is NOT dead \ + and the prune correctly declines to drop its entry. Something \ + reachable shares the owner's arena block — the lazy `globalThis` \ + realm bootstrap is the one that turned up (#7975); see \ + `GcTestIsolationGuard::with_realm_bootstrapped`" + ); +} + fn util_types_is_map_iterator(addr: usize) -> bool { crate::object::js_util_types_is_map_iterator(f64::from_bits(ptr_bits(addr))).to_bits() == crate::value::TAG_TRUE @@ -259,7 +311,10 @@ fn test_array_named_dead_set_iterator_marker_does_not_brand_exact_eden_reuse() { #[test] fn test_dead_owner_descriptor_entries_pruned_on_full_gc() { - let _guard = GcTestIsolationGuard::new(); + // `set_property_attrs` below resolves the process-global memoized + // `Object.prototype` address, and a MISS runs the whole lazy `globalThis` + // bootstrap inside this test — see `with_realm_bootstrapped` (#7975). + let _guard = GcTestIsolationGuard::with_realm_bootstrapped(); let (obj, _) = unsafe { alloc_nursery_test_object(0) }; let addr = obj as usize; crate::object::set_property_attrs( @@ -275,7 +330,7 @@ fn test_dead_owner_descriptor_entries_pruned_on_full_gc() { assert!(crate::object::get_property_attrs(addr, "frozenKey").is_some()); // No roots: the owner is dead at the full trace. - full_gc(); + full_gc_with_no_block_persistence(); assert!( crate::object::get_property_attrs(addr, "frozenKey").is_none(), @@ -393,6 +448,54 @@ fn test_dead_owner_symbol_property_entries_pruned_on_full_gc() { ); } +/// SABOTAGE CHECK for [`full_gc_with_no_block_persistence`] (#7975). Every +/// "the premise held" verdict it gives is vacuous unless the census it reads +/// can move — CLAUDE.md's fourth way a gate cannot fail. +/// +/// Plant the exact shape the #7975 confounder had, minus the realm: one ROOTED +/// object and one unrooted owner in the same arena block. Nothing here resolves +/// `globalThis` (`closure_set_dynamic_prop` does not), so the planted tenant is +/// the only reachable object in the persist window and the census has exactly +/// one satisfying path. +/// +/// Both halves are asserted: the census moves, AND the semantic consequence +/// that makes the premise guard necessary in the first place — a force-marked +/// owner is not dead, so the death prune correctly leaves its entry alone. +#[test] +fn test_block_persistence_census_moves_when_a_block_has_a_live_tenant() { + let _guard = CopyingNurseryTestGuard::new(1); + crate::arena::arena_reset_all_blocks_to_zero(); + + let (live, _) = unsafe { alloc_nursery_test_object(0) }; + js_shadow_slot_set(0, ptr_bits(live as usize)); + + let ptr = crate::arena::arena_alloc_gc( + std::mem::size_of::(), + 8, + GC_TYPE_CLOSURE, + ); + unsafe { init_test_closure(ptr) }; + let dead = ptr as usize; + crate::closure::closure_set_dynamic_prop(dead, "memo", 42.0); + assert!(crate::closure::closure_get_own_dynamic_prop(dead, "memo").is_some()); + + let force_marks_before = crate::gc::block_persist_force_mark_count(); + full_gc(); + + assert!( + crate::gc::block_persist_force_mark_count() > force_marks_before, + "block persistence must force-mark the unrooted neighbour of a rooted \ + object — a census that cannot move makes every \ + `full_gc_with_no_block_persistence` verdict in this file vacuous" + ); + assert!( + crate::closure::closure_get_own_dynamic_prop(dead, "memo").is_some(), + "and the consequence the premise guard exists to name: a force-marked \ + owner is NOT dead, so its side-table entry correctly survives — a case \ + that hits this shape is failing on its premise, not on the prune" + ); +} + #[test] fn test_dead_closure_side_table_entries_pruned_on_full_gc() { let _guard = GcTestIsolationGuard::new(); @@ -477,13 +580,17 @@ fn test_object_dead_payload_arm_clears_keys_index() { #[test] fn test_dead_arguments_object_entry_pruned_on_full_gc() { - let _guard = GcTestIsolationGuard::new(); + // `js_arguments_object_alloc` reaches `js_object_set_field_by_name`, which + // resolves the process-global memoized `Object.prototype` address; a MISS + // runs the lazy `globalThis` bootstrap inside this test — see + // `with_realm_bootstrapped` (#7975). + let _guard = GcTestIsolationGuard::with_realm_bootstrapped(); let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); let obj = crate::object::js_arguments_object_alloc(undefined, undefined, 0); let addr = obj as usize; assert!(crate::object::test_arguments_object_registered(addr)); - full_gc(); + full_gc_with_no_block_persistence(); assert!( !crate::object::test_arguments_object_registered(addr), diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index 2e2ad07efe..9b022a22cd 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -324,7 +324,56 @@ pub(super) struct GcTestIsolationGuard { impl GcTestIsolationGuard { pub(super) fn new() -> Self { + Self::build(RealmBootstrap::LeaveLazy) + } + + /// [`GcTestIsolationGuard::new`] plus: run the lazy `globalThis` realm + /// bootstrap BEFORE the measured window opens (#7975). + /// + /// REQUIRED by any test that asserts an unrooted object is DEAD at a + /// collection and reaches the runtime through an API that can resolve the + /// realm. Two facts compose into a scheduling-dependent false failure: + /// + /// 1. `object::set_property_attrs` and `js_object_set_field_by_name` both + /// consult the **process-global** memoized `Object.prototype` address + /// (`array::prototype_addr`), and a MISS runs the whole lazy + /// `globalThis` bootstrap — ~1.15 MB allocated, ~410 KB of it live, + /// rooted for the life of the thread — inside the caller. The cache + /// misses exactly once per PROCESS, so WHICH libtest thread pays is a + /// scheduling accident. + /// 2. Arena block reset is all-or-nothing, so + /// `gc::trace::mark_block_persisting_arena_objects` force-MARKS every + /// object in a block that holds one reachable object. A test owner that + /// shares its block with a freshly-bootstrapped realm is therefore NOT + /// dead — and the death prune *correctly* declines to drop its + /// side-table entry, because a block that persists cannot recycle the + /// owner's address. + /// + /// Measured on `origin/main` before this existed: the two affected cases in + /// `dead_owner_side_tables` failed **200/200** runs when scheduled first and + /// **0/200** when any sibling resolved the cache first — 10/200 at + /// `--test-threads=10` over the whole module (#7975). + /// + /// Bootstrapping here — inside the isolation lock, but before + /// [`ScopedRootScannerRegistryGuard`] takes the thread's scanners and + /// before `reset_global_roots` — puts the realm graph OUTSIDE the window: + /// the guard then un-roots it, so it cannot keep the test's own block + /// alive. + pub(super) fn with_realm_bootstrapped() -> Self { + Self::build(RealmBootstrap::RunItNow) + } + + fn build(realm: RealmBootstrap) -> Self { let lock = copying_nursery_isolation_lock(); + if matches!(realm, RealmBootstrap::RunItNow) { + let global = crate::object::js_get_global_this(); + assert!( + crate::value::JSValue::from_bits(global.to_bits()).is_pointer(), + "the realm bootstrap must have produced a singleton — otherwise \ + it did not run here, and the confounder this guard exists to \ + move out of the window is still inside it" + ); + } let scanner_guard = ScopedRootScannerRegistryGuard::new(); reset_copying_nursery_runtime_test_state(); reset_shadow_stack(); @@ -337,6 +386,14 @@ impl GcTestIsolationGuard { } } +/// Whether [`GcTestIsolationGuard::build`] forces the lazy `globalThis` +/// bootstrap before opening the window. See +/// [`GcTestIsolationGuard::with_realm_bootstrapped`]. +enum RealmBootstrap { + LeaveLazy, + RunItNow, +} + impl Drop for GcTestIsolationGuard { fn drop(&mut self) { reset_copying_nursery_runtime_test_state(); diff --git a/crates/perry-runtime/src/gc/trace.rs b/crates/perry-runtime/src/gc/trace.rs index aa7ffb450a..13ae67f2a6 100644 --- a/crates/perry-runtime/src/gc/trace.rs +++ b/crates/perry-runtime/src/gc/trace.rs @@ -865,6 +865,46 @@ pub(super) fn trace_marked_objects(valid_ptrs: &ValidPointerSet) { /// old-block neighbors as new block-persist candidates. pub(super) const BLOCK_PERSIST_WINDOW: usize = 5; +thread_local! { + /// Objects this thread's block-persistence pass has FORCE-MARKED since + /// process start — i.e. kept alive for no reason other than sharing a + /// block with something reachable. + /// + /// The census exists for the same reason `gc::scan_fallback`'s does + /// (#7148): force-marking is a correctness measure with a *semantic* side + /// effect — an unrooted object in a persisting block is not dead, so every + /// death-keyed consumer (the `dead_owner` side-table prunes, the Map/Set + /// registry sweeps) legitimately declines to act on it. A test that + /// asserts "this unrooted owner is dead" is therefore asserting something + /// about the tenancy of its arena block, and without this counter it has no + /// way to say so: the mark is cleared again by the sweep, so nothing + /// observable survives the collection. #7975 is what that costs — two + /// `dead_owner_side_tables` cases failed 200/200 when the lazy `globalThis` + /// bootstrap landed in their block, and the failure named the prune. + /// + /// Updated once per pass (per fixed-point round), never per object. + static BLOCK_PERSIST_FORCE_MARKS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Running count of [`BLOCK_PERSIST_FORCE_MARKS`] on this thread. Compare +/// across a collection: an unchanged value means block persistence force-marked +/// nothing, so an object left unmarked by that collection was genuinely +/// unreachable rather than merely a neighbour of something reachable. +pub(crate) fn block_persist_force_mark_count() -> u64 { + BLOCK_PERSIST_FORCE_MARKS.with(std::cell::Cell::get) +} + +/// Record one pass's force-mark count. BOTH arms call this: the whole-cycle +/// pass below and the budgeted `BlockPersistCycleState` in `gc::cycle`, which +/// force-marks through its own loop. A census that only counted one of them +/// would read zero on exactly the cycles that are hardest to reason about. +pub(super) fn note_block_persist_force_marks(marked: usize) { + if marked != 0 { + BLOCK_PERSIST_FORCE_MARKS + .with(|count| count.set(count.get().saturating_add(marked as u64))); + } +} + pub(super) fn mark_block_persisting_arena_objects( valid_ptrs: &ValidPointerSet, ) -> BlockPersistTraceStats { @@ -940,6 +980,7 @@ pub(super) fn mark_block_persisting_arena_objects( }, ); stats.marked_objects += newly_marked; + note_block_persist_force_marks(newly_marked); if newly_marked == 0 { break; From 423ad1ec4bd38d7744549aa282283cab8d0ca4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 20:52:19 +0200 Subject: [PATCH 2/2] docs: changelog fragment for #7987 --- ...-dead-owner-side-tables-realm-bootstrap.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 changelog.d/7987-dead-owner-side-tables-realm-bootstrap.md diff --git a/changelog.d/7987-dead-owner-side-tables-realm-bootstrap.md b/changelog.d/7987-dead-owner-side-tables-realm-bootstrap.md new file mode 100644 index 0000000000..e1b0c3d3d5 --- /dev/null +++ b/changelog.d/7987-dead-owner-side-tables-realm-bootstrap.md @@ -0,0 +1,48 @@ +### `dead_owner_side_tables` was measuring the `globalThis` bootstrap, not the death prune + +`test_dead_arguments_object_entry_pruned_on_full_gc` and +`test_dead_owner_descriptor_entries_pruned_on_full_gc` (#7975) failed 10 of 200 +`--test-threads=10` runs of their module. Filed as a self-race — it is not one. +Every test in the module already takes the same global isolation mutex, and run +**alone** both cases fail **200/200**; they pass only when a sibling ran first. + +Two facts compose. Both cases reach an API that resolves the **process-global** +memoized `Object.prototype` address (`array::prototype_addr`) — +`set_property_attrs` for one, `js_object_set_field_by_name` for the other — and a +MISS runs the whole lazy `globalThis` bootstrap, ~1.15 MB allocated and ~410 KB +of it live-and-rooted, *inside the caller*. The cache misses exactly once per +PROCESS, so which libtest thread pays is a scheduling accident. And arena block +reset is all-or-nothing, so `gc::trace::mark_block_persisting_arena_objects` +force-MARKS every object in a block that holds one reachable object — the test's +own unrooted owner included. `dead_owner::PostTraceProbe::owner_is_dead` then +correctly reports "not dead" and the prune correctly keeps the entry: a block +that persists cannot recycle the owner's address, so the entry is not stale. The +test failed on the prune and named the prune. + +`GcTestIsolationGuard::with_realm_bootstrapped()` runs the bootstrap inside the +isolation lock but before `ScopedRootScannerRegistryGuard` takes the thread's +scanners and before `reset_global_roots()`, putting the realm graph outside the +measured window. + +The force-mark is cleared again by the sweep, so nothing about it survived a +collection for a test to read — the premise was unstateable. +`gc::trace::block_persist_force_mark_count()` is now an always-on, +O(1)-per-pass thread-local census of block-persistence force-marks, recorded by +**both** the whole-cycle pass and the budgeted `BlockPersistCycleState` arm (a +census that counted one arm would read zero on exactly the cycles that are +hardest to reason about). Same rationale as `gc::scan_fallback` (#7148). +`full_gc_with_no_block_persistence()` now fails as a *premise* instead of +letting the subject assertion mis-name it, and +`test_block_persistence_census_moves_when_a_block_has_a_live_tenant` plants the +confounder's exact shape — one rooted object and one unrooted owner in one +block, no realm involved — asserting both that the census moves and that the +force-marked owner's side-table entry correctly survives. Without that the +"premise held" verdicts would be vacuous. + +No assertion was weakened. Verified: fixed binary 400/400 green on +`--test-threads=10 dead_owner_side_tables`, 200/200 on each case run alone, +200/200 on the new census case, and 300/300 on the full suite at default +parallelism — against `origin/main`'s 10/200, 200/200-failing alone, and +300/300 full-suite green (unchanged). Sabotage check: reverting only the guard +turns both cases into `test premise: block persistence force-marked objects +during this collection … left: 4083, right: 0`.