test(runtime): dead_owner_side_tables was measuring the globalThis bootstrap - #7987
Merged
proggeramlug merged 2 commits intoAug 12, 2026
Merged
Conversation
…otstrap `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
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe changes isolate lazy realm bootstrap work from GC measurements, add cumulative tracking for block-persistence force marks, and update dead-owner side-table tests with premise checks and shared-block regression coverage. ChangesDead-owner GC test isolation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DeadOwnerSideTableTest
participant GcTestIsolationGuard
participant globalThis
participant FullGC
participant SideTable
DeadOwnerSideTableTest->>GcTestIsolationGuard: bootstrap realm and isolate roots
GcTestIsolationGuard->>globalThis: initialize globalThis
globalThis-->>GcTestIsolationGuard: return pointer
DeadOwnerSideTableTest->>FullGC: run guarded full collection
FullGC-->>DeadOwnerSideTableTest: return force-mark census
DeadOwnerSideTableTest->>SideTable: verify dead-owner entry is pruned
Possibly related issues
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
proggeramlug
marked this pull request as ready for review
August 12, 2026 18:52
proggeramlug
deleted the
test/7975-dead-owner-side-tables-realm-bootstrap
branch
August 12, 2026 18:54
proggeramlug
pushed a commit
that referenced
this pull request
Aug 12, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Aug 12, 2026
`main` is currently RED on the `lint` job's `check_thread_locals.py` step, and has been since #7987 (`23a8aad31`): it added `BLOCK_PERSIST_FORCE_MARKS` to `crates/perry-runtime/src/gc/trace.rs` — a genuinely cold declaration, updated once per fixed-point round, never per object — without re-recording the file's count, so the ratchet reads "2 recorded, 3 found" and fails. Unrelated to the rest of this PR and deliberately its own commit, but the gate is a REQUIRED context, so nothing can go green until it is re-recorded. `--self-test` still passes in all six directions. `_hot_declarations` also moves 163 -> 174; that field is informational (the gate is the raw-block ratchet and the 768-slot capacity), and it had drifted independently of this PR's one new `perry_thread_local!`. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
proggeramlug
added a commit
that referenced
this pull request
Aug 12, 2026
#7994) * fix(gc): give each thread its own memoized prototype addresses (#7988) `ARRAY_PROTO_ADDR` / `OBJECT_PROTO_ADDR` were process-global `AtomicUsize` statics holding RAW ADDRESSES of objects in a thread-local arena, while the realm they name is per-thread: `js_get_global_this` bootstraps `THREAD_GLOBAL_THIS` once per THREAD, but `resolve_prototype_addr` missed only once per PROCESS. The first thread to touch either intrinsic decided the value for every other `perry/thread` agent. Three consequences, all closed structurally by per-thread storage: 1. Wrong identity — `object_prototype_addr_matches` on agent B compared B's objects against A's `Object.prototype`, so B's `Object.prototype[7] = v` never flipped `OBJECT_PROTO_HAS_INDEX` and B's `[1,2,3][7]` read undefined. 2. Unattributed dereference — `heal_prototype_addr` read the cached address's `GcHeader` on every indexed array write, on memory the reading thread had no claim to (A's collector may have swept or moved it; A's arena blocks are dealloc'd at A's exit). 3. Cross-thread root rewrite — `scan_prototype_addr_cache_roots_mut` wrote the collecting thread's to-space address into a cell naming another agent's heap. The recorded objection ("a single relaxed atomic load", "Darwin has no local-exec TLS") is stale: `crate::perry_thread_local!` puts the value's address in this thread's `HotTls` cache, reached with an `mrs` plus two loads that LLVM CSEs — not an out-of-line `_tlv_get_addr` call. Both intrinsics share ONE declaration (an array indexed positionally against `PROTOTYPE_ADDR_BUILTINS`), so a function that consults both pays one resolution, and the root scanner iterates that array itself — "a cell an accessor reads that the collector never rewrites" stays unrepresentable. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * test(7988): make the multi-agent probe discriminating, add notes + changeset The first version of `test_issue_7988_thread_realm_prototype.ts` was VACUOUS: its warm-up (`main[1] = 9`, `main[7]`) resolves neither memoized address, so the spawned agent was simply the first thread to fill the shared cell and the probe answered correctly on the unfixed runtime. Measured, not assumed — it printed the expected string 5/5 against a pre-fix `libperry_runtime.a`. The main thread now pollutes its OWN realm first (`Array.prototype[4]`, `Object.prototype[5]`), which is what forces both addresses to be resolved and memoized before any agent starts, and the agent asserts BOTH directions: * LEAK — `[1,2,3][4]` inside the agent must not see the main realm's prototype index (pre-fix it read "mainArr", dereferencing a GcHeader in an arena the agent does not own); * BLINDNESS — the agent's own `Array.prototype[8] = v` must be visible to the agent's own reads (pre-fix "undefined"). Measured with one compiler and two `libperry_runtime.a` pairs, swapped via PERRY_RUNTIME_DIR: pre-fix 5/5 `match: false` + `allMatch: false`, post-fix 5/5 `match: true` + `allMatch: true`. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * chore(lint): re-record the cold thread_local added by #7987 `main` is currently RED on the `lint` job's `check_thread_locals.py` step, and has been since #7987 (`23a8aad31`): it added `BLOCK_PERSIST_FORCE_MARKS` to `crates/perry-runtime/src/gc/trace.rs` — a genuinely cold declaration, updated once per fixed-point round, never per object — without re-recording the file's count, so the ratchet reads "2 recorded, 3 found" and fails. Unrelated to the rest of this PR and deliberately its own commit, but the gate is a REQUIRED context, so nothing can go green until it is re-recorded. `--self-test` still passes in all six directions. `_hot_declarations` also moves 163 -> 174; that field is informational (the gate is the raw-block ratchet and the 768-slot capacity), and it had drifted independently of this PR's one new `perry_thread_local!`. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * test(7988): gate the multi-agent probe on a stored expected output `perry/thread` has no Node equivalent, so `run_parity_tests.sh` scored the probe as a permanent `parity_fail` (output mismatch against Node's ERR_MODULE_NOT_FOUND) — which is what its four siblings on `main` already do: `parity_known_failures.py` reports all of test_issue_{4449_thread_promise_void, 7302_thread_throws, 7769_thread_class_dispatch, 7981_thread_shape_stamp_parent} as unlisted failures on macOS. The harness already has the right mechanism for a Perry-only API — a stored `test-parity/expected/<name>.txt`, compared against Perry's output and exit code instead of against Node (the `filehandle-thread-*` / `threaded-fd-semantics-*` files use it). Use it, so the probe is a gate that runs rather than a failure that is tolerated. Verified able to fail: the same file compiled against a pre-fix `libperry_runtime.a` produces `spawn agent: mainArr/undefined/undefined/obj1 match: false`, 5/5. Output is deterministic (6/6 identical md5 over repeat runs; every `parallelMap` element is the same string, so worker completion order is not observable). Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * docs(gc-handoff): record the #7988 measurement, sabotage and inventory Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
Closes #7975.
It is not a race
Every test in
gc::tests::dead_owner_side_tablesalready takes the same globalisolation mutex, so no two of them ever overlap. Run on their own, the two
named cases fail 200/200:
origin/main@b847afd1c--test-threads=10 dead_owner_side_tablesorigin/main--test-threads=1 dead_owner_side_tablesorigin/maintest_dead_arguments_object_entry_pruned_on_full_gcALONEorigin/maintest_dead_owner_descriptor_entries_pruned_on_full_gcALONEThey pass only because a sibling ran first.
--test-threads=10makes whichtest goes first nondeterministic, so ~5–7 % of filtered runs put one of them
there.
Mechanism — two facts and a confounder
1. A process-global one-shot decides which THREAD pays for the realm. Both
cases reach an API that resolves the process-global memoized prototype address
(
array::prototype_addr::{ARRAY_PROTO_ADDR, OBJECT_PROTO_ADDR}), and a miss runsthe whole lazy
globalThisbootstrap — ~1.15 MB allocated, ~410 KB live, rootedfor the life of the thread — inside the caller:
The bootstrap is per-thread (
THREAD_GLOBAL_THIS) but the cache isper-process, so once any libtest thread fills it no later thread ever asks for
globalThis.2. Block persistence force-marks the whole block. Arena block reset is
all-or-nothing, so
gc::trace::mark_block_persisting_arena_objects(
BLOCK_PERSIST_WINDOW = 5) marks every object in a block that holds onereachable object.
Confounder. With the bootstrap co-resident, the test's unrooted owner is
force-marked,
dead_owner::PostTraceProbe::owner_is_deadreturns false, and theentry is correctly not pruned — a persisting block cannot recycle the
address, so the entry is not stale. The test then fails on the prune and blames
the prune. Instrumented, in the same case:
Ruled out: conservative stack scan (
PERRY_CONSERVATIVE_STACK_SCAN=offand=onstill fail),force_full_scanfallback (no[gc-scan-fallback]line),root-scanner leakage (
root_scanner_registry_counts()is(0,0,0,0)in both).The change
GcTestIsolationGuard::with_realm_bootstrapped()— runs the bootstrap insidethe isolation lock but before
ScopedRootScannerRegistryGuardtakes thethread's scanners and before
reset_global_roots(), so the realm graph isoutside the measured window and cannot keep the test's block alive.
gc::trace::block_persist_force_mark_count()— an always-on, O(1)-per-passthread-local census of block-persistence force-marks, recorded by both the
whole-cycle pass and the budgeted
BlockPersistCycleStatearm. Same rationaleas
gc::scan_fallback(gc: Track A2's premise is wrong — six force_full_scan() sites, four automatic, and PERRY_CONSERVATIVE_STACK_SCAN=full fails 134 runtime tests #7148): the mark is cleared again by the sweep, sobefore this there was no observable a test could use to state its own premise.
full_gc_with_no_block_persistence()fails as a premise rather thanletting the subject assertion mis-name it. Sabotage-checked: reverting only the
guard turns both cases from the old misleading
"…entry must be pruned" into
"test premise: block persistence force-marked objects during this collection…
left: 4083, right: 0".
test_block_persistence_census_moves_when_a_block_has_a_live_tenant()plantsthe confounder's exact shape (one rooted object + one unrooted owner in one
block, no realm involved) and asserts both halves — the census moves, and
the force-marked owner's side-table entry correctly survives. Without it every
"premise held" verdict would be vacuous.
No assertion was weakened.
Verification
--test-threads=10 dead_owner_side_tablesorigin/maincargo fmt --all -- --checkandscripts/check_file_size.shclean.Not fixed here — a product defect this was sitting on
ARRAY_PROTO_ADDR/OBJECT_PROTO_ADDRare process-globalAtomicUsizeholdingraw addresses of objects in a thread-local arena, while the
globalThisrealm they name is per-thread. The first thread to touch
Array.prototype/Object.prototypedecides the value for every other agent in the process. Filedseparately; same family as #7955.
Summary by CodeRabbit
Bug Fixes
Tests