From bddba825d141942a01dbebff5c79ba76e081f0c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 24 May 2026 17:00:15 +0200 Subject: [PATCH] fix(#1663): --debug-symbols retains symbols on Linux/macOS + harden microtask reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-entrant-drain SIGSEGV reported in #1663 (compiled Fastify + @perryts/mysql POST handler) is fixed by b199d30c (#1675): on both macOS arm64 and Linux x86-64, the Task::Promise arm's post-callback reload of the running promise/next is emitted as a real out-of-line RuntimeHandle:: get_raw_mut_ptr call that re-resolves the GC-root handle stack, so a re-entrant microtask drain that reallocates that stack can't leave a stale slot address behind. Verified by disassembly on both targets and by repros of the reporter's shape (nested non-transformed handler closure with 3 sequential awaiting-calls after a prior completed awaiting-call; timer-based real-async; heavy allocation under forced GC evacuation) — all byte-identical to `node --experimental-strip-types`. This change addresses the reporter's other blocker and hardens the path: 1. `--debug-symbols` now retains a symbol table on Linux/macOS, not just a PDB on Windows. Previously it was a no-op off-Windows, so a SIGSEGV in a compiled service symbolized to an unreadable wall of `??`. The flag now promotes itself to the PERRY_DEBUG_SYMBOLS env var in the compile driver (single-threaded, before rayon spawns), which codegen (`-g`/DWARF), the object-cache key, and the final `strip` step already honor. A --debug-symbols build now retains js_*/user function names + file:line. 2. Defensive hardening at the historical crash site: capture async_id + trigger_async_id as plain values before invoking the callback, so async_hooks::after() no longer dereferences a reloaded promise at all. Zero-cost (async_id is immutable for the promise's life). 3. Regression test test_issue_1663_deep_async_reentry.ts: the deeper, Fastify-handler-shaped variant of the re-entrant async-resume pattern. --- .../perry-runtime/src/promise/microtasks.rs | 16 ++++++- crates/perry/src/commands/compile.rs | 16 +++++++ .../perry/src/commands/compile/post_link.rs | 4 +- crates/perry/src/commands/compile/types.rs | 8 +++- .../test_issue_1663_deep_async_reentry.ts | 46 +++++++++++++++++++ 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 test-files/test_issue_1663_deep_async_reentry.ts diff --git a/crates/perry-runtime/src/promise/microtasks.rs b/crates/perry-runtime/src/promise/microtasks.rs index 4b7b734598..9bc77cc5fe 100644 --- a/crates/perry-runtime/src/promise/microtasks.rs +++ b/crates/perry-runtime/src/promise/microtasks.rs @@ -218,16 +218,28 @@ pub extern "C" fn js_promise_run_microtasks() -> i32 { } else { None }; - crate::async_hooks::before((*promise).async_id, (*promise).trigger_async_id); + // #1663: capture async_id + trigger as plain values BEFORE + // the callback. They are immutable for the promise's life, + // and the callback can re-entrantly drain microtasks (which + // can move the promise via GC or realloc the GC-root handle + // stack). Reading `(*promise).async_id` AFTER the callback + // to feed `after()` was the exact deref that segfaulted; use + // the captured value so `after()` needs no live promise. + let async_id = (*promise).async_id; + let trigger_async_id = (*promise).trigger_async_id; + crate::async_hooks::before(async_id, trigger_async_id); let result = crate::closure::js_closure_call1(callback, value); // Keep the callback result rooted across `after()` (which // can run JS when async_hooks are active) via the value // cell, then reload promise/next from our handles — never // the TLS cells, which a re-entrant drain may have nulled. + // The reload goes through the out-of-line `get_raw_mut_ptr` + // (#1663) so it re-resolves the handle stack after the + // callback instead of reading a stale cached slot address. CURRENT_MICROTASK_VALUE.with(|c| c.set(result)); let promise = promise_handle.get_raw_mut_ptr::(); let next = next_handle.get_raw_mut_ptr::(); - crate::async_hooks::after((*promise).async_id); + crate::async_hooks::after(async_id); if let Some(t) = t1 { MT_TIME_NS_CALLBACK .fetch_add(t.elapsed().as_nanos() as u64, Ordering::Relaxed); diff --git a/crates/perry/src/commands/compile.rs b/crates/perry/src/commands/compile.rs index 7adf3fb968..bc06286fb9 100644 --- a/crates/perry/src/commands/compile.rs +++ b/crates/perry/src/commands/compile.rs @@ -129,6 +129,22 @@ pub fn run_with_parse_cache( // bleed into this build's auto-link decisions. let _ = perry_codegen::ext_registry::take_used_providers(); + // #1663: make `--debug-symbols` retain a symbol table on every native + // target, not just emit a PDB on Windows. Previously the flag was a no-op + // on Linux/macOS, so a SIGSEGV in a compiled service (e.g. the Fastify + + // @perryts/mysql crash reported in #1663) symbolized to an unreadable wall + // of `??`, making runtime crashes nearly impossible to report. The + // canonical knob for "keep symbols" is the PERRY_DEBUG_SYMBOLS env var, + // which the codegen (`-g`/DWARF), the object-cache key, and the final + // `strip` step already all honor. Promote the flag to that env var here — + // single-threaded, before module codegen spawns rayon workers — so every + // layer observes it uniformly. Only set (never unset): the flag is an + // explicit opt-in, and a `perry dev` session that asked for symbols once + // wants them for the rest of the session. + if args.debug_symbols && std::env::var_os("PERRY_DEBUG_SYMBOLS").is_none() { + std::env::set_var("PERRY_DEBUG_SYMBOLS", "1"); + } + match format { OutputFormat::Text => println!("Collecting modules..."), OutputFormat::Json => {} diff --git a/crates/perry/src/commands/compile/post_link.rs b/crates/perry/src/commands/compile/post_link.rs index b6bc612e66..389a34b29c 100644 --- a/crates/perry/src/commands/compile/post_link.rs +++ b/crates/perry/src/commands/compile/post_link.rs @@ -16,7 +16,9 @@ use super::{CompilationContext, ObjectCache}; /// compilation target whose host `strip` can't parse foreign object /// formats (iOS/visionOS/tvOS/watchOS/HarmonyOS/Android), and when /// `PERRY_DEBUG_SYMBOLS=1` is set so crash backtraces stay -/// symbolicated. +/// symbolicated. `--debug-symbols` (#1663) promotes itself to that env +/// var in the compile driver, so passing the flag also takes this skip +/// path on Linux/macOS. /// /// When `ctx.needs_plugins` is true the build uses `strip -x` to /// retain exported symbols — `dlopen`'d plugins resolve diff --git a/crates/perry/src/commands/compile/types.rs b/crates/perry/src/commands/compile/types.rs index 2321ea06cd..41e47f243f 100644 --- a/crates/perry/src/commands/compile/types.rs +++ b/crates/perry/src/commands/compile/types.rs @@ -155,7 +155,13 @@ pub struct CompileArgs { /// otherwise an unreadable wall of ``. Also skips `/OPT:ICF` /// (identical-COMDAT folding) so distinct functions don't collapse /// to one symbol in the backtrace. Larger binary; off by default. - /// (Non-Windows targets: reserved — no behavior change today.) + /// + /// On Linux/macOS (#1663) this now skips the final `strip` and emits + /// `-g` DWARF, so a SIGSEGV in a compiled service backtraces to real + /// `js_*`/user function names + `file:line` under lldb/gdb instead of + /// `??`. Implemented by promoting the flag to the `PERRY_DEBUG_SYMBOLS` + /// env var in the compile driver, which codegen, the object-cache key, + /// and the strip step already honor. #[arg(long)] pub debug_symbols: bool, diff --git a/test-files/test_issue_1663_deep_async_reentry.ts b/test-files/test_issue_1663_deep_async_reentry.ts new file mode 100644 index 0000000000..dd52b99dce --- /dev/null +++ b/test-files/test_issue_1663_deep_async_reentry.ts @@ -0,0 +1,46 @@ +// Regression test for #1663: SIGSEGV in js_promise_run_microtasks during +// async resumption — the deeper, Fastify-handler-shaped variant. +// +// This widens the original test_issue_1663_async_reentry_microtask.ts case: +// a prior COMPLETED awaiting-call (like a Fastify onRequest hook), followed +// by a non-transformed async closure (like a route handler passed to +// app.post) that performs THREE sequential awaiting-calls (the read-body → +// SELECT → INSERT chain). Each `await` re-entrantly drains the microtask +// queue; before #1675 the Task::Promise arm reloaded the running promise +// from a clobbered TLS cell and dereferenced `(*promise).async_id` (offset +// 0x30) on a NULL pointer. This shape exercises a deeper re-entrant nesting +// than the original repro. +// +// Expected output (byte-identical to `node --experimental-strip-types`): +// prior: ok +// handler: 6 +// reached + +async function awaitingCall(n: number): Promise { + await Promise.resolve(); + return n; +} + +async function runCb(cb: () => Promise) { + await cb(); +} + +async function priorCompleted() { + await Promise.resolve(); + console.log("prior: ok"); +} + +async function handler() { + await runCb(async () => { + const a = await awaitingCall(1); + const b = await awaitingCall(2); + const c = await awaitingCall(3); + console.log("handler:", a + b + c); + }); +} + +(async () => { + await runCb(priorCompleted); + await handler(); + console.log("reached"); +})();