Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions changelog.d/8044-next-request-import-rooting.md
Original file line number Diff line number Diff line change
@@ -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.
54 changes: 38 additions & 16 deletions crates/perry-codegen/src/lower_call/extern_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion crates/perry-runtime/src/gc/tests/runtime_roots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
47 changes: 39 additions & 8 deletions crates/perry-runtime/src/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions crates/perry/src/commands/compile/run_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions test-files/fixtures/gc_next_request_import/lazy_work.ts
Original file line number Diff line number Diff line change
@@ -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;
}
37 changes: 37 additions & 0 deletions test-files/fixtures/gc_next_request_import/request.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
1 change: 1 addition & 0 deletions test-files/fixtures/gc_next_request_import/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { GET, POST, syncSummary } from "./route_impl.ts";
60 changes: 60 additions & 0 deletions test-files/fixtures/gc_next_request_import/route_impl.ts
Original file line number Diff line number Diff line change
@@ -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;
48 changes: 48 additions & 0 deletions test-files/test_gap_gc_next_request_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// #8036: preserve a production-shaped request object when a generated route
// module re-exports its imported user handler. The request and all of its
// nested state must remain live while the callee allocates, dynamically
// imports a module, resumes a promise, resumes a timer, and reads the body.
// The GC moving-witness matrix registers this test below and runs it with
// forced/verified evacuation so a stale-but-unreused pointer cannot pass.

import { GET, POST, syncSummary } from "./fixtures/gc_next_request_import/route.ts";
import { FixtureNextRequest } from "./fixtures/gc_next_request_import/request.ts";

function makeRequest(id: string, iterations: number, method = "GET", body = "") {
return new FixtureNextRequest(
method,
"https://perry.invalid/api/benchmark?id=" + id + "&iterations=" + iterations,
id,
body,
);
}

const syncRequest = makeRequest("sync-request", 17);
console.log("sync", syncSummary(syncRequest));

// Mirror the production verifier's 20 concurrent GETs without making this
// request-boundary regression depend on Promise.all's separate array-forwarding
// contract. Starting every handler before awaiting the first result also makes
// cross-request ID/header swaps observable.
const pending: Array<Promise<Record<string, unknown>>> = [];
for (let index = 0; index < 20; index += 1) {
pending.push(GET(makeRequest("request-" + index, index + 1)));
}

// Perry exposes an explicit collector hook; Node does not unless launched with
// --expose-gc. Under the matrix's force/verify arm this evacuates the request
// graphs while every handler is suspended and all 20 timers are still queued.
const collect = (globalThis as unknown as { gc?: () => void }).gc;
if (collect) {
collect();
}

for (const result of pending) {
console.log(JSON.stringify(await result));
}

console.log(
JSON.stringify(
await POST(makeRequest("post-request", 31, "POST", "perry-request-body")),
),
);
Loading
Loading