Skip to content

test(runtime): fix the last two suite flakes — shared prototype-address cache (#7955), wall-clock pause budget (#7956) #7974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 1 commit into from
Aug 12, 2026
Merged

test(runtime): fix the last two suite flakes — shared prototype-address cache (#7955), wall-clock pause budget (#7956) #7974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions changelog.d/7974-runtime-suite-flakes-7955-7956.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
### test(runtime): the last two `perry-runtime` suite flakes — a shared prototype-address cache, and a wall-clock pause assertion

Closes #7955 and #7956, the two members of the #7946 flaky set that #7954 named
rather than papered over. Both are test-side; no shipped behaviour changes.

**#7955 — `gc::tests::runtime_roots::prototype_addr_cache`.** All five cases in
that file drove the SHIPPED `ARRAY_PROTO_ADDR` / `OBJECT_PROTO_ADDR` statics, so
their real racing partner turned out to be *each other*: libtest schedules a
module's tests concurrently, and
`prototype_addr_cache_scanner_leaves_the_unset_sentinel_alone` stores
`usize::MAX` into both cells while a sibling has a synthetic forwarded stub
planted there. Co-scheduling them deliberately reproduces it at **28 failures in
400 runs** (`--test-threads=8 prototype_addr`), every one of them
`prototype_addr_cache_is_rewritten_by_the_collector` reading back `usize::MAX`.
The 1-in-100 full-suite failure #7955 reports is the same mechanism with the
partner arriving by luck. #7954's save/restore guard made it worse rather than
better — restoring the value read at test entry stamps a stale address over
whatever another thread resolved meanwhile.

Both #6981 defences are algebra over an `&AtomicUsize` (`heal_prototype_addr`
already took one), so `memoized_prototype_addr` and the new
`rewrite_prototype_addr_slot` take the cell as an argument and every mutating
case now owns its cell. Nothing hands out a writable reference to the realm's
real intrinsic cells any more.

What that decomposition would otherwise lose — *the collector rewrites every
cell an accessor reads*, the #6981 invariant the old test proved by mutation —
is not recovered by another test but by construction: the accessors, the
`globalThis` builtin each resolves, and the root scanner's visit list are now
one table (`PROTOTYPE_ADDR_CACHES`), so a cell some accessor reads and the
collector never rewrites is unrepresentable. One new read-only case
(`the_shipped_cells_are_the_ones_the_scanner_visits`) pins the table itself —
two DISTINCT cells, each paired with its builtin — and writes nothing, so it
cannot be raced either.

`per_test_global!` was deliberately NOT retried. The obstacle #7955 records is
real (a fresh `PerThread` cell starts unresolved, so the first read on every
libtest thread runs the allocating `globalThis` bootstrap, from paths like
`note_array_index_write` that sit on the array element-write path), but the
decisive objection is different: the accessors are hot-path code whose design
note requires "a single relaxed atomic load", Darwin has no local-exec TLS, and
per-thread storage in a test build only would give the test build a
representation the product does not have. The cache being a process-global
holding a raw address into a THREAD-LOCAL arena is still a real cross-agent
hazard under `perry/thread`; that is a shipped-representation change and wants
its own issue.

The cache moved to `crates/perry-runtime/src/array/prototype_addr.rs` —
`indexing.rs` reached 2012 lines and `scripts/check_file_size.sh` caps at 2000.

**#7956 — `gc::tests::telemetry_verifier`.** `verify_ordinary_pause_budget`
asserted `elapsed_pause_us <= soft_pause_target_us` per step. That is not a
mis-tuned bound, it is the wrong instrument, on three independent readings of
the source: `GcPauseBudget` documents itself as "hard work-unit limit plus a
**soft pause target for telemetry**"; `GcCycle::step` runs a phase for
`budget.work_units` and measures elapsed *afterwards*, so no code path can make
a step honour `pause_us`; and these fixtures drive with
`js_gc_step_work_units(1, …)`, the smallest step that exists, so a 4.9 ms work
unit leaves the pacer no smaller choice. The second arm was a tautology of the
first — `within_soft_pause_target` is computed as `elapsed_us <= target` — two
assertions carrying one bit.

The verifier now checks what the pacer actually controls, and checks more than
before: at least one step counted in ordinary pause stats (the old verifier had
no subject-was-live check at all), every included step bounded in work units
with `applied_work_units <= configured_work_budget` and never labelled
unbounded, `within_soft_pause_target` coherent with the numbers printed beside
it, and `pause_budget.max_observed_step_pause_us` equal to the max over the
steps in its own event. Elapsed microseconds stay in the trace and in every
error message as a diagnostic. Not moved behind an env opt-in: a timing arm no
CI job runs is a gate that cannot fail.

Six sabotage cases replace the single `verifier_rejects_over_budget_ordinary_step`,
one per rejection path, plus `verifier_accepts_a_slow_but_coherent_ordinary_step`
which encodes #7956's own failing numbers (4936 us against a 2000 us target) as
a case that must now PASS, so the decision cannot be reverted silently.

**Verification.** Pristine baseline preserved as a binary so each A/B swaps the
artifact, not the tree. M1 mini, load 70–98 throughout.

| arm | runs | failed |
|---|---|---|
| #7955 repro, pristine, `--test-threads=8 prototype_addr` | 400 | **28** |
| #7955, fixed | 400 | **0** |
| #7956 repro, pristine, `telemetry_verifier` under 16 spinners | 150 | **3** |
| #7956, fixed | 150 | **0** |
| full suite, pristine, default parallelism | 300 | 0 |
| full suite, fixed, default parallelism | 300 | **0** |

The two full-suite arms are a regression guard, not evidence: at a 1–2 % per-run
rate, 300 runs of that configuration is consistent with either state of the
code, which is why both targeted reproductions exist.
170 changes: 9 additions & 161 deletions crates/perry-runtime/src/array/indexing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use super::header::{array_numeric_layout, NumericArrayLayout};
use super::*;
use std::ptr;
use std::sync::atomic::{AtomicBool, AtomicU8, AtomicUsize, Ordering};
use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};

const MAX_DENSE_ARRAY_GROW_LENGTH: u32 = 1_000_000;

Expand Down Expand Up @@ -41,41 +41,21 @@ fn throw_array_not_extensible_add(index: u32) -> ! {
));
}

/// Lazily-memoized address of the `Array.prototype` array, and a sticky flag
/// recording whether anyone has installed an indexed property on it. An
/// out-of-bounds element read on an ordinary array must fall through to
/// `Array.prototype[index]` (ECMA-262 OrdinaryGet → prototype chain), but in
/// real code nobody adds numeric indices to `Array.prototype`, so the hot OOB
/// path stays a single relaxed atomic load until the (rare) write flips the
/// flag. `usize::MAX` marks the address as not-yet-computed.
///
/// ***THIS IS A RAW ADDRESS OF A MOVABLE OBJECT*** (#6981). `Array.prototype`
/// relocates two different ways, and BOTH leave this cache pointing at a
/// `GC_FLAG_FORWARDED` stub while every reader resolves its own receiver
/// through `clean_arr_ptr` (which follows forwarding):
///
/// 1. `js_array_grow` — an indexed write past the dense capacity
/// (`Array.prototype[300] = v`) reallocates and forwards the old head;
/// 2. the copying young-gen minor — it evacuates the prototype and forwards.
///
/// A stale cache is not merely a wrong value: `array_oob_prototype_get`'s
/// self-recursion guard is `proto != receiver`, and after a move those are two
/// different addresses **for the same object**, so the guard stops firing and
/// `js_array_get_f64` ⇄ `array_oob_prototype_get` recurse until the stack guard
/// page (SIGSEGV, "excessive recursion"). Hence the two defences below:
/// `array_prototype_addr` resolves the forwarding chain and self-heals, and
/// `scan_prototype_addr_cache_roots_mut` lets the collector rewrite the slot so
/// the address stays live even once the from-space stub is recycled.
static ARRAY_PROTO_ADDR: AtomicUsize = AtomicUsize::new(usize::MAX);
/// Sticky flag: someone installed an indexed property on `Array.prototype`.
/// An out-of-bounds element read on an ordinary array must fall through to
/// `Array.prototype[index]` (ECMA-262 OrdinaryGet -> prototype chain), but in
/// real code nobody adds numeric indices there, so the hot OOB path stays a
/// single relaxed atomic load until the (rare) write flips this. The address
/// it is compared against lives in [`super::prototype_addr`], which also owns
/// the GC hazard that address carries (#6981).
static ARRAY_PROTO_HAS_INDEX: AtomicBool = AtomicBool::new(false);

/// Same idea for `Object.prototype`: a numeric index installed there
/// (`Object.prototype[2] = 2`, or a defineProperty accessor) shows through
/// array HOLES and OOB reads (chain: arr → Array.prototype →
/// array HOLES and OOB reads (chain: arr -> Array.prototype ->
/// Object.prototype; test262 concat/S15.4.4.4_A3_T3). Flipped by the object
/// index-write/defineProperty hooks; consulted by the typed-feedback guards
/// and the hole/OOB read fallbacks.
static OBJECT_PROTO_ADDR: AtomicUsize = AtomicUsize::new(usize::MAX);
static OBJECT_PROTO_HAS_INDEX: AtomicBool = AtomicBool::new(false);

/// Sticky summary of the process-wide conditions that invalidate codegen's
Expand All @@ -90,103 +70,6 @@ pub(crate) fn invalidate_array_index_fast_path() {
PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.store(1, Ordering::Relaxed);
}

/// GC root scanner for the two memoized prototype addresses (#6981).
///
/// `ARRAY_PROTO_ADDR` / `OBJECT_PROTO_ADDR` hold raw addresses of movable
/// objects, so a relocating cycle must REWRITE them exactly like the other
/// address-holding side tables (`CLASS_PROTOTYPE_OBJECTS`,
/// `TYPED_ARRAY_VIEW_META`, …). Forwarding-chain healing alone is not
/// sufficient: once the from-space stub is swept and its block recycled the
/// `GC_FLAG_FORWARDED` bit is gone, and the cache would then name an unrelated
/// live object. Both intrinsics are reachable from `globalThis`, so the marking
/// half of this visit is redundant; the rewriting half is the point.
pub fn scan_prototype_addr_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
for cache in [&ARRAY_PROTO_ADDR, &OBJECT_PROTO_ADDR] {
let cached = cache.load(Ordering::Relaxed);
if cached == usize::MAX || cached == 0 {
continue;
}
let mut addr = cached;
if visitor.visit_usize_slot(&mut addr) {
// GC_STORE_AUDIT(ROOT): this IS the collector's root-rewrite of a
// registered side-table slot, running inside a root scan with the
// mutator stopped. `visit_usize_slot` returns true only when it
// relocated the object, and the value written is the visitor's own
// to-space address — barriering it would push an edge into the
// remembered set that this very cycle is rebuilding.
cache.store(addr, Ordering::Relaxed);
}
}
}

/// Test-only handles on the two memoized prototype addresses, so the #6981
/// regression tests can install a synthetic forwarded stub without touching the
/// realm's real intrinsics.
#[cfg(test)]
pub(crate) fn test_array_proto_addr_cache() -> &'static AtomicUsize {
&ARRAY_PROTO_ADDR
}

#[cfg(test)]
pub(crate) fn test_object_proto_addr_cache() -> &'static AtomicUsize {
&OBJECT_PROTO_ADDR
}

/// Re-read a memoized prototype address through the GC forwarding chain and
/// write the healed address back, so every caller compares (and dereferences)
/// the object's CURRENT location. See the `ARRAY_PROTO_ADDR` doc for why an
/// unresolved cache is a hang, not just a wrong answer (#6981).
///
/// `note_array_index_write` calls this on every indexed array write until the
/// prototype is polluted, so the not-forwarded case must stay call-free: the
/// `try_read_gc_header` probe is `#[inline(always)]` and reduces to two range
/// compares plus one load of a `gc_flags` byte at a fixed, permanently-hot
/// address. It also classifies the address band before dereferencing, so the
/// not-yet-resolved sentinel (`usize::MAX`) and any non-heap value fall
/// straight through.
#[inline]
fn heal_prototype_addr(cache: &AtomicUsize, cached: usize) -> usize {
let forwarded = unsafe {
crate::value::addr_class::try_read_gc_header(cached)
.is_some_and(|header| header.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0)
};
if !forwarded {
return cached;
}
let resolved = crate::value::resolve_forwarding(cached);
if resolved != cached {
cache.store(resolved, Ordering::Relaxed);
}
resolved
}

pub(crate) fn object_prototype_addr() -> usize {
let cached = OBJECT_PROTO_ADDR.load(Ordering::Relaxed);
if cached != usize::MAX {
return heal_prototype_addr(&OBJECT_PROTO_ADDR, cached);
}
let ctor = crate::object::js_get_global_this_builtin_value(b"Object".as_ptr(), 6);
let ctor_value = crate::value::JSValue::from_bits(ctor.to_bits());
let addr = if ctor_value.is_pointer() {
let ctor_ptr = ctor_value.as_pointer::<u8>() as usize;
let proto = crate::closure::closure_get_dynamic_prop(ctor_ptr, "prototype");
let proto_value = crate::value::JSValue::from_bits(proto.to_bits());
if proto_value.is_pointer() {
proto_value.as_pointer::<u8>() as usize
} else {
0
}
} else {
0
};
// Cache only a successful resolution — an early call (before globalThis
// init) must retry later rather than pinning 0.
if addr != 0 {
OBJECT_PROTO_ADDR.store(addr, Ordering::Relaxed);
}
addr
}

/// Record (if `obj` is the canonical `Object.prototype`) that it now carries
/// an indexed property. Called from the object index-write / numeric
/// defineProperty paths; cheap (relaxed loads + compare).
Expand All @@ -203,12 +86,6 @@ pub(crate) fn object_prototype_has_index_flag() -> bool {
OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed)
}

/// `true` when `addr` is the canonical `Object.prototype` (cheap: cached
/// atomic + compare; lazily computes the address on first use).
pub(crate) fn object_prototype_addr_matches(addr: usize) -> bool {
addr != 0 && addr == object_prototype_addr()
}

/// Sticky flag: user code replaced or deleted `Array.prototype[Symbol.iterator]`.
/// `js_get_iterator`'s array short-circuit assumes the builtin values iterator;
/// once this flips, GetIterator on an array must consult the (patched) method
Expand Down Expand Up @@ -256,35 +133,6 @@ pub(crate) fn array_proto_iterator_modified() -> bool {
ARRAY_PROTO_ITERATOR_MODIFIED.load(Ordering::Relaxed)
}

pub(crate) fn array_prototype_addr() -> usize {
let cached = ARRAY_PROTO_ADDR.load(Ordering::Relaxed);
if cached != usize::MAX {
return heal_prototype_addr(&ARRAY_PROTO_ADDR, cached);
}
let ctor = crate::object::js_get_global_this_builtin_value(b"Array".as_ptr(), 5);
let ctor_value = crate::value::JSValue::from_bits(ctor.to_bits());
let addr = if ctor_value.is_pointer() {
let ctor_ptr = ctor_value.as_pointer::<u8>() as usize;
let proto = crate::closure::closure_get_dynamic_prop(ctor_ptr, "prototype");
let proto_value = crate::value::JSValue::from_bits(proto.to_bits());
if proto_value.is_pointer() {
proto_value.as_pointer::<u8>() as usize
} else {
0
}
} else {
0
};
// Don't poison the cache with 0: during runtime init the global `Array`
// constructor may not be materialized yet (symbol writes on other builtin
// prototypes call into here via `note_array_proto_iterator_write`).
// Re-derive until it resolves.
if addr != 0 {
ARRAY_PROTO_ADDR.store(addr, Ordering::Relaxed);
}
addr
}

/// Record (if `arr` is `Array.prototype`) that the prototype now carries an
/// indexed property, so subsequent out-of-bounds reads consult it. Called from
/// the array element-write paths; cheap (two relaxed atomic loads + compare).
Expand Down
24 changes: 15 additions & 9 deletions crates/perry-runtime/src/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod iter_methods;
mod iter_object;
mod iterator;
mod jsvalue_api;
mod prototype_addr;
mod push_pop;
mod reduce_right;
mod search;
Expand Down Expand Up @@ -102,13 +103,14 @@ pub use self::immutable::{
js_array_to_sorted_default, js_array_to_sorted_with_comparator, js_array_to_spliced,
js_array_with, js_arraylike_copy_within,
};
#[cfg(test)]
pub(crate) use self::indexing::test_keys_array_slot_fallbacks;
pub(crate) use self::indexing::{
array_has_own_index, array_iteration_is_exotic, array_proto_iterator_modified,
array_prototype_addr, array_prototype_has_index_flag, array_spec_get, array_spec_has_index,
array_prototype_has_index_flag, array_spec_get, array_spec_has_index,
invalidate_array_index_fast_path, keys_array_len_capped_to_capacity, keys_array_slot,
note_array_proto_iterator_write, note_object_prototype_index_write, object_prototype_addr,
object_prototype_addr_matches, object_prototype_has_index_flag,
PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED,
note_array_proto_iterator_write, note_object_prototype_index_write,
object_prototype_has_index_flag, PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED,
};
pub use self::indexing::{
js_array_get_element, js_array_get_element_f64, js_array_get_f64, js_array_get_f64_unchecked,
Expand All @@ -117,11 +119,6 @@ pub use self::indexing::{
js_array_numeric_set_f64_unboxed, js_array_set_f64, js_array_set_f64_extend,
js_array_set_f64_extend_strict, js_array_set_f64_unchecked, js_array_set_index_or_string,
js_array_set_index_or_string_strict, js_array_set_string_key,
scan_prototype_addr_cache_roots_mut,
};
#[cfg(test)]
pub(crate) use self::indexing::{
test_array_proto_addr_cache, test_keys_array_slot_fallbacks, test_object_proto_addr_cache,
};
pub use self::is_array::js_array_is_array;
pub(crate) use self::iter_methods::throw_reduce_of_empty;
Expand All @@ -142,6 +139,15 @@ pub(crate) use self::iterator::iter_bt_dump;
pub use self::iterator::{
js_array_spread_append, js_for_of_to_array, js_get_async_iterator, js_iterator_to_array,
};
pub use self::prototype_addr::scan_prototype_addr_cache_roots_mut;
pub(crate) use self::prototype_addr::{
array_prototype_addr, object_prototype_addr, object_prototype_addr_matches,
};
#[cfg(test)]
pub(crate) use self::prototype_addr::{
test_memoized_prototype_addr, test_prototype_addr_cache_wiring,
test_rewrite_prototype_addr_slot,
};
pub(crate) use self::sort::object_prototype_has_index_prop;
pub(crate) use self::sort::object_prototype_index_get as sort_object_prototype_index_get;
pub use self::subclass::{
Expand Down
Loading
Loading