fix(gc): preserve request graphs across route imports (#8044)

* fix(gc): preserve requests across route imports

* docs: add changelog for request import rooting

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>

Author: proggeramlug

changelog.d/8044-next-request-import-rooting.md

@@ -0,0 +1,6 @@
+### Fixed
+
+- Preserve request objects and their nested URL, search-parameter, header, and
+  body state when generated route modules re-export imported handlers. Imported
+  handler closures and complete expired-timer batches now remain rooted across
+  allocation, promise/timer continuations, microtask checkpoints, and moving GC.

crates/perry-codegen/src/lower_call/extern_func.rs

@@ -1688,26 +1688,48 @@ pub fn try_lower_extern_func_call(
     // an arrow-bound exported value (hono's `mergePath` from utils/url.js,
     // any `export const foo = () => …` cross-module use).
     if ctx.imported_vars.contains(name) {
-        ctx.pending_declares.push((fname.clone(), DOUBLE, vec![]));
-        let closure_box = ctx.block().call(DOUBLE, &fname, &[]);
-        let mut lowered_args: Vec<String> = Vec::with_capacity(args.len());
-        for a in args {
-            lowered_args.push(lower_expr(ctx, a)?);
-        }
-        if lowered_args.len() > 16 {
+        if args.len() > 16 {
             anyhow::bail!(
                 "perry-codegen Phase D.1: closure call with {} args (max 16)",
-                lowered_args.len()
+                args.len()
             );
         }
-        let blk = ctx.block();
-        let closure_handle = unbox_to_i64(blk, &closure_box);
-        let runtime_fn = format!("js_closure_call{}", lowered_args.len());
-        let mut call_args: Vec<(crate::types::LlvmType, &str)> = vec![(I64, &closure_handle)];
-        for v in &lowered_args {
-            call_args.push((DOUBLE, v.as_str()));
-        }
-        return Ok(Some(blk.call(DOUBLE, &runtime_fn, &call_args)));
+        ctx.pending_declares.push((fname.clone(), DOUBLE, vec![]));
+        // Fetch the callee before evaluating arguments, as JavaScript requires,
+        // but keep that closure rooted while argument expressions run. Next's
+        // App Route exports are commonly `export const GET = ...`; constructing
+        // a request argument can allocate and evacuate the closure before this
+        // path finally dispatches it (#8036).
+        let closure_box = ctx.block().call(DOUBLE, &fname, &[]);
+        let arg_refs: Vec<&Expr> = args.iter().collect();
+        let lowered_args = std::cell::RefCell::new(Vec::<String>::new());
+        let result = crate::rooting::with_rooted_accumulator(
+            ctx,
+            crate::rooting::Repr::Boxed,
+            &closure_box,
+            crate::rooting::any_operand_may_collect(ctx, args.iter()),
+            |ctx, _closure| {
+                crate::rooting::with_operands_rooted(ctx, &arg_refs, |_ctx, vals| {
+                    *lowered_args.borrow_mut() = vals.to_vec();
+                    Ok(())
+                })
+            },
+            |ctx, closure_box| {
+                // Re-read and unbox only after every collecting argument and
+                // after the argument group's own re-reads have completed.
+                let lowered = lowered_args.borrow();
+                let blk = ctx.block();
+                let closure_handle = unbox_to_i64(blk, closure_box);
+                let runtime_fn = format!("js_closure_call{}", lowered.len());
+                let mut call_args: Vec<(crate::types::LlvmType, &str)> =
+                    vec![(I64, &closure_handle)];
+                for value in lowered.iter() {
+                    call_args.push((DOUBLE, value.as_str()));
+                }
+                Ok(blk.call(DOUBLE, &runtime_fn, &call_args))
+            },
+        )?;
+        return Ok(Some(result));
     }
     // Record the cross-module call so the caller can add a `declare`
     // line for it after the &mut LlFunction borrow is released. The

crates/perry-runtime/src/gc/tests/runtime_roots.rs

@@ -485,17 +485,24 @@ extern "C" fn test_async_hook_event_force_minor_gc(
 thread_local! {
     static TEST_TIMER_ARG_BITS: Cell<u64> = const { Cell::new(0) };
     static TEST_TIMER_CALLED: Cell<bool> = const { Cell::new(false) };
+    static TEST_TIMER_CALLBACK_PTR: Cell<usize> = const { Cell::new(0) };
 }
 
 extern "C" fn test_timer_capture_arg(
-    _closure: *const crate::closure::ClosureHeader,
+    closure: *const crate::closure::ClosureHeader,
     arg: f64,
 ) -> f64 {
+    TEST_TIMER_CALLBACK_PTR.with(|slot| slot.set(closure as usize));
     TEST_TIMER_ARG_BITS.with(|slot| slot.set(arg.to_bits()));
     TEST_TIMER_CALLED.with(|slot| slot.set(true));
     f64::from_bits(crate::value::TAG_UNDEFINED)
 }
 
+extern "C" fn test_timer_force_minor_gc(_closure: *const crate::closure::ClosureHeader) -> f64 {
+    let _ = crate::gc::gc_collect_minor();
+    f64::from_bits(crate::value::TAG_UNDEFINED)
+}
+
 extern "C" fn test_rest_first_value(
     _closure: *const crate::closure::ClosureHeader,
     rest: f64,

crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs

@@ -133,6 +133,53 @@ fn test_timer_tick_roots_callback_args_and_previous_context_across_hooks() {
     crate::timer::clearTimeout(timer_id);
 }
 
+#[test]
+fn test_timer_tick_roots_the_complete_detached_expired_batch() {
+    let _async_hook_guard = AsyncHookRuntimeTestGuard::new();
+    let _guard = CopyingNurseryTestGuard::new(0);
+    let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+    register_runtime_handle_root_scanner_for_tests();
+    gc_register_mutable_root_scanner(crate::timer::scan_timer_roots_mut);
+
+    let collecting_callback =
+        crate::closure::js_closure_alloc(test_timer_force_minor_gc as *const u8, 0);
+    let second_callback = crate::closure::js_closure_alloc(test_timer_capture_arg as *const u8, 0);
+    let second_callback_original = second_callback as usize;
+    let second_arg = test_string_value(b"detached-timer-batch");
+    let second_arg_original = (second_arg.to_bits() & POINTER_MASK) as usize;
+    let second_args = [second_arg];
+
+    let first_id = crate::timer::js_set_timeout_callback(collecting_callback as i64, 0.0);
+    let second_id = unsafe {
+        crate::timer::js_set_timeout_callback_args(
+            second_callback as i64,
+            0.0,
+            second_args.as_ptr(),
+            1,
+        )
+    };
+    TEST_TIMER_ARG_BITS.with(|slot| slot.set(0));
+    TEST_TIMER_CALLED.with(|slot| slot.set(false));
+    TEST_TIMER_CALLBACK_PTR.with(|slot| slot.set(0));
+
+    assert_eq!(crate::timer::js_callback_timer_tick(), 2);
+    assert!(TEST_TIMER_CALLED.with(|slot| slot.get()));
+    let dispatched_callback = TEST_TIMER_CALLBACK_PTR.with(|slot| slot.get());
+    assert_ne!(
+        dispatched_callback, second_callback_original,
+        "detached timer callback should be refreshed after copied-minor GC"
+    );
+    assert!(crate::closure::is_closure_ptr(dispatched_callback));
+    assert_moved_string_value(
+        f64::from_bits(TEST_TIMER_ARG_BITS.with(|slot| slot.get())),
+        second_arg_original,
+        b"detached-timer-batch",
+    );
+
+    crate::timer::clearTimeout(first_id);
+    crate::timer::clearTimeout(second_id);
+}
+
 #[test]
 fn test_next_tick_previous_context_survives_hook_gc() {
     const ALS_HANDLE: i64 = -8_502;

crates/perry-runtime/src/timer.rs

@@ -1116,24 +1116,55 @@ pub extern "C" fn js_callback_timer_tick() -> i32 {
     // #6287: timers phase (by deadline) before check phase (FIFO immediates).
     order_expired_callback_batch(&mut expired);
 
+    // #8036: draining removes the WHOLE expired batch from CALLBACK_TIMERS
+    // before the first callback runs. A callback can run arbitrary JS and the
+    // microtask checkpoint below can collect, so rooting only the timer being
+    // dispatched leaves every later callback, argument, and captured async
+    // context sitting in an ordinary Rust Vec the collector cannot see. With
+    // concurrent request timers, the first resolve callback's checkpoint moved
+    // the next resolve closure and the second timer called its from-space
+    // address (`TypeError: value is not a function`). Protect the complete
+    // detached batch for the complete dispatch loop.
+    let batch_scope = crate::gc::RuntimeHandleScope::new();
+    let callback_handles: Vec<_> = expired
+        .iter()
+        .map(|timer| {
+            batch_scope.root_raw_const_ptr(timer.callback as *const crate::closure::ClosureHeader)
+        })
+        .collect();
+    let arg_handles: Vec<_> = expired
+        .iter()
+        .map(|timer| batch_scope.root_nanbox_f64_slice(&timer.args))
+        .collect();
+    let context_roots: Vec<_> = expired
+        .iter()
+        .map(|timer| crate::async_context::root_snapshot(&batch_scope, &timer.context))
+        .collect();
+
     let mut fired = 0;
     // Call the callbacks, forwarding any trailing args captured at
     // `setTimeout(fn, delay, ...args)` time. Refs #665.
-    for timer in expired {
+    for (index, mut timer) in expired.into_iter().enumerate() {
         if !timer.cleared {
-            let scope = crate::gc::RuntimeHandleScope::new();
-            let cb_handle =
-                scope.root_raw_const_ptr(timer.callback as *const crate::closure::ClosureHeader);
-            let arg_handles = scope.root_nanbox_f64_slice(&timer.args);
+            crate::async_context::refresh_snapshot_from_roots(
+                &mut timer.context,
+                &context_roots[index],
+            );
             let previous = crate::async_context::enter_context(&timer.context);
             let mut previous = previous;
-            let previous_roots = crate::async_context::root_snapshot(&scope, &previous);
+            let previous_roots = crate::async_context::root_snapshot(&batch_scope, &previous);
             crate::async_hooks::before(timer.async_id, timer.trigger_async_id);
-            let a = crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&arg_handles);
-            let cb = cb_handle.get_raw_const_ptr::<crate::closure::ClosureHeader>();
             let prev_this = crate::object::js_implicit_this_set(timer_handle_value(timer.id));
             enter_timer_callback_dispatch();
             with_timer_uncaught_trap(|| {
+                // Installing the timer receiver above is itself a collecting
+                // boundary. Re-read both roots inside the trap, immediately
+                // before dispatch, so this callback cannot be evacuated in
+                // between the handle read and js_closure_callN.
+                let a =
+                    crate::gc::RuntimeHandleScope::refreshed_nanbox_f64_slice(&arg_handles[index]);
+                let cb =
+                    callback_handles[index].get_raw_const_ptr::<crate::closure::ClosureHeader>();
                 match a.len() {
                     0 => {
                         js_closure_call0(cb);

crates/perry/src/commands/compile/run_pipeline.rs

@@ -4313,6 +4313,22 @@ pub fn run_with_parse_cache(
             }
             if references_interface {
                 for (src_pathbuf, src_hir) in &ctx.native_modules {
+                    // #8036: this augmentation supplies FOREIGN implementors
+                    // to a consumer's polymorphic dispatch tower. Feeding the
+                    // module its own exported classes back through
+                    // `imported_classes` creates a second, imported-constructor
+                    // identity for each local class. `lower_new` then treats a
+                    // local class as cross-module and calls its standalone
+                    // constructor metadata instead of the local implicit-super
+                    // path. That is observably wrong for native-backed derived
+                    // classes such as Next's ReadonlyURLSearchParams: the local
+                    // path installs the URLSearchParams backing, while the
+                    // accidental self-import path loses it. Local classes are
+                    // already present in codegen's class table, so they must
+                    // never be added by this foreign-class fallback.
+                    if src_pathbuf == path {
+                        continue;
+                    }
                     let src_path = src_pathbuf.to_string_lossy().to_string();
                     for class in &src_hir.classes {
                         if !class.is_exported {

test-files/fixtures/gc_next_request_import/lazy_work.ts

@@ -0,0 +1,7 @@
+export function checksum(iterations: number): number {
+  let value = 0x811c9dc5;
+  for (let index = 0; index < iterations; index += 1) {
+    value = Math.imul(value ^ index, 0x01000193) >>> 0;
+  }
+  return value;
+}

test-files/fixtures/gc_next_request_import/request.ts

@@ -0,0 +1,37 @@
+export class ReadonlyFixtureSearchParams extends URLSearchParams {
+  set(_name: string, _value: string): void {
+    throw new TypeError("readonly search params");
+  }
+}
+
+export class FixtureNextUrl {
+  readonly pathname: string;
+  readonly searchParams: ReadonlyFixtureSearchParams;
+
+  constructor(url: string) {
+    const parsed = new URL(url);
+    this.pathname = parsed.pathname;
+    this.searchParams = new ReadonlyFixtureSearchParams(parsed.search);
+  }
+}
+
+export class FixtureNextRequest {
+  readonly method: string;
+  readonly url: string;
+  readonly nextUrl: FixtureNextUrl;
+  readonly headers: Headers;
+  private readonly requestBody: string;
+
+  constructor(method: string, url: string, id: string, requestBody = "") {
+    this.method = method;
+    this.url = url;
+    this.nextUrl = new FixtureNextUrl(url);
+    this.headers = new Headers({ "x-request-id": id });
+    this.requestBody = requestBody;
+  }
+
+  async text(): Promise<string> {
+    await Promise.resolve();
+    return this.requestBody;
+  }
+}

test-files/fixtures/gc_next_request_import/route.ts

@@ -0,0 +1 @@
+export { GET, POST, syncSummary } from "./route_impl.ts";

test-files/fixtures/gc_next_request_import/route_impl.ts

@@ -0,0 +1,60 @@
+import type { FixtureNextRequest } from "./request.ts";
+
+function churn(seed: number): number {
+  const allocations: Array<{ index: number; label: string; pad: number[] }> = [];
+  for (let index = 0; index < 300; index += 1) {
+    allocations.push({
+      index,
+      label: "request-churn-" + seed + "-" + index,
+      pad: [seed, index, seed + index],
+    });
+  }
+  return allocations.length;
+}
+
+function snapshot(request: FixtureNextRequest): string {
+  return [
+    request.method,
+    request.url,
+    request.nextUrl.pathname,
+    request.nextUrl.searchParams.get("id"),
+    request.nextUrl.searchParams.get("iterations"),
+    request.headers.get("x-request-id"),
+  ].join("|");
+}
+
+async function handle(request: FixtureNextRequest): Promise<Record<string, unknown>> {
+  const beforeAwait = snapshot(request);
+  const { checksum } = await import("./lazy_work.ts");
+  const iterations = Number(request.nextUrl.searchParams.get("iterations"));
+  const checksumValue = checksum(iterations);
+  const allocationCount = churn(iterations);
+  await new Promise<void>((resolve) => setTimeout(resolve, 1));
+  const afterAwait = snapshot(request);
+  const requestBody = request.method === "POST" ? await request.text() : "";
+  const method = request.method;
+  const pathname = request.nextUrl.pathname;
+  const header = request.headers.get("x-request-id");
+  const id = request.nextUrl.searchParams.get("id");
+
+  return {
+    beforeAwait,
+    afterAwait,
+    method,
+    pathname,
+    id,
+    header,
+    iterations,
+    checksum: checksumValue,
+    allocationCount,
+    requestBody,
+  };
+}
+
+export function syncSummary(request: FixtureNextRequest): string {
+  churn(17);
+  return snapshot(request);
+}
+
+export const GET = handle;
+export const POST = handle;