fix(gc): root the ten unrooted runtime-side caches, and the scanner that walked 1 of 3 sibling slots (#7231)

changelog.d/7239-gc-unrooted-runtime-caches.md

@@ -0,0 +1,56 @@
+**GC: root the ten unrooted runtime-side caches, and close a scanner that walked one of three sibling slots (#7231).**
+
+The class #7226 established — a runtime table holding a GC pointer that is not a
+registered root. It is strictly worse than the #7154 stale-register class (it goes
+bad at collection #0 and stays bad, rather than needing a collection to land in a
+narrow window) and it is **invisible to every static instrument**, because
+`gc_root_dominance_check.py` reads emitted LLVM IR and a runtime table is not in it.
+
+★ **`process.env`** is the load-bearing one. `js_process_env_impl` builds it once
+with `js_object_alloc` — the nursery — and caches it in a thread-local `Cell<f64>`
+that is the *entire* reference graph: `process.env` is a getter CALL, not a field of
+the `process` object. The first minor swept or evacuated it, and every later
+`process.env.X = v` wrote through a dangling pointer. The observable is
+ENUMERATION (`Object.keys`, `for…in`, spread — how `@next/env` and `dotenv` consume
+it), because a direct read lowers to `js_getenv` and asks the OS. Measured at
+`c9cd73ba5` under `PERRY_GC_MOVING_LOOP_POLLS=1` at compile and run:
+**SIGBUS 10/10 before, `bad 0` 10/10 after**, byte-exact against node 26.5.1, clean
+on the shipped default both sides. The sibling `PROCESS_FINALIZATION_OBJECT` uses the
+same materialize-once idiom and was already rooted — this was an omission, not a
+design.
+
+Also rooted: `CACHED_PERMISSION` and `CACHED_REPORT` (same shape; the
+`runtime_write_barrier_root_nanbox` beside the first is an incremental *mark*
+barrier, not a root registration); `ERROR_CONSTRUCTOR_PTR` (a raw duplicate of a
+`globalThis` closure, outside the object graph, so stale after a move);
+`tui/input.rs` `INPUT_HANDLER` (the inline `useInput` arrow, which nothing else
+refers to); `tty.rs` `RESIZE_CALLBACK` (bypasses the rooted EventEmitter listener
+array); `frame.rs` `FRAME_CALLBACKS` (rooted only transiently during registration —
+its `unsafe impl Send` SAFETY comment asserted the opposite and is corrected);
+`CURRENT_NEW_TARGET`; `ACCESSOR_RECEIVER_OVERRIDE`; and `PENDING_FETCH_SIGNAL`.
+
+**Scanner gap**, the shape #7230 found twice: `worker_threads.rs`'s
+`scan_parent_port_event_roots_mut` visited `MESSAGE_EVENT_CALLBACKS` and neither
+`MESSAGE_CALLBACK` nor `CLOSE_CALLBACK` — three slots in the same `thread_local!`
+block holding the same raw `ClosureHeader*`. `parentPort.on('message')` /
+`on('close')` handlers were reclaimed by the next collection inside a worker.
+
+Two further windows closed in `frame.rs` while rooting its queue: `js_frame_tick`
+drained into an unrooted local `Vec` and rooted each callback only as it invoked it,
+leaving the rest of the batch naked across arbitrary user code (#7230's
+staging-buffer shape); and `js_on_frame_callback` held the queue mutex across an
+allocating `capture_context()`, which becomes a self-deadlock once a scanner locks
+the same mutex.
+
+**Refuted, and worth recording.** All 8 budgeted (FULL, STEP) scanner pairs were
+diffed field-by-field: **no drift** — #7230's `IntervalTimer.args` fix reached both
+twins. `buffer/header.rs`'s nine address registries are not root gaps
+(`GC_TYPE_BUFFER`/`GC_TYPE_TYPED_ARRAY` are `movable: false`; they are identity sets,
+not liveness references). `static_plugins.rs` is unreachable — `perry_register_static_plugin`
+has no caller in the workspace.
+
+Not closed, and stated: `promise/rejection.rs`'s `internally_handled` needs a
+**rekey** rather than a root; the save/restore idiom in `CURRENT_NEW_TARGET` and
+`ACCESSOR_RECEIVER_OVERRIDE` still parks the displaced value in a bare Rust local;
+and `MODULE_PATH_REGISTRY` is process-global while arenas are per-thread, so a naive
+scanner would be unsound.

crates/perry-runtime/src/frame.rs

@@ -24,9 +24,22 @@ struct FrameCallback {
     cleared: bool,
 }
 
-// SAFETY: closure pointers point to global compiled code / GC-rooted data.
+// SAFETY: `callback` is a heap `ClosureHeader*`, kept live and address-current
+// by `scan_frame_callback_roots_mut` (registered in `gc_init`). It is NOT
+// "global compiled code", which is what this comment used to claim — that was
+// the premise under which the queue went unrooted for its whole existence
+// (#7231).
 unsafe impl Send for FrameCallback {}
 
+/// Pending `onFrame(cb)` registrations.
+///
+/// **This is a GC root, and must stay one (#7231).** `js_on_frame_callback`
+/// roots the closure with a `RuntimeHandleScope` only for the duration of the
+/// registration call; once that scope drops, this queue is the sole reference
+/// to it until the next `js_frame_tick`. Before
+/// `scan_frame_callback_roots_mut` a collection between registration and the
+/// next vsync reclaimed or relocated the closure and `js_closure_call2` then
+/// called through a dangling `ClosureHeader*`.
 static FRAME_CALLBACKS: Mutex<Vec<FrameCallback>> = Mutex::new(Vec::new());
 static NEXT_FRAME_ID: Mutex<i64> = Mutex::new(1);
 static LAST_FIRE_BY_CLOSURE: Mutex<Option<HashMap<i64, f64>>> = Mutex::new(None);
@@ -60,17 +73,34 @@ pub extern "C" fn js_on_frame_callback(callback: i64) -> i64 {
     let cb_handle = scope.root_raw_const_ptr(callback as *const ClosureHeader);
 
     let id = next_frame_id();
+    // `capture_context` can allocate, so it runs BEFORE the queue lock is
+    // taken. Evaluating it inside the `push(...)` argument list held the
+    // mutex across an allocation, which — now that a root scanner locks the
+    // same mutex — would be a self-deadlock the moment that allocation
+    // triggered a collection.
+    let context = crate::async_context::capture_context();
 
     FRAME_CALLBACKS.lock().unwrap().push(FrameCallback {
         id,
+        // Re-read below the allocation: `capture_context` may have moved it.
         callback: cb_handle.get_raw_const_ptr::<ClosureHeader>() as i64,
-        context: crate::async_context::capture_context(),
+        context,
         cleared: false,
     });
 
     id
 }
 
+/// Root + rewrite every pending frame callback closure.
+pub(crate) fn scan_frame_callback_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
+    let mut queue = FRAME_CALLBACKS
+        .lock()
+        .unwrap_or_else(|poisoned| poisoned.into_inner());
+    for cb in queue.iter_mut() {
+        visitor.visit_i64_slot(&mut cb.callback);
+    }
+}
+
 /// Cancel a previously-registered frame callback. No-op if `id` is unknown
 /// or already fired.
 #[no_mangle]
@@ -109,10 +139,22 @@ pub extern "C" fn js_frame_tick(timestamp_ms: f64) -> i32 {
         queue.drain(..).filter(|t| !t.cleared).collect()
     };
 
+    // The drain removes every callback from the (now scanned) queue, so for
+    // the length of this loop `pending` is the ONLY reference to all of them
+    // — and callback #1 runs arbitrary user code that allocates. Rooting each
+    // one individually inside the loop covers the callback being invoked and
+    // leaves #2..#N naked, which is the same shape as #7230's staging buffer.
+    // One batch scope, entered before the first call, covers the whole set.
+    let batch = crate::gc::RuntimeHandleScope::new();
+    let batch_handles: Vec<_> = pending
+        .iter()
+        .map(|cb| batch.root_raw_const_ptr(cb.callback as *const ClosureHeader))
+        .collect();
+
     let mut fired = 0;
-    for cb in pending {
+    for (cb, batch_handle) in pending.iter().zip(batch_handles.iter()) {
         let scope = crate::gc::RuntimeHandleScope::new();
-        let cb_handle = scope.root_raw_const_ptr(cb.callback as *const ClosureHeader);
+        let cb_handle = scope.root_raw_const_ptr(batch_handle.get_raw_const_ptr::<ClosureHeader>());
 
         let delta_ms = {
             let mut slot = LAST_FIRE_BY_CLOSURE.lock().unwrap();

crates/perry-runtime/src/gc/mod.rs

@@ -625,6 +625,41 @@ pub fn gc_init() {
     gc_register_mutable_root_scanner(crate::tls::scan_tls_roots_mut);
     gc_register_mutable_root_scanner(crate::process::scan_process_finalization_roots_mut);
     gc_register_mutable_root_scanner(crate::process::scan_process_module_loader_roots_mut);
+    // #7231: the materialize-once `process.*` caches. Each is a thread-local
+    // cell holding a NURSERY-allocated object that nothing else refers to —
+    // `process.env` / `.permission` / `.report` are getter CALLS, not fields
+    // of the `process` object, so the cache is the whole reference graph.
+    // `scan_process_finalization_roots_mut` above is the identical idiom and
+    // was already registered; these three were an omission, not a design.
+    // `CACHED_ENV` is the load-bearing one: `process.env` is touched by nearly
+    // every real Node program, and every `process.env.X = v` after the first
+    // collection wrote through a dangling pointer.
+    gc_register_mutable_root_scanner(crate::process::scan_process_env_cache_roots_mut);
+    gc_register_mutable_root_scanner(crate::process::scan_permission_cache_roots_mut);
+    gc_register_mutable_root_scanner(crate::process::scan_report_cache_roots_mut);
+    // #7231: the raw `Error` constructor address behind
+    // `Error.prepareStackTrace`. The closure is reachable through `globalThis`
+    // so it is not swept, but this duplicate lives outside the object graph
+    // and goes stale on a move.
+    gc_register_mutable_root_scanner(crate::object::scan_error_constructor_root_mut);
+    // #7231: native callback slots that bypass their rooted sibling
+    // structures. `RESIZE_CALLBACK` bypasses the EventEmitter listener array;
+    // `FRAME_CALLBACKS` is rooted only transiently by a `RuntimeHandleScope`
+    // during registration; `INPUT_HANDLER` holds the `useInput` arrow, which
+    // in idiomatic inline form has no other reference at all.
+    gc_register_mutable_root_scanner(crate::tty::scan_tty_resize_callback_root_mut);
+    gc_register_mutable_root_scanner(crate::frame::scan_frame_callback_roots_mut);
+    gc_register_mutable_root_scanner(crate::tui::input::scan_tui_input_handler_root_mut);
+    // #7231: three in-flight cells that hold a NaN-boxed heap value across a
+    // window in which user code can run. Each is a second copy of a value
+    // whose original is rooted elsewhere, or the only copy for the length of
+    // the window; both shapes are the #7226 `prev_this` family. Rooting the
+    // CELL is the half a scanner can close — the displaced value each
+    // save/restore idiom parks in a bare Rust local is noted at each
+    // declaration and needs `RuntimeHandleScope` plumbing, not a scanner.
+    gc_register_mutable_root_scanner(crate::object::scan_current_new_target_root_mut);
+    gc_register_mutable_root_scanner(crate::object::scan_accessor_receiver_override_root_mut);
+    gc_register_mutable_root_scanner(crate::object::scan_pending_fetch_signal_root_mut);
     gc_register_mutable_root_scanner(crate::os::scan_process_event_listener_roots_mut);
     // #6077: keep promises tracked for an unhandled rejection alive + address-
     // stable until reported, so the program-end report is not a stale/UAF read.

crates/perry-runtime/src/object/class_registry.rs

@@ -39,6 +39,7 @@ use super::*;
 
 mod class_meta;
 mod construct;
+pub(crate) use construct::scan_current_new_target_root_mut;
 mod dispatch;
 mod gc_roots;
 pub(crate) mod parent_static;

crates/perry-runtime/src/object/class_registry/construct.rs

@@ -2,10 +2,36 @@ use super::*;
 use crate::JSValue;
 
 thread_local! {
+    /// `new.target` for the construction currently on this thread's stack.
+    ///
+    /// **This is a GC root, and must stay one (#7231).** It holds a NaN-boxed
+    /// closure/class value for the whole constructor body, and a constructor
+    /// body runs arbitrary user code. `this_binding.rs`'s `NEW_TARGET` holds
+    /// the same value under `scan_implicit_this_roots_mut`; this is a second
+    /// copy on a different path, and a second copy of a root that is not
+    /// itself a root is exactly the shape #7226 found in `prev_this`.
+    ///
+    /// RESIDUAL, deliberately not closed here: the save/restore idiom parks
+    /// the DISPLACED value in a bare Rust local (`prev_current_new_target`)
+    /// across the construction and republishes it afterwards. Runtime frames
+    /// are not covered by the precise scan, so that local is #7226's
+    /// `prev_this` defect in Rust rather than in codegen. Closing it means
+    /// routing the three save sites through a `RuntimeHandleScope`, which
+    /// wants its own before/after rather than being appended here.
     static CURRENT_NEW_TARGET: std::cell::Cell<u64> =
         const { std::cell::Cell::new(crate::value::TAG_UNDEFINED) };
 }
 
+/// Root + rewrite the in-flight `new.target`.
+pub(crate) fn scan_current_new_target_root_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
+    CURRENT_NEW_TARGET.with(|cell| {
+        let mut bits = cell.get();
+        if visitor.visit_nanbox_u64_slot(&mut bits) {
+            cell.set(bits);
+        }
+    });
+}
+
 #[no_mangle]
 pub extern "C" fn js_new_target_value() -> f64 {
     f64::from_bits(CURRENT_NEW_TARGET.with(|value| value.get()))

crates/perry-runtime/src/object/field_get_set.rs

@@ -117,6 +117,7 @@ pub(crate) fn is_fetch_subclass_body_method(name: &[u8]) -> bool {
 
 // ── Topical sub-modules (issue #1103: keep every file < 2000 lines) ──
 mod accessors;
+pub(crate) use accessors::scan_accessor_receiver_override_root_mut;
 mod buffer_own_prop;
 mod class_object_props;
 mod crypto_key;

crates/perry-runtime/src/object/field_get_set/accessors.rs

@@ -206,10 +206,35 @@ thread_local! {
     /// `[[Get]](P, Receiver)`. (object-literal getters on a `Object.create`
     /// prototype — e.g. @hono/node-server's request prototype reading
     /// `this[incomingKey].method`.)
+    ///
+    /// **This is a GC root, and must stay one (#7231).** The stashed receiver
+    /// is a NaN-boxed heap value that stays armed for the whole prototype
+    /// walk, and a walk can reach a Proxy `get` trap — arbitrary user code
+    /// that allocates. Nothing else refers to it while it sits here, so
+    /// without `scan_accessor_receiver_override_root_mut` the getter is
+    /// invoked with a `this` naming from-space.
+    ///
+    /// RESIDUAL, same shape as `CURRENT_NEW_TARGET`: the displaced value that
+    /// `accessor_receiver_override_begin` returns rides a bare Rust local
+    /// through the walk and is republished by `_end`. Rooting the cell
+    /// protects the ARMED value, not the saved one.
     static ACCESSOR_RECEIVER_OVERRIDE: std::cell::Cell<Option<f64>>
         = const { std::cell::Cell::new(None) };
 }
 
+/// Root + rewrite the in-flight inherited-accessor receiver.
+pub(crate) fn scan_accessor_receiver_override_root_mut(
+    visitor: &mut crate::gc::RuntimeRootVisitor<'_>,
+) {
+    ACCESSOR_RECEIVER_OVERRIDE.with(|cell| {
+        if let Some(mut value) = cell.get() {
+            if visitor.visit_nanbox_f64_slot(&mut value) {
+                cell.set(Some(value));
+            }
+        }
+    });
+}
+
 pub(crate) fn accessor_receiver_override_begin(receiver: f64) -> Option<f64> {
     ACCESSOR_RECEIVER_OVERRIDE.with(|c| {
         // Keep the OUTERMOST receiver across multi-hop prototype walks.

crates/perry-runtime/src/object/global_fetch.rs

@@ -12,10 +12,26 @@ thread_local! {
     /// The `signal` from the in-progress `fetch(url, { signal })` call, stashed
     /// so the stdlib `js_fetch_with_options` (whose 4-arg ABI predates
     /// AbortSignal support) can pick it up at entry without an ABI change.
+    ///
+    /// **This is a GC root, and must stay one (#7231).** The `AbortSignal` is
+    /// a NaN-boxed heap object, and between the stash and
+    /// `js_fetch_with_options`'s consume the argument lowering for the fetch
+    /// call itself still runs and allocates. The window is short, but the
+    /// cell is the only reference across it.
     static PENDING_FETCH_SIGNAL: Cell<f64> =
         const { Cell::new(f64::from_bits(crate::value::TAG_UNDEFINED)) };
 }
 
+/// Root + rewrite the stashed in-flight `fetch` `AbortSignal`.
+pub(crate) fn scan_pending_fetch_signal_root_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
+    PENDING_FETCH_SIGNAL.with(|cell| {
+        let mut value = cell.get();
+        if visitor.visit_nanbox_f64_slot(&mut value) {
+            cell.set(value);
+        }
+    });
+}
+
 /// Stash the `signal` for the fetch call about to be dispatched. Set on the main
 /// thread immediately before the fetch call and consumed at the start of
 /// `js_fetch_with_options` — JS is single-threaded between those two points, so

crates/perry-runtime/src/object/global_this.rs

@@ -143,7 +143,8 @@ pub(crate) use math_temporal::install_temporal_namespace;
 pub(crate) use math_temporal::temporal_kind_prototype;
 pub(crate) use math_temporal::{install_math_namespace, temporal_ctor_kind};
 pub(crate) use populate::{
-    default_prepare_stack_trace_func_ptr, populate_global_this_builtins, ERROR_CONSTRUCTOR_PTR,
+    default_prepare_stack_trace_func_ptr, populate_global_this_builtins,
+    scan_error_constructor_root_mut, ERROR_CONSTRUCTOR_PTR,
 };
 pub(crate) use proto_methods::{
     install_error_prototype_data_properties, populate_builtin_prototype_methods,

crates/perry-runtime/src/object/global_this/populate.rs

@@ -819,10 +819,29 @@ thread_local! {
     /// `perry/thread` agent has its own arena + realm, and an `Error`
     /// constructor / `prepareStackTrace` from another thread's arena can be a
     /// foreign or freed pointer — the same reason `globalThis` is per-thread.
+    ///
+    /// **This is a GC root, and must stay one (#7231).** The address is a RAW
+    /// `*mut ClosureHeader` from `js_closure_alloc` — a nursery allocation.
+    /// The canonical `Error` closure is also a field of `globalThis`, so the
+    /// structural trace keeps it alive and rewrites THAT reference; this
+    /// duplicate lives outside the object graph, so an evacuating collection
+    /// leaves it naming from-space and `error_prepare_stack_trace_override`
+    /// then reads `prepareStackTrace` off an abandoned closure. Rooted by
+    /// `scan_error_constructor_root_mut`.
     pub(crate) static ERROR_CONSTRUCTOR_PTR: std::cell::Cell<usize> =
         const { std::cell::Cell::new(0) };
 }
 
+/// Root + rewrite the raw `Error` constructor address.
+pub(crate) fn scan_error_constructor_root_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
+    ERROR_CONSTRUCTOR_PTR.with(|cell| {
+        let mut addr = cell.get();
+        if addr != 0 && visitor.visit_usize_slot(&mut addr) {
+            cell.set(addr);
+        }
+    });
+}
+
 /// The default `Error.prepareStackTrace` thunk's address — used to tell a
 /// user override apart from Perry's built-in default.
 pub(crate) fn default_prepare_stack_trace_func_ptr() -> usize {

crates/perry-runtime/src/object/mod.rs

@@ -36,6 +36,7 @@ mod class_constructors;
 mod class_gc_roots;
 mod class_handles;
 mod class_registry;
+pub(crate) use class_registry::scan_current_new_target_root_mut;
 mod collection_proto_thunks;
 mod data_view_registry;
 mod dataview_proto_thunks;
@@ -45,12 +46,16 @@ mod descriptors;
 mod disposable_proto_thunks;
 pub(crate) mod exotic_expando;
 mod field_get_set;
+pub(crate) use field_get_set::scan_accessor_receiver_override_root_mut;
 mod field_set_by_name;
 mod global_fetch;
+pub(crate) use global_fetch::scan_pending_fetch_signal_root_mut;
 mod global_this;
 pub mod handle_expando;
 pub(crate) mod prop_plan;
-pub(crate) use global_this::{default_prepare_stack_trace_func_ptr, ERROR_CONSTRUCTOR_PTR};
+pub(crate) use global_this::{
+    default_prepare_stack_trace_func_ptr, scan_error_constructor_root_mut, ERROR_CONSTRUCTOR_PTR,
+};
 mod global_this_tables;
 mod groupby;
 pub(crate) mod has_own_helpers;