fix(runtime): root every saved implicit-this across the user code it brackets — 121 sites, not ~20 (#9445)

changelog.d/9445-implicit-this-restore-sweep.md

@@ -0,0 +1,62 @@
+**Every runtime callback site now restores the caller's `this` through a GC
+root** (#9445) — the sweep PR #9444 asked for after fixing four accessor sites
+for #9417.
+
+The runtime binds a callback's receiver by writing the GC-rooted
+`IMPLICIT_THIS` cell and keeping the previous occupant in a **bare Rust local**
+for the duration of the callback:
+
+```rust
+let prev = js_implicit_this_set(receiver);
+… user code, which allocates …
+js_implicit_this_set(prev);               // pre-collection address
+```
+
+That local is the caller's receiver, and the collector cannot see or rewrite
+it. An evacuating young-gen minor inside the window — perry's default GC since
+PR #7019 — relocates the caller's object, and the restore reinstalls a retired
+from-space address as the caller's `this`. Nothing faults: the caller's next
+`this.<field>` fails the object-type check on the recycled cell and answers
+`undefined`, so the member access after it throws a TypeError naming a
+property nowhere near the defect (#9417's `Cannot read properties of
+undefined (reading 'def')`).
+
+The issue counted ~20 sites; a grep of the whole runtime finds **122**
+unrooted save/restores in 65 files (one of them landed with #9518 while this
+sweep was in flight) (timers, node streams, dgram, cluster,
+`fs.watch`, `EventTarget`, `Map`/`Set`/`URLSearchParams.forEach`, promisify,
+JSON `toJSON`/replacer/reviver, ToPrimitive and ToPropertyKey, the iterator
+protocol, Proxy traps and `Reflect`, bound functions, `super.x`, static
+dispatch, …). Every one is now the idiom `prototype_chain.rs` and PR #9444
+already use: root the saved value in a `RuntimeHandleScope` and re-read it at
+the restore. Nine sites were already rooted; `dyn_eval/bridge.rs` roots
+through its own stack. None of the 122 could be left alone — every one calls
+user code (a closure, a class accessor or static method, a Proxy trap, a
+`then`), which can allocate. Callback loops (`Map`/`Set`/`URLSearchParams.forEach`,
+`EventTarget` dispatch, the emitters, watchers and timer batches) root the
+displaced receiver **once per loop** rather than once per callback, and sites
+that already own a `RuntimeHandleScope` reuse it, so the hot per-callback cost
+is one handle read. Three sites also consumed their receiver again *after* the
+call (`intl_subclass_super`, `temporal_subclass_super`, and the `process.stdin`
+listener loops); those re-read it through a root too.
+
+**Also fixed, same family, found by the fixture:** `JSON.stringify(value,
+replacerFn)` handed the walk a **raw replacer closure pointer** after the
+root-level replacer call (and the root `toJSON`) had run user code
+(`json/replacer.rs`, both the pretty and the compact entry points). With an
+allocating replacer this was a SIGSEGV in `js_closure_call2` — the walk called
+a retired closure — and it survived the `prev` rooting alone. The closure and
+the `""` key are now rooted across those calls.
+
+**Test.** `test-files/test_gap_9445_implicit_this_restore_sweep.ts` — 34
+cases, one per synchronously reachable site family, each a `function`-method
+on a fresh young object that drives the site with an allocating callback and
+then reads `this`. Deterministic, no GC env knobs; the PR description records
+which cases print a non-zero `bad=` count on unfixed `main`. Event-loop-driven
+sites (timers, `process.stdin`, dgram, cluster, `fs.watch`, pty, child
+process) only see a heap `prev` from a nested pump and have no synchronous
+reproduction; they carry the same mechanical fix. Two further candidate cases
+(a `defineProperty` accessor on a typed array, a `toISOString` override on a
+`Date`) diverge from node for a reason unrelated to rooting and are filed as
+#9529; a `util.callbackify` case crashed the microtask pump at exit on every
+build (a promise-side rooting bug, filed separately) and is not in the file.

crates/perry-runtime/src/array/iterator.rs

@@ -482,7 +482,7 @@ fn async_from_sync_call_raw(iter: f64, method: &[u8], args: &[f64]) -> Result<Op
     };
 
     let prev_this = if callable {
-        Some(crate::object::js_implicit_this_set(iter))
+        Some(scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter)))
     } else {
         None
     };
@@ -524,7 +524,7 @@ fn async_from_sync_call_raw(iter: f64, method: &[u8], args: &[f64]) -> Result<Op
         }
     };
     if let Some(prev) = prev_this {
-        crate::object::js_implicit_this_set(prev);
+        crate::object::js_implicit_this_set(prev.get_nanbox_f64());
     }
     crate::exception::js_try_end();
     result
@@ -547,7 +547,8 @@ fn async_from_sync_call_cached_raw(
             b"Async-from-sync iterator method is not callable",
         ));
     }
-    let prev_this = crate::object::js_implicit_this_set(iter);
+    let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+    let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter));
     let trap_buf = crate::exception::js_try_push();
     let outcome = crate::exception::arm_trap_and_run(trap_buf, || {
         let args_ptr = if args.is_empty() {
@@ -565,7 +566,7 @@ fn async_from_sync_call_cached_raw(
             Err(exc)
         }
     };
-    crate::object::js_implicit_this_set(prev_this);
+    crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
     crate::exception::js_try_end();
     result
 }
@@ -848,10 +849,11 @@ pub extern "C" fn js_get_async_iterator(value: f64) -> f64 {
             if !is_callable_value(method) {
                 throw_iterator_method_not_callable();
             }
-            let prev_this = crate::object::js_implicit_this_set(value);
+            let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+            let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(value));
             let iterator =
                 unsafe { crate::closure::js_native_call_value(method, std::ptr::null(), 0) };
-            crate::object::js_implicit_this_set(prev_this);
+            crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
             // GetIterator step 5: the result must be an Object.
             if !is_async_iterator_object(iterator) {
                 throw_iterator_result_not_object();
@@ -1301,9 +1303,10 @@ fn throw_iterator_result_not_object() -> ! {
 pub extern "C" fn js_iterator_next_result(iter_f64: f64) -> f64 {
     let next = named_field(iter_f64, b"next");
     let result = if is_callable_value(next) {
-        let prev_this = crate::object::js_implicit_this_set(iter_f64);
+        let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+        let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter_f64));
         let result = unsafe { crate::closure::js_native_call_value(next, std::ptr::null(), 0) };
-        crate::object::js_implicit_this_set(prev_this);
+        crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
         result
     } else if next.to_bits() == crate::value::TAG_UNDEFINED
         && is_builtin_iterator_class_id(crate::value::js_nanbox_get_pointer(iter_f64) as usize)
@@ -1363,9 +1366,10 @@ pub extern "C" fn js_iterator_close_if_not_done(iter_f64: f64, done_f64: f64) ->
         crate::closure::throw_not_callable();
     }
 
-    let prev_this = crate::object::js_implicit_this_set(iter_f64);
+    let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+    let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter_f64));
     let result = unsafe { crate::closure::js_native_call_value(ret, std::ptr::null(), 0) };
-    crate::object::js_implicit_this_set(prev_this);
+    crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
     if !is_object_like_value(result) {
         throw_iterator_result_not_object();
     }
@@ -1439,9 +1443,11 @@ pub(crate) fn sync_iterator_to_array_if_not_async(iter_f64: f64) -> Option<*mut
         } else {
             // Call(next, iterator) — bind `this` like `js_iterator_to_array`
             // does for its stored-closure path (#9019).
-            let prev_this = crate::object::js_implicit_this_set(iter_f64);
+            let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+            let prev_this =
+                this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter_f64));
             let r = closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED));
-            crate::object::js_implicit_this_set(prev_this);
+            crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
             r
         };
         if crate::promise::js_value_is_promise(step) != 0 {
@@ -1477,9 +1483,10 @@ pub(crate) fn call_symbol_async_iterator(value: f64) -> Option<f64> {
     if !is_callable_value(method) {
         return None;
     }
-    let prev_this = crate::object::js_implicit_this_set(value);
+    let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+    let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(value));
     let iterator = unsafe { crate::closure::js_native_call_value(method, std::ptr::null(), 0) };
-    crate::object::js_implicit_this_set(prev_this);
+    crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
     if iterator.to_bits() == crate::value::TAG_UNDEFINED {
         None
     } else {
@@ -1633,12 +1640,13 @@ pub extern "C" fn js_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
             // Call(next, iterator): bind `this` for the stored-closure path
             // exactly like `js_iterator_next_result` — a user-assigned
             // `it.next = function () { … }` may read `this` (#9019).
-            let prev_this = crate::object::js_implicit_this_set(iter_h.get_nanbox_f64());
+            let prev_this =
+                scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter_h.get_nanbox_f64()));
             let r = closure::js_closure_call1(
                 js_nanbox_get_pointer(next_h.get_nanbox_f64()) as *const closure::ClosureHeader,
                 f64::from_bits(TAG_UNDEFINED),
             );
-            crate::object::js_implicit_this_set(prev_this);
+            crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
             r
         };
         // IteratorNext (ECMA-262 §7.4.2 step 3): if Type(result) is not
@@ -1755,9 +1763,11 @@ fn js_async_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
         } else {
             // Call(next, iterator) — bind `this` for the stored-closure path
             // (#9019), mirroring `js_iterator_to_array`.
-            let prev_this = crate::object::js_implicit_this_set(iter_f64);
+            let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+            let prev_this =
+                this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(iter_f64));
             let r = closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED));
-            crate::object::js_implicit_this_set(prev_this);
+            crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
             r
         };
         let Some(step_result) = settled_promise_value(step) else {

crates/perry-runtime/src/async_hooks.rs

@@ -1811,7 +1811,9 @@ fn call_callback_with_rest(callback_value: f64, this_arg: f64, rest: f64) -> f64
     }
     let args_array = ptr_from_nanboxed(rest) as *const ArrayHeader;
     let args_array_handle = scope.root_raw_const_ptr(args_array);
-    let prev_this = crate::object::js_implicit_this_set(this_arg_handle.get_nanbox_f64());
+    let prev_this = scope.root_nanbox_f64(crate::object::js_implicit_this_set(
+        this_arg_handle.get_nanbox_f64(),
+    ));
     let result = if args_array.is_null() {
         unsafe { js_closure_call_array(callback as i64, ptr::null(), 0) }
     } else {
@@ -1824,7 +1826,7 @@ fn call_callback_with_rest(callback_value: f64, this_arg: f64, rest: f64) -> f64
         };
         unsafe { js_closure_call_array(callback as i64, data, len) }
     };
-    crate::object::js_implicit_this_set(prev_this);
+    crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
     result
 }
 

crates/perry-runtime/src/child_process/emitter.rs

@@ -52,6 +52,9 @@ pub(crate) fn cp_emit(target: f64, event: &str, args: &[f64]) -> bool {
     let key = cp_listener_key(event);
     let mut i: u32 = 0;
     let mut fired = false;
+    let this_scope = crate::gc::RuntimeHandleScope::new();
+    // #9445: the displaced receiver is rooted ONCE here, not once per callback.
+    let prev = this_scope.root_nanbox_f64(crate::object::js_implicit_this_get());
     loop {
         let arr = match cp_array_ptr(cp_get_field(target, &key)) {
             Some(a) => a,
@@ -61,11 +64,11 @@ pub(crate) fn cp_emit(target: f64, event: &str, args: &[f64]) -> bool {
             break;
         }
         let cb = crate::array::js_array_get_f64(arr, i);
-        let prev = js_implicit_this_set(target);
+        js_implicit_this_set(target);
         unsafe {
             let _ = js_native_call_value(cb, args.as_ptr(), args.len());
         }
-        js_implicit_this_set(prev);
+        js_implicit_this_set(prev.get_nanbox_f64());
         fired = true;
         i += 1;
     }
@@ -255,12 +258,13 @@ pub(crate) extern "C" fn cp_pipe_data_thunk(closure: *const ClosureHeader, chunk
     let dest = f64::from_bits(js_closure_get_capture_ptr(closure, 0) as u64);
     let write = cp_get_field(dest, b"write");
     if !crate::fs::extract_closure_ptr(write).is_null() {
-        let prev = js_implicit_this_set(dest);
+        let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+        let prev = this_scope.root_nanbox_f64(js_implicit_this_set(dest));
         let args = [chunk];
         unsafe {
             let _ = js_native_call_value(write, args.as_ptr(), args.len());
         }
-        js_implicit_this_set(prev);
+        js_implicit_this_set(prev.get_nanbox_f64());
     }
     cp_undefined()
 }
@@ -272,12 +276,13 @@ pub(crate) extern "C" fn cp_pipe_end_thunk(closure: *const ClosureHeader) -> f64
     let dest = f64::from_bits(js_closure_get_capture_ptr(closure, 0) as u64);
     let end = cp_get_field(dest, b"end");
     if !crate::fs::extract_closure_ptr(end).is_null() {
-        let prev = js_implicit_this_set(dest);
+        let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+        let prev = this_scope.root_nanbox_f64(js_implicit_this_set(dest));
         let args = [cp_undefined()];
         unsafe {
             let _ = js_native_call_value(end, args.as_ptr(), 0);
         }
-        js_implicit_this_set(prev);
+        js_implicit_this_set(prev.get_nanbox_f64());
     }
     cp_undefined()
 }

crates/perry-runtime/src/closure/dispatch/bound.rs

@@ -214,7 +214,8 @@ unsafe fn dispatch_symbol_bound_method(
         // the direct-call path. The one-shot static-`this` override (armed by
         // the Function.prototype call/apply arms for a static bound-method
         // value) still wins in the static-method prologue.
-        let prev_this = crate::object::js_implicit_this_set(receiver);
+        let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+        let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
         crate::object::static_private_owner_push(receiver);
         let result = crate::object::call_registered_static_method(
             func_ptr,
@@ -224,7 +225,7 @@ unsafe fn dispatch_symbol_bound_method(
             has_rest,
         );
         crate::object::static_private_owner_pop();
-        crate::object::js_implicit_this_set(prev_this);
+        crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
         result
     } else {
         // Computed symbol methods never synthesize an `arguments` object but
@@ -267,14 +268,15 @@ pub unsafe fn dispatch_bound_function(closure: *const ClosureHeader, args: &[f64
     // slot, not IMPLICIT_THIS — rebind it to the bound receiver so the bound
     // `this` is honored (arrows/non-captures_this targets are returned as-is).
     let target = rebind_explicit_this(target, bound_this);
-    let prev_this = crate::object::js_implicit_this_set(bound_this);
+    let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+    let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(bound_this));
     let (call_ptr, call_len) = if combined.is_empty() {
         (std::ptr::null::<f64>(), 0usize)
     } else {
         (combined.as_ptr(), combined.len())
     };
     let result = js_native_call_value(target, call_ptr, call_len);
-    crate::object::js_implicit_this_set(prev_this);
+    crate::object::js_implicit_this_set(prev_this.get_nanbox_f64());
     result
 }
 

crates/perry-runtime/src/closure/dynamic_props.rs

@@ -600,9 +600,10 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 {
             return f64::from_bits(crate::value::TAG_UNDEFINED);
         }
         let receiver = crate::value::js_nanbox_pointer(ptr as i64);
-        let prev = crate::object::js_implicit_this_set(receiver);
+        let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+        let prev = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
         let result = crate::closure::js_closure_call0(closure);
-        crate::object::js_implicit_this_set(prev);
+        crate::object::js_implicit_this_set(prev.get_nanbox_f64());
         return result;
     }
 
@@ -661,9 +662,11 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 {
                 if getter.is_null() {
                     return f64::from_bits(crate::value::TAG_UNDEFINED);
                 }
-                let prev = crate::object::js_implicit_this_set(receiver);
+                let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+                let prev =
+                    this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
                 let result = crate::closure::js_closure_call0(getter);
-                crate::object::js_implicit_this_set(prev);
+                crate::object::js_implicit_this_set(prev.get_nanbox_f64());
                 return result;
             }
             if let Ok(props) = get_closure_props().lock() {
@@ -692,9 +695,10 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 {
             if getter.is_null() {
                 return f64::from_bits(crate::value::TAG_UNDEFINED);
             }
-            let prev = crate::object::js_implicit_this_set(receiver);
+            let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+            let prev = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
             let result = crate::closure::js_closure_call0(getter);
-            crate::object::js_implicit_this_set(prev);
+            crate::object::js_implicit_this_set(prev.get_nanbox_f64());
             return result;
         }
         {
@@ -724,9 +728,11 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 {
                     (acc.get & crate::value::POINTER_MASK) as *const crate::closure::ClosureHeader;
                 if !getter.is_null() {
                     let receiver = crate::value::js_nanbox_pointer(ptr as i64);
-                    let prev = crate::object::js_implicit_this_set(receiver);
+                    let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445
+                    let prev =
+                        this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
                     let result = crate::closure::js_closure_call0(getter);
-                    crate::object::js_implicit_this_set(prev);
+                    crate::object::js_implicit_this_set(prev.get_nanbox_f64());
                     return result;
                 }
             }