From e7dafc919f822e47fd3fba6f7084e804f990248b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:27:42 +0000 Subject: [PATCH 1/8] Runtime inspector activation via SIGUSR1 and process._debugProcess Sending SIGUSR1 to a running bun process (or calling process._debugProcess(pid), which also works on Windows) starts the inspector, matching Node.js. This works even when the JS thread is stuck in a loop that never returns to the event loop. Mechanism: SIGUSR1 handler sem_post only (async-signal-safe) SignalInspector thread sets a flag, fires VM::notifyNeedDebuggerBreak on the main VM, wakes the event loop JSC services the trap at the next safe point in any JIT tier (SignalSender patches DFG/FTL invalidation points) and calls the per-VM callback added in oven-sh/WebKit#287 callback (JS thread) starts the inspector if requested, drains queued CDP messages, and enters Debugger::breakProgram() when a Debugger.pause was dispatched CDP delivery after activation reuses the same trap, so the debugger thread never blocks on the target. An idle target is handled by the event-loop wakeup instead. Platforms where JSC's GC owns SIGUSR1 (FreeBSD) leave the signal alone. Also: --inspect-port and --disable-sigusr1 flags; a user SIGUSR1 listener takes over the signal and hands it back when removed; inspector.open() and this path now share Debugger::start_at_runtime. Replaces #26867 and #34106. Requires oven-sh/WebKit#287. --- docs/runtime/debugger.mdx | 22 + docs/snippets/cli/run.mdx | 8 + src/jsc/Debugger.rs | 39 +- src/jsc/RuntimeInspector.rs | 398 ++++++++++++++++++ src/jsc/VM.rs | 8 + src/jsc/VirtualMachine.rs | 21 + src/jsc/bindings/BunDebugger.cpp | 234 ++++++++-- src/jsc/bindings/BunProcess.cpp | 112 ++++- src/jsc/bindings/vm/Semaphore.cpp | 40 +- src/jsc/event_loop.rs | 6 + src/jsc/lib.rs | 2 + src/options_types/context.rs | 7 + src/runtime/cli/Arguments.rs | 8 + src/runtime/cli/repl_command.rs | 6 + src/runtime/cli/run_command.rs | 12 + src/runtime/cli/test_command.rs | 6 + src/runtime/jsc_hooks.rs | 39 ++ test/js/bun/runtime-inspector/helpers.ts | 164 ++++++++ .../runtime-inspector-posix.test.ts | 149 +++++++ .../runtime-inspector.test.ts | 169 ++++++++ test/js/node/process/process.test.js | 1 - 21 files changed, 1401 insertions(+), 50 deletions(-) create mode 100644 src/jsc/RuntimeInspector.rs create mode 100644 test/js/bun/runtime-inspector/helpers.ts create mode 100644 test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts create mode 100644 test/js/bun/runtime-inspector/runtime-inspector.test.ts diff --git a/docs/runtime/debugger.mdx b/docs/runtime/debugger.mdx index 022304a8232c..23a1e97c7bab 100644 --- a/docs/runtime/debugger.mdx +++ b/docs/runtime/debugger.mdx @@ -51,6 +51,28 @@ bun --inspect=localhost:4000 server.ts bun --inspect=localhost:4000/prefix server.ts ``` +### Attaching to a running process + +A process that was started without `--inspect` can still be debugged. Sending it `SIGUSR1` starts the inspector on the spot and prints the same banner as `--inspect`, even if the process is busy in a long-running loop and never returns to the event loop. This matches Node.js. + +```sh icon="terminal" title="terminal" +kill -USR1 +``` + +`process._debugProcess(pid)` does the same from JavaScript and also works on Windows, where there is no `SIGUSR1`: + +```sh icon="terminal" title="terminal" +bun -e 'process._debugProcess(12345)' +``` + +By default the inspector listens on port `6499`. To choose the port ahead of time, start the process with `--inspect-port`; `--inspect-port=0` picks a free port. + +```sh icon="terminal" title="terminal" +bun --inspect-port=4000 server.ts # later: kill -USR1 +``` + +If your program installs its own `process.on("SIGUSR1", ...)` listener, that listener receives the signal instead; the inspector shortcut comes back once the last listener is removed. To keep `SIGUSR1` at its default action (terminating the process) and never start an inspector, pass `--disable-sigusr1`. Processes started with `--inspect`, `--inspect-brk`, or `--inspect-wait` already have an inspector and ignore `SIGUSR1`. + --- ## Debuggers diff --git a/docs/snippets/cli/run.mdx b/docs/snippets/cli/run.mdx index 440f46fff00f..6a02e78bd49c 100644 --- a/docs/snippets/cli/run.mdx +++ b/docs/snippets/cli/run.mdx @@ -138,6 +138,14 @@ bun run Activate Bun's debugger, set breakpoint on first line of code and wait + + Port for the debugger started later by `SIGUSR1` or `process._debugProcess()` (default `6499`, `0` for a free port) + + + + Do not start the debugger on `SIGUSR1`; leave the signal at its default action + + ### Dependency & Module Resolution diff --git a/src/jsc/Debugger.rs b/src/jsc/Debugger.rs index da6c7a0aa0a1..d5c74e8b7a40 100644 --- a/src/jsc/Debugger.rs +++ b/src/jsc/Debugger.rs @@ -577,21 +577,13 @@ impl Debugger { /// thread, or when the debugger thread could not be started. // HOST_EXPORT(Debugger__startNodeInspectorServer, c) pub fn start_node_inspector_server(url: &mut BunString, wait_for_connection: bool) -> bool { - // Short-lived borrows only — `Debugger::create` re-enters JS and forms its - // own `&mut VirtualMachine` (see the aliasing note on - // `wait_for_debugger_if_necessary`). - let this: &VirtualMachine = VirtualMachine::get(); - if !this.is_main_thread { + if !can_start_at_runtime() { return false; } - if this.debugger.is_some() || HAS_CREATED_DEBUGGER.load(Ordering::Relaxed) { - return false; - } - // The URL outlives the process: the debugger struct stores `'static` slices // (CLI-arena lifetimes), so leak the runtime-provided URL the same way. let url_bytes: &'static [u8] = Box::leak(url.to_utf8_bytes().into_boxed_slice()); - this.as_mut().debugger = Some(Box::new(Debugger { + start_at_runtime(Debugger { path_or_port: Some(url_bytes), wait_for_connection: if wait_for_connection { Wait::Forever @@ -600,7 +592,32 @@ pub fn start_node_inspector_server(url: &mut BunString, wait_for_connection: boo }, protocol: Protocol::NodeInspector, ..Default::default() - })); + }) +} + +/// False when an inspector is already configured (CLI `--inspect`, +/// `BUN_INSPECT`, `inspector.open()`, or an earlier runtime activation) or +/// when called off the main thread. +pub(crate) fn can_start_at_runtime() -> bool { + let this: &VirtualMachine = VirtualMachine::get(); + this.is_main_thread + && this.debugger.is_none() + && !HAS_CREATED_DEBUGGER.load(Ordering::Relaxed) +} + +/// Shared tail of every "start the inspector after startup" path +/// (`inspector.open()`, SIGUSR1 / `process._debugProcess`): install +/// `config` on the main VM, switch the transpiler to debuggable output, spawn +/// the debugger thread, and install Bun's inspector controller. Returns false +/// (leaving `vm.debugger` unset) if the thread could not be started. Caller +/// has checked [`can_start_at_runtime`]. +pub(crate) fn start_at_runtime(config: Debugger) -> bool { + // Short-lived borrows only — `Debugger::create` re-enters JS and forms its + // own `&mut VirtualMachine` (see the aliasing note on + // `wait_for_debugger_if_necessary`). + let this: &VirtualMachine = VirtualMachine::get(); + debug_assert!(can_start_at_runtime()); + this.as_mut().debugger = Some(Box::new(config)); // Frontends need positions that map back to the original source, so stop // minifying and caching transpiled output for code loaded from now on. diff --git a/src/jsc/RuntimeInspector.rs b/src/jsc/RuntimeInspector.rs new file mode 100644 index 000000000000..b2f3c9443424 --- /dev/null +++ b/src/jsc/RuntimeInspector.rs @@ -0,0 +1,398 @@ +//! Runtime Inspector Activation (SIGUSR1 / `process._debugProcess`) +//! +//! Activates the inspector at runtime, matching Node.js behaviour where +//! `kill -USR1 ` attaches a debugger to a running process. +//! +//! POSIX: a dedicated `SignalInspector` thread sleeps on an async-signal-safe +//! semaphore; the SIGUSR1 handler only posts to it. The woken thread sets a +//! flag, fires `notifyNeedDebuggerBreak` on the main VM (thread-safe; sets a +//! trap bit and starts JSC's SignalSender), and wakes the event loop for the +//! idle case. `VMTraps::handleTraps(NeedDebuggerBreak)` then invokes the +//! per-VM callback registered in `BunDebugger.cpp`, which activates the +//! inspector and (when a frontend has asked for a pause) enters +//! `Debugger::breakProgram()`. +//! +//! Windows: a named file mapping `bun-debug-handler-` holds a function +//! pointer that an external tool invokes via `CreateRemoteThread`, exactly as +//! Node.js does. + +use core::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; + +use crate::debugger::{Debugger, Mode, Wait}; +use crate::{VM, VirtualMachineRef as VirtualMachine}; + +bun_core::declare_scope!(RuntimeInspector, hidden); + +/// Default port for runtime-activated inspector. Overridden by `--inspect-port`. +const DEFAULT_INSPECTOR_PORT: &[u8] = b"6499"; + +/// Set once by [`install_if_not_already`] when this process is eligible for +/// runtime activation (not `--disable-sigusr1`, not already `--inspect`ing, +/// GC does not own SIGUSR1). Never cleared; a user SIGUSR1 listener only +/// changes the signal disposition, see [`reinstall_after_user_handler`]. +static ARMED: AtomicBool = AtomicBool::new(false); +static ACTIVATION_REQUESTED: AtomicBool = AtomicBool::new(false); + +/// The main thread's `JSC::VM*`, published by [`on_main_vm_ready`] once +/// `VirtualMachine::init` has written it, with the trap callback already +/// installed. The SignalInspector thread reads this (Acquire) instead of +/// the plain `VirtualMachine::jsc_vm` field so the cross-thread read is +/// properly ordered against the main thread's write. +static MAIN_JSC_VM: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + +unsafe extern "C" { + fn Bun__installDebuggerTrapCallback(vm: *mut VM); + fn Bun__activateRuntimeInspectorMode(); + #[cfg(unix)] + fn Bun__gcSuspendResumeSignal() -> core::ffi::c_int; +} + +/// Main thread, from `VirtualMachine::init` after `jsc_vm` is set. Installs +/// the per-VM trap callback and publishes the VM for the signal thread. +/// +/// # Safety +/// `jsc_vm` is the main thread's live `JSC::VM`, which outlives the process +/// (the main VM is never destroyed before exit). +pub unsafe fn on_main_vm_ready(jsc_vm: *mut VM) { + if !ARMED.load(Ordering::Acquire) || jsc_vm.is_null() { + return; + } + // SAFETY: per fn contract. + unsafe { Bun__installDebuggerTrapCallback(jsc_vm) }; + MAIN_JSC_VM.store(jsc_vm, Ordering::Release); +} + +/// Called from the SignalInspector thread (POSIX) or remote thread (Windows). +/// Runs in normal thread context, so calling thread-safe JSC APIs is fine. +fn request_inspector_activation() { + ACTIVATION_REQUESTED.store(true, Ordering::Release); + + // Busy-loop path: fire the trap. `notifyNeedDebuggerBreak` is + // CONCURRENT_SAFE. Null only if the signal raced `VirtualMachine::init`; + // the event-loop wakeup below still activates in that case. + let jsc_vm = MAIN_JSC_VM.load(Ordering::Acquire); + if !jsc_vm.is_null() { + VM::opaque_ref(jsc_vm).notify_need_debugger_break(); + } + + // Idle path: the JS thread is parked in epoll/kqueue and no trap check + // runs until it wakes, so kick the loop; `check_and_activate_inspector` + // picks the request up on the next tick. + if let Some(vm) = VirtualMachine::get_main_thread_vm() { + // SAFETY: main VM pointer is valid for process lifetime and + // `EventLoop::wakeup` is safe to call from any thread. + unsafe { (*(*vm).event_loop()).wakeup() }; + } +} + +/// True on platforms where JSC's GC thread-suspend/resume handler owns +/// SIGUSR1 (e.g. FreeBSD); installing our handler there would hang GC. +pub fn gc_owns_sigusr1() -> bool { + #[cfg(unix)] + { + // SAFETY: pure read of g_wtfConfig. + return unsafe { Bun__gcSuspendResumeSignal() } == libc::SIGUSR1; + } + #[allow(unreachable_code)] + false +} + +/// Called on the main thread from the event loop tick. Handles the idle-VM +/// case where the JS thread is blocked in epoll/kqueue and the trap never +/// fires. +#[inline] +pub fn check_and_activate_inspector() { + // Hot path: one relaxed load of a flag that only the SignalInspector + // thread ever writes, so no cacheline bouncing in the common case. + if !ACTIVATION_REQUESTED.load(Ordering::Relaxed) { + return; + } + if !ACTIVATION_REQUESTED.swap(false, Ordering::AcqRel) { + return; + } + if try_activate_inspector() { + // The trap callback itself was installed in `on_main_vm_ready`; this + // just flips BunDebugger.cpp into trap-assisted CDP delivery, same as + // the trap path does after `Bun__tryActivateInspector`. + // SAFETY: pure C++ atomic store. + unsafe { Bun__activateRuntimeInspectorMode() }; + } +} + +/// Main thread only (event-loop tick or the trap callback). Starts the +/// inspector unless one is already configured or the VM is going away. +fn try_activate_inspector() -> bool { + // Short-lived `&` only: `start_at_runtime` re-enters the VM through the + // thread-local accessor. + let vm: &VirtualMachine = VirtualMachine::get(); + if vm.is_shutting_down || !crate::debugger::can_start_at_runtime() { + bun_core::scoped_log!(RuntimeInspector, "ignoring activation request"); + return false; + } + bun_core::scoped_log!(RuntimeInspector, "activating"); + let port = vm.inspect_port.unwrap_or(DEFAULT_INSPECTOR_PORT); + let started = crate::debugger::start_at_runtime(Debugger { + path_or_port: Some(port), + wait_for_connection: Wait::Off, + mode: Mode::Listen, + ..Default::default() + }); + if !started { + bun_core::pretty_errorln!("error: failed to start the inspector"); + bun_core::output::flush(); + } + started +} + +/// Arm runtime activation: start the platform delivery mechanism and (POSIX) +/// point SIGUSR1 at it. Idempotent. Must run before `on_main_vm_ready`. +pub fn install_if_not_already() { + if ARMED.swap(true, Ordering::AcqRel) { + return; + } + if !platform::install() { + ARMED.store(false, Ordering::Release); + } +} + +/// The last user `process.on("SIGUSR1")` listener was removed. While it was +/// registered, BunProcess.cpp had pointed the signal at its own forwarder; +/// nothing on our side was torn down (the SignalInspector thread stayed +/// parked on its semaphore), so handing the signal back is just re-applying +/// the sigaction. No-op unless this process was armed at startup. +pub fn reinstall_after_user_handler() { + if !ARMED.load(Ordering::Acquire) { + return; + } + #[cfg(unix)] + platform::install_sigaction(); +} + +/// Reset SIGUSR1 to default action for `--disable-sigusr1`. +pub fn set_default_sigusr1_action() { + #[cfg(unix)] + // SAFETY: `sigaction` with `SIG_DFL` is always valid. + unsafe { + let mut act: libc::sigaction = bun_core::ffi::zeroed(); + act.sa_sigaction = libc::SIG_DFL; + libc::sigemptyset(&raw mut act.sa_mask); + libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); + } +} + +/// Ignore SIGUSR1 when the debugger is already enabled via CLI flags. +pub fn ignore_sigusr1() { + #[cfg(unix)] + // SAFETY: `sigaction` with `SIG_IGN` is always valid. + unsafe { + let mut act: libc::sigaction = bun_core::ffi::zeroed(); + act.sa_sigaction = libc::SIG_IGN; + libc::sigemptyset(&raw mut act.sa_mask); + libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); + } +} + +#[cfg(unix)] +mod platform { + use super::*; + use core::ffi::c_void; + use core::sync::atomic::AtomicPtr; + + // Async-signal-safe semaphore (Mach on macOS, POSIX sem_t on Linux). + unsafe extern "C" { + fn Bun__Semaphore__create(value: core::ffi::c_uint) -> *mut c_void; + fn Bun__Semaphore__destroy(sem: *mut c_void); + fn Bun__Semaphore__signal(sem: *mut c_void) -> bool; + fn Bun__Semaphore__wait(sem: *mut c_void) -> bool; + } + + /// Live for the rest of the process once `install` succeeds; the thread + /// parked on it is never joined (detaching is fine, it holds nothing). + static SEMAPHORE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + + extern "C" fn sigusr1_handler(_: libc::c_int) { + // Signal context: only async-signal-safe calls allowed. `sem_post` / + // `semaphore_signal` are. + let sem = SEMAPHORE.load(Ordering::Acquire); + if !sem.is_null() { + // SAFETY: `sem` is live for the rest of the process (see static doc). + unsafe { Bun__Semaphore__signal(sem) }; + } + } + + fn signal_inspector_thread(sem: *mut c_void) { + bun_core::output::Source::configure_named_thread(bun_core::zstr!("SignalInspector")); + loop { + // SAFETY: `sem` is live for the rest of the process (see static doc). + unsafe { Bun__Semaphore__wait(sem) }; + bun_core::scoped_log!(RuntimeInspector, "SignalInspector woke"); + request_inspector_activation(); + } + } + + pub(super) fn install() -> bool { + // SAFETY: FFI to `new Bun::Semaphore(0)`. + let sem = unsafe { Bun__Semaphore__create(0) }; + if sem.is_null() { + bun_core::scoped_log!(RuntimeInspector, "semaphore create failed"); + return false; + } + + // `*mut` is `!Send`; the pointee is a `Bun::Semaphore`, internally + // synchronized and live for the rest of the process, so moving the + // address to the thread is sound. + struct SendPtr(*mut c_void); + // SAFETY: see above. + unsafe impl Send for SendPtr {} + let thread_sem = SendPtr(sem); + let spawn = std::thread::Builder::new() + .name("SignalInspector".to_string()) + .stack_size(512 * 1024) + .spawn(move || { + // Rebind the whole wrapper first: edition-2021 closures + // otherwise capture the `!Send` field directly. + let thread_sem = thread_sem; + signal_inspector_thread(thread_sem.0) + }); + if spawn.is_err() { + bun_core::scoped_log!(RuntimeInspector, "thread spawn failed"); + // SAFETY: `sem` was just created above; no other thread holds it. + unsafe { Bun__Semaphore__destroy(sem) }; + return false; + } + + // Publish for the signal handler only once the consumer thread exists, + // so a post can never be lost. + SEMAPHORE.store(sem, Ordering::Release); + install_sigaction(); + true + } + + pub(super) fn install_sigaction() { + // SAFETY: `sigaction` POD; all-zero is valid, fields overwritten below. + unsafe { + let mut act: libc::sigaction = bun_core::ffi::zeroed(); + act.sa_sigaction = sigusr1_handler as *const () as usize; + act.sa_flags = libc::SA_RESTART; + libc::sigemptyset(&raw mut act.sa_mask); + libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); + } + } +} + +#[cfg(windows)] +#[allow(non_camel_case_types, non_snake_case)] +mod platform { + use super::*; + use core::ffi::c_void as void; + use core::sync::atomic::AtomicPtr; + + type HANDLE = *mut void; + type DWORD = u32; + type BOOL = i32; + type LPCWSTR = *const u16; + type LPTHREAD_START_ROUTINE = unsafe extern "system" fn(*mut void) -> DWORD; + + const INVALID_HANDLE_VALUE: HANDLE = usize::MAX as HANDLE; + const PAGE_READWRITE: DWORD = 0x04; + const FILE_MAP_ALL_ACCESS: DWORD = 0xF001F; + + #[link(name = "kernel32")] + unsafe extern "system" { + fn CreateFileMappingW( + hFile: HANDLE, + lpFileMappingAttributes: *mut void, + flProtect: DWORD, + dwMaximumSizeHigh: DWORD, + dwMaximumSizeLow: DWORD, + lpName: LPCWSTR, + ) -> HANDLE; + fn MapViewOfFile( + hFileMappingObject: HANDLE, + dwDesiredAccess: DWORD, + dwFileOffsetHigh: DWORD, + dwFileOffsetLow: DWORD, + dwNumberOfBytesToMap: usize, + ) -> *mut void; + fn UnmapViewOfFile(lpBaseAddress: *const void) -> BOOL; + fn CloseHandle(hObject: HANDLE) -> BOOL; + fn GetCurrentProcessId() -> DWORD; + } + + static MAPPING_HANDLE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + + unsafe extern "system" fn start_debug_thread_proc(_: *mut void) -> DWORD { + request_inspector_activation(); + 0 + } + + pub(super) fn install() -> bool { + // SAFETY: plain Win32 calls; all pointers below are either null or + // returned by the kernel. + unsafe { + let pid = GetCurrentProcessId(); + let mut name: [u16; 64] = [0; 64]; + let s = format!("bun-debug-handler-{}", pid); + for (i, c) in s.encode_utf16().enumerate() { + if i >= 63 { + break; + } + name[i] = c; + } + + let mapping = CreateFileMappingW( + INVALID_HANDLE_VALUE, + core::ptr::null_mut(), + PAGE_READWRITE, + 0, + core::mem::size_of::() as DWORD, + name.as_ptr(), + ); + if mapping.is_null() { + bun_core::scoped_log!(RuntimeInspector, "CreateFileMappingW failed"); + return false; + } + + let view = MapViewOfFile( + mapping, + FILE_MAP_ALL_ACCESS, + 0, + 0, + core::mem::size_of::(), + ); + if view.is_null() { + bun_core::scoped_log!(RuntimeInspector, "MapViewOfFile failed"); + CloseHandle(mapping); + return false; + } + + *(view as *mut LPTHREAD_START_ROUTINE) = start_debug_thread_proc; + UnmapViewOfFile(view); + MAPPING_HANDLE.store(mapping, Ordering::Release); + true + } + } +} + +#[cfg(not(any(unix, windows)))] +mod platform { + pub(super) fn install() -> bool { + false + } +} + +/// Called from BunProcess.cpp when the last user SIGUSR1 listener is removed. +#[unsafe(no_mangle)] +pub extern "C" fn Bun__Sigusr1Handler__reinstall() { + reinstall_after_user_handler(); +} + +/// Called from the C++ debugger-trap callback on the JS thread. +/// Consumes the activation flag and activates the inspector if requested. +#[unsafe(no_mangle)] +pub extern "C" fn Bun__tryActivateInspector() -> bool { + if !ACTIVATION_REQUESTED.swap(false, Ordering::AcqRel) { + return false; + } + try_activate_inspector() +} diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index f3aee8381018..73e6daff8e48 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -33,6 +33,7 @@ unsafe extern "C" { safe fn JSC__VM__setExecutionForbidden(vm: &VM, forbidden: bool); safe fn JSC__VM__executionForbidden(vm: &VM) -> bool; safe fn JSC__VM__notifyNeedTermination(vm: &VM); + safe fn JSC__VM__notifyNeedDebuggerBreak(vm: &VM); safe fn JSC__VM__throwError(vm: &VM, global_object: &JSGlobalObject, value: JSValue); safe fn JSC__VM__releaseWeakRefs(vm: &VM); safe fn JSC__VM__drainMicrotasks(vm: &VM); @@ -118,6 +119,13 @@ impl VM { JSC__VM__notifyNeedTermination(self) } + /// Fires NeedDebuggerBreak Trap. Thread safe. The VM services it at its + /// next safe point by calling the callback installed via + /// `Bun__installDebuggerTrapCallback` (see `runtime_inspector`). + pub(crate) fn notify_need_debugger_break(&self) { + JSC__VM__notifyNeedDebuggerBreak(self) + } + /// Has termination been requested on this VM (worker.terminate(), or /// teardown's forbidExecution)? JS thread. pub fn has_termination_request(&self) -> bool { diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 0ee8a9c4075e..3438f2d54ecc 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -112,6 +112,11 @@ pub struct InitOptions { /// Forwarded as `mini_mode` to `Zig__GlobalObject__create`. For the /// main-thread path this is `smol`; for workers it is `WebWorker::mini`. pub mini_mode: bool, + /// `--disable-sigusr1`: leave SIGUSR1 at its default action instead of + /// arming the runtime-inspector handler. + pub disable_sigusr1: bool, + /// `--inspect-port`: port for the runtime-activated inspector. + pub inspect_port: Option<&'static [u8]>, } impl Default for InitOptions { @@ -129,6 +134,8 @@ impl Default for InitOptions { worker_ptr: core::ptr::null_mut(), context_id: None, mini_mode: false, + disable_sigusr1: false, + inspect_port: None, } } } @@ -342,6 +349,9 @@ pub struct VirtualMachine { pub debugger: Option>, pub(crate) has_started_debugger: bool, + /// Port for runtime inspector activation (`--inspect-port`); `None` falls + /// back to the runtime-inspector default. + pub inspect_port: Option<&'static [u8]>, pub(crate) has_terminated: bool, /// `Cell` so [`EventLoop`] (a value field of this struct) can flip the flag @@ -2559,6 +2569,11 @@ impl VirtualMachine { if opts.is_main_thread { // SAFETY: `vm` is the freshly-initialised per-thread VM singleton. bun_io::ParentDeathWatchdog::install_on_event_loop(unsafe { Self::event_loop_ctx(vm) }); + // Publishes `jsc_vm` for the SignalInspector thread; must run after + // the `(*vm).jsc_vm` write above. No-op unless the handler was + // armed by `init_runtime_state`. + // SAFETY: `jsc_vm` is this (main-thread) VM's live `JSC::VM`. + unsafe { crate::runtime_inspector::on_main_vm_ready(jsc_vm) }; } if opts.smol { @@ -3137,6 +3152,10 @@ pub struct Options { // CLI option struct lives in `bun_cli`, a forward dep). See // `runtime/jsc_hooks.rs` for the `configureDebugger` call site. pub is_main_thread: bool, + /// See [`InitOptions::disable_sigusr1`]. + pub disable_sigusr1: bool, + /// See [`InitOptions::inspect_port`]. + pub inspect_port: Option<&'static [u8]>, } /// Inherited IPC channel recorded at env load; consumed by `bun_runtime`'s @@ -3911,6 +3930,8 @@ impl VirtualMachine { mini_mode: opts.smol, eval_mode: false, is_main_thread: opts.is_main_thread, + disable_sigusr1: opts.disable_sigusr1, + inspect_port: opts.inspect_port, ..Default::default() }; let vm = Self::init(init_opts)?; diff --git a/src/jsc/bindings/BunDebugger.cpp b/src/jsc/bindings/BunDebugger.cpp index 4dc13cd2a11a..dc0106325fdd 100644 --- a/src/jsc/bindings/BunDebugger.cpp +++ b/src/jsc/bindings/BunDebugger.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include "ScriptExecutionContext.h" #include "debug-helpers.h" #include "BunInjectedScriptHost.h" @@ -29,6 +30,13 @@ using namespace JSC; using namespace WebCore; class BunInspectorConnection; +static void installRunWhilePausedCallback(JSC::JSGlobalObject*); + +// True once the inspector has been activated at runtime via SIGUSR1 / +// process._debugProcess, as opposed to --inspect at startup. When true, +// CDP message delivery additionally fires notifyNeedDebuggerBreak so +// messages reach a VM that never returns to the event loop. +static std::atomic runtimeInspectorActivated { false }; static WebCore::ScriptExecutionContext* debuggerScriptExecutionContext = nullptr; static WTF::Lock inspectorConnectionsLock = WTF::Lock(); @@ -150,13 +158,19 @@ class BunInspectorConnection : public ThreadSafeRefCountedinspectorController().connectFrontend(*this, true, false); // waitingForConnection - Inspector::JSGlobalObjectDebugger* debugger = reinterpret_cast(globalObject->debugger()); - if (debugger) { - debugger->runWhilePausedCallback = [](JSC::JSGlobalObject& globalObject, bool& isDoneProcessingEvents) -> void { - BunInspectorConnection::runWhilePaused(globalObject, isDoneProcessingEvents); - }; + // On the runtime-activation path the frontend's Debugger.enable may not + // have arrived yet, but we need a debugger attached so breakProgram() + // (from the trap callback) has a Debugger to enter the pause loop on. + // Debugger::attach() is idempotent, so the later Debugger.enable call + // only replays sourceParsed for observers. + if (runtimeInspectorActivated.load()) { + auto* ctrlDebugger = globalObject->inspectorController().debugger(); + if (ctrlDebugger && !globalObject->debugger()) + ctrlDebugger->attach(globalObject); } + installRunWhilePausedCallback(globalObject); + this->receiveMessagesOnInspectorThread(context, static_cast(globalObject), false); } @@ -185,6 +199,12 @@ class BunInspectorConnection : public ThreadSafeRefCountedinPauseLoop.load()) + this->globalObject->vm().notifyNeedDebuggerBreak(); } void disconnect() @@ -201,30 +221,42 @@ class BunInspectorConnection : public ThreadSafeRefCountedstatus == ConnectionStatus::Disconnected) - return; + connection->doDisconnect(context); + }); - connection->status = ConnectionStatus::Disconnected; + // Same reasoning as connect(): on a busy-loop target the posted task + // never runs, so let the trap callback finish the teardown. + if (runtimeInspectorActivated.load() && !this->inPauseLoop.load()) + this->globalObject->vm().notifyNeedDebuggerBreak(); + } - // Do not call .disconnect() if we never actually connected. - if (connection->hasEverConnected) { - connection->inspector().disconnect(connection.get()); - } + // Runs on the connection's owning JS thread, either from the task posted + // by disconnect() or from the debugger-trap callback. + void doDisconnect(ScriptExecutionContext& context) + { + if (this->status == ConnectionStatus::Disconnected) + return; - if (connection->unrefOnDisconnect) { - connection->unrefOnDisconnect = false; - Bun__VmHandle__refKeepAlive(WebCore::clientData(context.vm())->vmHandle, -1); - } + this->status = ConnectionStatus::Disconnected; - { - Locker locker(inspectorConnectionsLock); - if (inspectorConnections) { - auto it = inspectorConnections->find(connection->scriptExecutionContextIdentifier); - if (it != inspectorConnections->end()) - it->value.removeFirstMatching([&](auto& c) { return c.get() == connection.ptr(); }); - } + // Do not call .disconnect() if we never actually connected. + if (this->hasEverConnected) { + this->inspector().disconnect(*this); + } + + if (this->unrefOnDisconnect) { + this->unrefOnDisconnect = false; + Bun__VmHandle__refKeepAlive(WebCore::clientData(context.vm())->vmHandle, -1); + } + + { + Locker locker(inspectorConnectionsLock); + if (inspectorConnections) { + auto it = inspectorConnections->find(this->scriptExecutionContextIdentifier); + if (it != inspectorConnections->end()) + it->value.removeFirstMatching([&](auto& c) { return c.get() == this; }); } - }); + } } JSC::JSGlobalObjectDebuggable& inspector() @@ -249,6 +281,15 @@ class BunInspectorConnection : public ThreadSafeRefCountedget(global->scriptExecutionContext()->identifier())); } + // Mark connections as in the pause loop so interruptForMessageDelivery + // skips firing traps (messages are already pumped by the loop below). + for (auto& connection : connections) + connection->inPauseLoop.store(true); + auto clearInPauseLoop = WTF::makeScopeExit([&] { + for (auto& connection : connections) + connection->inPauseLoop.store(false); + }); + for (auto& connection : connections) { if (connection->status == ConnectionStatus::Pending) { connection->connect(); @@ -408,11 +449,8 @@ class BunInspectorConnection : public ThreadSafeRefCounted(globalObject->debugger()); - if (debugger) { - debugger->runWhilePausedCallback = [](JSC::JSGlobalObject& globalObject, bool& isDoneProcessingEvents) -> void { - runWhilePaused(globalObject, isDoneProcessingEvents); - }; - } + if (debugger) + installRunWhilePausedCallback(globalObject); } } } else { @@ -479,6 +517,7 @@ class BunInspectorConnection : public ThreadSafeRefCountedreceiveMessagesOnInspectorThread(context, static_cast(context.jsGlobalObject()), true); }); + this->interruptForMessageDelivery(); } } @@ -495,9 +534,25 @@ class BunInspectorConnection : public ThreadSafeRefCountedreceiveMessagesOnInspectorThread(context, static_cast(context.jsGlobalObject()), true); }); + this->interruptForMessageDelivery(); } } + // Fire a NeedDebuggerBreak trap on the JS VM so the trap callback drains + // queued CDP messages even when the JS thread never returns to the event + // loop. Only used on the runtime-activation path; with --inspect the + // event loop task posted above is sufficient. + void interruptForMessageDelivery() + { + if (!runtimeInspectorActivated.load()) + return; + // Already pumping messages in runWhilePaused; notifyPausedThread above + // woke it. A trap would be redundant and could re-enter breakProgram. + if (this->inPauseLoop.load()) + return; + this->globalObject->vm().notifyNeedDebuggerBreak(); + } + WTF::Vector debuggerThreadMessages; WTF::Lock debuggerThreadMessagesLock = WTF::Lock(); std::atomic debuggerThreadMessageScheduledCount { 0 }; @@ -512,11 +567,25 @@ class BunInspectorConnection : public ThreadSafeRefCounted status = ConnectionStatus::Pending; + // True while this connection is inside runWhilePaused. Read from the + // debugger thread to skip redundant debugger-break traps. + std::atomic inPauseLoop { false }; + bool unrefOnDisconnect = false; bool hasEverConnected = false; }; +static void installRunWhilePausedCallback(JSC::JSGlobalObject* globalObject) +{ + auto* debugger = reinterpret_cast(globalObject->debugger()); + if (debugger) { + debugger->runWhilePausedCallback = [](JSC::JSGlobalObject& go, bool& done) { + BunInspectorConnection::runWhilePaused(go, done); + }; + } +} + JSC_DECLARE_HOST_FUNCTION(jsFunctionSend); JSC_DECLARE_HOST_FUNCTION(jsFunctionDisconnect); @@ -656,12 +725,7 @@ extern "C" void Bun__ensureDebugger(ScriptExecutionContextIdentifier scriptId, b auto& inspector = globalObject->inspectorDebuggable(); inspector.setInspectable(true); - Inspector::JSGlobalObjectDebugger* debugger = reinterpret_cast(globalObject->debugger()); - if (debugger) { - debugger->runWhilePausedCallback = [](JSC::JSGlobalObject& globalObject, bool& isDoneProcessingEvents) -> void { - BunInspectorConnection::runWhilePaused(globalObject, isDoneProcessingEvents); - }; - } + installRunWhilePausedCallback(globalObject); if (pauseOnStart) { waitingForConnection = true; } @@ -1094,4 +1158,104 @@ extern "C" void Bun__InspectorConnection__disconnectAllOnExit(Zig::GlobalObject* [[maybe_unused]] auto* leakedController = globalObject->m_inspectorController.release(); globalObject->m_inspectorController = makeUnique(*globalObject, Bun::BunInjectedScriptHost::create()); } + +extern "C" bool Bun__tryActivateInspector(); + +// Called from VMTraps::handleTraps(NeedDebuggerBreak) on the JS thread at a +// safe point, after invalidateCodeBlocksOnStack. Installed on the main VM +// when the runtime-inspector signal handler is armed. +static void onDebuggerTrap(JSC::VM& vm) +{ + if (Bun__tryActivateInspector()) + runtimeInspectorActivated.store(true); + + if (!runtimeInspectorActivated.load()) + return; + + // Hold refs: doDisconnect() below removes the connection from the map, + // which may otherwise drop its last reference mid-iteration. + Vector, 8> connections; + { + Locker locker(inspectorConnectionsLock); + if (inspectorConnections) { + for (auto& entry : *inspectorConnections) { + for (auto& conn : entry.value) { + if (conn->globalObject && &conn->globalObject->vm() == &vm) + connections.append(conn); + } + } + } + } + + bool anyPaused = false; + for (auto& conn : connections) { + if (conn->inPauseLoop.load()) { + anyPaused = true; + continue; + } + auto* ctx = ScriptExecutionContext::getScriptExecutionContext(conn->scriptExecutionContextIdentifier); + if (!ctx) + continue; + switch (conn->status.load()) { + case ConnectionStatus::Pending: + conn->doConnect(*ctx); + break; + case ConnectionStatus::Disconnecting: + conn->doDisconnect(*ctx); + continue; + case ConnectionStatus::Connected: + break; + case ConnectionStatus::Disconnected: + continue; + } + conn->receiveMessagesOnInspectorThread(*ctx, static_cast(conn->globalObject), false); + } + + // runWhilePaused is already pumping messages for this VM; breakProgram() + // would be a no-op (m_isPaused), and re-entering the pause loop from + // inside a CDP dispatch (Runtime.evaluate etc.) would deadlock. + if (anyPaused) + return; + + for (auto& conn : connections) { + if (conn->status.load() != ConnectionStatus::Connected) + continue; + auto* globalObject = conn->globalObject; + if (!globalObject) + continue; + auto* debugger = globalObject->debugger(); + if (!debugger) + continue; + // Force the pause only for an explicit Debugger.pause dispatched during + // the drain above. A step-over/into/out in flight also enables stepping + // mode but must be left to reach its own target frame; pausing it here + // would stop inside the stepped-over callee. On initial SIGUSR1 + // activation with no frontend yet, nothing is requested and we just + // keep running. + if (debugger->isPauseAtNextOpportunitySet()) { + debugger->breakProgram(); + return; + } + } +} + +extern "C" void Bun__installDebuggerTrapCallback(JSC::VM* vm) +{ + ASSERT(vm); + vm->setDebuggerTrapCallback(onDebuggerTrap); +} + +extern "C" void Bun__activateRuntimeInspectorMode() +{ + runtimeInspectorActivated.store(true); +} + +extern "C" int Bun__gcSuspendResumeSignal() +{ +#if OS(WINDOWS) || OS(DARWIN) + return 0; +#else + return g_wtfConfig.sigThreadSuspendResume; +#endif +} } diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index ce39ea85cff8..ff7efdf65ca7 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -1533,6 +1533,9 @@ extern "C" void Bun__unrefChannelUnlessOverridden(JSC::JSGlobalObject* globalObj extern "C" bool Bun__shouldIgnoreOneDisconnectEventListener(JSC::JSGlobalObject* globalObject); extern "C" void Bun__ensureSignalHandler(); +#ifdef SIGUSR1 +extern "C" void Bun__Sigusr1Handler__reinstall(); +#endif extern "C" bool Bun__isMainThreadVM(); extern "C" void Bun__onPosixSignal(int signalNumber); extern "C" void Bun__onSignalListenerCountChanged(int signalNumber, int listenerCount); @@ -1647,6 +1650,8 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e }; #if !OS(WINDOWS) Bun__ensureSignalHandler(); + // For SIGUSR1 this also displaces the runtime-inspector + // activation handler; the removal path below hands it back. installForwardSignalHandler(signalNumber); #else signal_handle.handle = Bun__UVSignalHandle__init( @@ -1671,6 +1676,13 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e // Don't uninstall the old handler if it's not the one we installed. signal(signalNumber, oldHandler); } +#ifdef SIGUSR1 + // Last user listener gone: hand SIGUSR1 back to the + // runtime-inspector handler (no-op if it was never + // armed, e.g. --disable-sigusr1 or --inspect). + else if (signalNumber == SIGUSR1) + Bun__Sigusr1Handler__reinstall(); +#endif #else SignalHandleValue signal_handle = signalToContextIdsMap->get(signalNumber); Bun__UVSignalHandle__close(signal_handle.handle); @@ -4657,6 +4669,104 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionReallyKill, (JSC::JSGlobalObject * glob RELEASE_AND_RETURN(scope, JSValue::encode(jsNumber(result))); } +JSC_DEFINE_HOST_FUNCTION(Process_functionDebugProcess, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject)); + + if (callFrame->argumentCount() < 1) { + return Bun::ERR::MISSING_ARGS(scope, globalObject, "The \"pid\" argument must be specified"_s); + } + + int pid = callFrame->argument(0).toInt32(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + + if (pid <= 0) { + return Bun::ERR::INVALID_ARG_VALUE(scope, globalObject, "pid"_s, callFrame->argument(0), "must be a positive integer"_s); + } + +#if !OS(WINDOWS) + int result = kill(pid, SIGUSR1); + if (result < 0) { + throwSystemError(scope, globalObject, "kill"_s, errno); + return {}; + } +#else + // Node.js on Windows throws a plain Error whose message is the + // FormatMessageW string (see winapi_strerror in node.cc), with no .code + // or .syscall. Match that so test/js/node/test/parallel/test-debug-process.js + // passes. + auto throwWinapiError = [&](DWORD err) { + LPWSTR buf = nullptr; + DWORD n = FormatMessageW( + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPWSTR)&buf, 0, NULL); + WTF::String message; + if (buf && n > 0) { + while (n > 0 && (buf[n - 1] == L'\r' || buf[n - 1] == L'\n' || buf[n - 1] == L' ')) + n--; + message = WTF::String({ buf, n }); + } else { + message = makeString("Unknown error "_s, static_cast(err)); + } + if (buf) + LocalFree(buf); + throwVMError(globalObject, scope, message); + }; + + wchar_t mappingName[64]; + swprintf(mappingName, 64, L"bun-debug-handler-%d", pid); + + HANDLE hMapping = OpenFileMappingW(FILE_MAP_READ, FALSE, mappingName); + if (!hMapping) { + throwWinapiError(GetLastError()); + return {}; + } + + void* pFunc = MapViewOfFile(hMapping, FILE_MAP_READ, 0, 0, sizeof(void*)); + if (!pFunc) { + DWORD err = GetLastError(); + CloseHandle(hMapping); + throwWinapiError(err); + return {}; + } + + LPTHREAD_START_ROUTINE threadProc = *reinterpret_cast(pFunc); + UnmapViewOfFile(pFunc); + CloseHandle(hMapping); + + // The target writes the handler pointer after creating the named mapping; + // a reader that races the install window sees zeroed memory. Treat that + // the same as no mapping so the caller retries instead of CreateRemoteThread + // failing (or worse, succeeding with a bogus entry point). + if (!threadProc) { + throwWinapiError(ERROR_FILE_NOT_FOUND); + return {}; + } + + HANDLE hProcess = OpenProcess(PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION | PROCESS_VM_OPERATION | PROCESS_VM_WRITE | PROCESS_VM_READ, FALSE, pid); + if (!hProcess) { + throwWinapiError(GetLastError()); + return {}; + } + + HANDLE hThread = CreateRemoteThread(hProcess, NULL, 0, threadProc, NULL, 0, NULL); + if (!hThread) { + DWORD err = GetLastError(); + CloseHandle(hProcess); + throwWinapiError(err); + return {}; + } + + // Wait briefly so the remote thread finishes signalling the target + // before we close the handle. + WaitForSingleObject(hThread, 1000); + CloseHandle(hThread); + CloseHandle(hProcess); +#endif + + return JSValue::encode(jsUndefined()); +} + JSC_DEFINE_HOST_FUNCTION(Process_functionKill, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject)); @@ -4809,7 +4919,7 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu /* Source for Process.lut.h @begin processObjectTable _debugEnd Process_stubEmptyFunction Function 0 - _debugProcess Process_stubEmptyFunction Function 0 + _debugProcess Process_functionDebugProcess Function 1 _eval processGetEval CustomAccessor _getActiveHandles Process_stubFunctionReturningArray Function 0 _getActiveRequests Process_stubFunctionReturningArray Function 0 diff --git a/src/jsc/bindings/vm/Semaphore.cpp b/src/jsc/bindings/vm/Semaphore.cpp index f1eec7ef05fc..186a187f9725 100644 --- a/src/jsc/bindings/vm/Semaphore.cpp +++ b/src/jsc/bindings/vm/Semaphore.cpp @@ -1,5 +1,9 @@ #include "Semaphore.h" +#if !OS(WINDOWS) && !OS(DARWIN) +#include +#endif + namespace Bun { Semaphore::Semaphore(unsigned int value) @@ -42,10 +46,42 @@ bool Semaphore::wait() uv_sem_wait(&m_semaphore); return true; #elif OS(DARWIN) - return semaphore_wait(m_semaphore) == KERN_SUCCESS; + kern_return_t result; + while ((result = semaphore_wait(m_semaphore)) != KERN_SUCCESS) { + if (result != KERN_ABORTED) + return false; + } + return true; #else - return sem_wait(&m_semaphore) == 0; + while (sem_wait(&m_semaphore) != 0) { + if (errno != EINTR) + return false; + } + return true; #endif } } // namespace Bun + +extern "C" { + +Bun::Semaphore* Bun__Semaphore__create(unsigned int value) +{ + return new Bun::Semaphore(value); +} + +void Bun__Semaphore__destroy(Bun::Semaphore* sem) +{ + delete sem; +} + +bool Bun__Semaphore__signal(Bun::Semaphore* sem) +{ + return sem->signal(); +} + +bool Bun__Semaphore__wait(Bun::Semaphore* sem) +{ + return sem->wait(); +} +} diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index aa9643fe10ef..0ddbbcc17da4 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -500,6 +500,12 @@ impl EventLoop { pub fn tick_concurrent_with_count(&mut self) -> usize { self.apply_concurrent_ref_delta(); + // SAFETY: `vm()` returns the live owning VM; `is_main_thread` is a + // zero-valid bool set in `VirtualMachine::init`. + if unsafe { (*self.vm()).is_main_thread } { + crate::runtime_inspector::check_and_activate_inspector(); + } + #[cfg(unix)] { if let Some(signal_handler) = self.signal_handler { diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index b59ace1f259f..f5a0c2f3b98b 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -426,6 +426,8 @@ pub mod posix_signal_handle; pub mod resolve_path_jsc; #[path = "resolver_jsc.rs"] pub mod resolver_jsc; +#[path = "RuntimeInspector.rs"] +pub mod runtime_inspector; #[path = "virtual_machine_exports.rs"] pub mod virtual_machine_exports; diff --git a/src/options_types/context.rs b/src/options_types/context.rs index 4190731b8ed8..85e2856839cb 100644 --- a/src/options_types/context.rs +++ b/src/options_types/context.rs @@ -560,6 +560,11 @@ pub struct RuntimeOptions { pub cron_period: Box<[u8]>, pub cpu_prof: CpuProf, pub heap_prof: HeapProf, + /// `--disable-sigusr1`: leave SIGUSR1 at its default action instead of + /// arming the runtime-inspector handler. + pub disable_sigusr1: bool, + /// `--inspect-port`: port for the runtime-activated inspector. + pub inspect_port: Option>, } #[derive(Default)] @@ -627,6 +632,8 @@ impl Default for RuntimeOptions { cron_period: Box::default(), cpu_prof: CpuProf::default(), heap_prof: HeapProf::default(), + disable_sigusr1: false, + inspect_port: None, } } } diff --git a/src/runtime/cli/Arguments.rs b/src/runtime/cli/Arguments.rs index 0ace6e3b893f..e4cc2667fd0d 100644 --- a/src/runtime/cli/Arguments.rs +++ b/src/runtime/cli/Arguments.rs @@ -188,6 +188,12 @@ const RUNTIME_PARAMS_: &[ParamType] = &[ parse_param!( "--inspect-brk ? Activate Bun's debugger, set breakpoint on first line of code and wait" ), + parse_param!( + "--inspect-port Port for the debugger started by SIGUSR1 / process._debugProcess() (default 6499, 0 for a free port)" + ), + parse_param!( + "--disable-sigusr1 Do not start the debugger on SIGUSR1; leave the signal at its default action" + ), parse_param!( "--cpu-prof Start CPU profiler and write profile to disk on exit" ), @@ -1216,6 +1222,8 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result::from); if args.flag(b"--expose-internals") { // Same gate the env var `BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING` // sets (VirtualMachine::configure_from_env): allows resolving diff --git a/src/runtime/cli/repl_command.rs b/src/runtime/cli/repl_command.rs index 061ac4de2b1a..b0361d90b1c8 100644 --- a/src/runtime/cli/repl_command.rs +++ b/src/runtime/cli/repl_command.rs @@ -81,6 +81,12 @@ impl ReplCommand { smol: ctx.runtime_options.smol, eval_mode: true, is_main_thread: true, + disable_sigusr1: ctx.runtime_options.disable_sigusr1, + inspect_port: ctx + .runtime_options + .inspect_port + .as_deref() + .map(crate::cli::cli_dupe), ..Default::default() })?; diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index aa99cd121241..35038ab694e7 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -944,6 +944,12 @@ Full documentation is available at https://bun.com/docs/cli/run mini_mode: ctx.runtime_options.smol, eval_mode: ctx.runtime_options.eval.eval_and_print, is_main_thread: true, + disable_sigusr1: ctx.runtime_options.disable_sigusr1, + inspect_port: ctx + .runtime_options + .inspect_port + .as_deref() + .map(crate::cli::cli_dupe), ..Default::default() })?; // SAFETY: `init` returns the unique freshly-boxed VM on this thread. @@ -1165,6 +1171,12 @@ Full documentation is available at https://bun.com/docs/cli/run dns_result_order: bun_dns::Order::from_string_or_die( &ctx.runtime_options.dns_result_order, ) as u8, + disable_sigusr1: ctx.runtime_options.disable_sigusr1, + inspect_port: ctx + .runtime_options + .inspect_port + .as_deref() + .map(crate::cli::cli_dupe), ..Default::default() })?; // SAFETY: `init_with_module_graph` returns the unique freshly-boxed VM diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index a682e399fa12..490e00770998 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2304,6 +2304,12 @@ impl TestCommand { store_fd: true, smol: ctx.runtime_options.smol, is_main_thread: true, + disable_sigusr1: ctx.runtime_options.disable_sigusr1, + inspect_port: ctx + .runtime_options + .inspect_port + .as_deref() + .map(crate::cli::cli_dupe), ..Default::default() })? }; diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 60b81f6a0412..f097d8830378 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -553,11 +553,50 @@ unsafe fn init_runtime_state( if opts.worker_ptr.is_null() { // SAFETY: `vm` is the freshly-boxed unique VM on this thread. unsafe { configure_debugger(vm, &opts.debugger) }; + // SAFETY: `vm` unique; `jsc_vm`/`debugger` written above. + unsafe { configure_sigusr1_handler(vm, opts) }; } Ok(state.cast()) } +/// Install (or suppress) the SIGUSR1 runtime-inspector handler on the main +/// thread. Must run after [`configure_debugger`] so the `vm.debugger.is_some()` +/// check reflects `--inspect*` flags. +/// +/// The per-VM trap callback is NOT installed here: `init_runtime_state` fires +/// before `vm.jsc_vm` is set (same ordering as the `ParentDeathWatchdog` note +/// above). `VirtualMachine::init` calls `runtime_inspector::on_main_vm_ready` +/// once `jsc_vm` exists, which installs it if this function armed the handler. +/// +/// # Safety +/// `vm` is the freshly-boxed unique VM on this thread. +unsafe fn configure_sigusr1_handler(vm: *mut VirtualMachine, opts: &InitOptions) { + // SAFETY: per fn contract. + if !unsafe { (*vm).is_main_thread } { + return; + } + use bun_jsc::runtime_inspector; + // SAFETY: per fn contract. + unsafe { (*vm).inspect_port = opts.inspect_port }; + // On platforms where JSC's GC uses SIGUSR1 for thread suspend/resume + // (e.g. FreeBSD), replacing that handler would hang the first + // conservative stack scan. Leave the GC handler in place; runtime + // activation via `process._debugProcess` (Windows-style file mapping is + // not available here either) is simply unsupported on those platforms. + if runtime_inspector::gc_owns_sigusr1() { + return; + } + if opts.disable_sigusr1 { + runtime_inspector::set_default_sigusr1_action(); + // SAFETY: per fn contract; `debugger` written by `configure_debugger`. + } else if unsafe { (*vm).debugger.is_some() } { + runtime_inspector::ignore_sigusr1(); + } else { + runtime_inspector::install_if_not_already(); + } +} + /// Translate the CLI flag / /// `BUN_INSPECT*` env vars into `vm.debugger = Some(Debugger { .. })` so /// `ensure_debugger` (below) actually starts the inspector. diff --git a/test/js/bun/runtime-inspector/helpers.ts b/test/js/bun/runtime-inspector/helpers.ts new file mode 100644 index 000000000000..b3cefc47ca14 --- /dev/null +++ b/test/js/bun/runtime-inspector/helpers.ts @@ -0,0 +1,164 @@ +import type { Subprocess } from "bun"; +import { expect } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +/** Inspector activation involves a thread handoff plus a server start; this bounds each wait. */ +export const STREAM_TIMEOUT_MS = 30_000; + +/** + * Reads `reader` until `condition(accumulated)` holds or the stream ends. + * Each read is raced against one timer so a child that is alive but silent + * fails with whatever it did print, instead of hanging to the test timeout. + */ +export async function readStreamUntil( + reader: ReadableStreamDefaultReader, + condition: (output: string) => boolean, + timeoutMs = STREAM_TIMEOUT_MS, +): Promise { + const decoder = new TextDecoder(); + let output = ""; + let timer!: ReturnType; + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`Timed out after ${timeoutMs}ms waiting for stream condition. Got: ${JSON.stringify(output)}`)), + timeoutMs, + ); + timer.unref(); + }); + timeout.catch(() => {}); + + try { + while (!condition(output)) { + const { value, done } = await Promise.race([reader.read(), timeout]); + if (done) break; + output += decoder.decode(value, { stream: true }); + } + return output + decoder.decode(); + } finally { + clearTimeout(timer); + } +} + +/** Drains `reader` to EOF, appending to `prefix`. Use after killing the child. */ +export async function readStreamToEnd(reader: ReadableStreamDefaultReader, prefix = ""): Promise { + const decoder = new TextDecoder(); + let output = prefix; + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + output += decoder.decode(value, { stream: true }); + } + return output + decoder.decode(); +} + +/** The activation banner prints "Bun Inspector" in both its header and footer rule. */ +export function countBanners(stderr: string): number { + return (stderr.match(/Bun Inspector/g) ?? []).length / 2; +} + +export function hasBanner(stderr: string): boolean { + return countBanners(stderr) >= 1; +} + +/** Reads the target's stderr until one full banner has been printed. */ +export async function waitForBanner(proc: Subprocess): Promise { + const reader = proc.stderr.getReader(); + try { + return await readStreamUntil(reader, hasBanner); + } finally { + reader.releaseLock(); + } +} + +/** + * Spawns bun running `script` with a random inspector port and returns it once + * it has printed its pid on the first stdout line, i.e. once JS is executing. + * `script` must `console.log(process.pid)` first. + */ +export async function spawnTarget(script: string, extraArgs: string[] = []) { + const proc = Bun.spawn({ + cmd: [bunExe(), "--inspect-port=0", ...extraArgs, "-e", script], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const reader = proc.stdout.getReader(); + let first: string; + try { + first = await readStreamUntil(reader, s => s.includes("\n")); + } finally { + reader.releaseLock(); + } + const pid = parseInt(first, 10); + expect(pid).toBeGreaterThan(0); + return { proc, pid }; +} + +/** Runs `process._debugProcess(pid)` in a separate bun and asserts it succeeded. */ +export async function debugProcess(pid: number): Promise { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `process._debugProcess(${pid})`], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ exitCode, hasError: stderr.includes("error:") }).toEqual({ exitCode: 0, hasError: false }); +} + +/** + * Bun.serve binds "localhost" to whichever family resolves first, which on + * some CI hosts is ::1 while the WebSocket client tries 127.0.0.1 first. + */ +export function connectInspector(url: string): Promise { + const attempt = (u: string) => + new Promise((resolve, reject) => { + const ws = new WebSocket(u); + ws.onopen = () => resolve(ws); + ws.onerror = e => reject(e); + }); + return attempt(url).catch(() => attempt(url.replace("localhost", "[::1]"))); +} + +export function wsUrlFromBanner(stderr: string): string { + const match = stderr.match(/ws:\/\/\S+/); + expect(match).not.toBeNull(); + return match![0]; +} + +/** Minimal request/response CDP client over `ws`; `onEvent` sees notifications. */ +export function cdpClient(ws: WebSocket, onEvent?: (msg: any) => void) { + let nextId = 1; + const pending = new Map void>(); + ws.onmessage = event => { + const msg = JSON.parse(event.data as string); + if (msg.id !== undefined) { + pending.get(msg.id)?.(msg); + pending.delete(msg.id); + } else { + onEvent?.(msg); + } + }; + return function send(method: string, params: Record = {}): Promise { + const id = nextId++; + const { promise, resolve } = Promise.withResolvers(); + pending.set(id, resolve); + ws.send(JSON.stringify({ id, method, params })); + return withTimeout(`response to ${method}`, promise); + }; +} + +export function withTimeout(label: string, p: Promise, ms = 20_000): Promise { + let timer!: ReturnType; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Timed out after ${ms}ms waiting for ${label}`)), ms); + timer.unref(); + }); + timeout.catch(() => {}); + return Promise.race([p, timeout]).finally(() => clearTimeout(timer)); +} + +/** Keeps the event loop alive without doing anything. */ +export const IDLE = `console.log(process.pid); setInterval(() => {}, 1000);`; +/** Never returns to the event loop; only a trap can interrupt it. */ +export const BUSY_LOOP = `console.log(process.pid); while (true) {}`; diff --git a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts new file mode 100644 index 000000000000..e275be1ee676 --- /dev/null +++ b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows } from "harness"; +import { + cdpClient, + connectInspector, + countBanners, + hasBanner, + IDLE, + readStreamToEnd, + readStreamUntil, + spawnTarget, + waitForBanner, + wsUrlFromBanner, +} from "./helpers"; + +describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { + test("kill -USR1 activates the inspector", async () => { + const { proc, pid } = await spawnTarget(IDLE); + await using _ = proc; + + process.kill(pid, "SIGUSR1"); + + expect(await waitForBanner(proc)).toMatch(/ws:\/\/localhost:\d+\//); + }); + + test("a process can signal itself", async () => { + // The handler then runs on the JS thread itself, while it is still inside + // kill(2); setImmediate runs after startup has armed the handler. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "--inspect-port=0", + "-e", + `setImmediate(() => process.kill(process.pid, "SIGUSR1")); setInterval(() => {}, 1000);`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + expect(await waitForBanner(proc)).toMatch(/ws:\/\/localhost:\d+\//); + }); + + test("a second SIGUSR1 does not start a second inspector", async () => { + const { proc, pid } = await spawnTarget(IDLE); + await using _ = proc; + + process.kill(pid, "SIGUSR1"); + const reader = proc.stderr.getReader(); + let stderr = await readStreamUntil(reader, hasBanner); + + process.kill(pid, "SIGUSR1"); + // A CDP round trip proves the target's event loop has turned since the + // second signal landed, so a second banner would be visible by now. + const ws = await connectInspector(wsUrlFromBanner(stderr)); + try { + const result = await cdpClient(ws)("Runtime.evaluate", { expression: "6 * 7" }); + expect(result.result.result.value).toBe(42); + } finally { + ws.close(); + } + + proc.kill(); + stderr = await readStreamToEnd(reader, stderr); + reader.releaseLock(); + + expect(countBanners(stderr)).toBe(1); + }); + + test("a user SIGUSR1 listener takes precedence", async () => { + const { proc, pid } = await spawnTarget( + `console.log(process.pid); + let n = 0; + process.on("SIGUSR1", () => { console.log("user " + ++n); if (n === 3) process.exit(0); }); + setInterval(() => {}, 1000);`, + ); + await using _ = proc; + + const reader = proc.stdout.getReader(); + let stdout = ""; + for (let i = 1; i <= 3; i++) { + process.kill(pid, "SIGUSR1"); + stdout = await readStreamUntil(reader, s => s.includes(`user ${i}`)); + } + reader.releaseLock(); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect({ stdout, banners: countBanners(stderr), exitCode }).toEqual({ + stdout: "user 1\nuser 2\nuser 3\n", + banners: 0, + exitCode: 0, + }); + }); + + test("removing the last user listener hands SIGUSR1 back to the inspector", async () => { + const { proc, pid } = await spawnTarget( + `console.log(process.pid); + const onSignal = () => { console.log("user"); process.off("SIGUSR1", onSignal); console.log("removed"); }; + process.on("SIGUSR1", onSignal); + setInterval(() => {}, 1000);`, + ); + await using _ = proc; + + const stdoutReader = proc.stdout.getReader(); + process.kill(pid, "SIGUSR1"); + const stdout = await readStreamUntil(stdoutReader, s => s.includes("removed")); + stdoutReader.releaseLock(); + expect(stdout).toBe("user\nremoved\n"); + + process.kill(pid, "SIGUSR1"); + + expect(await waitForBanner(proc)).toMatch(/ws:\/\/localhost:\d+\//); + }); + + // With the inspector already configured from the command line there is + // nothing for SIGUSR1 to do; it must neither print a second banner nor + // terminate the process (the default action). + test.each(["--inspect=0", "--inspect-wait=0", "--inspect-brk=0"])("SIGUSR1 is ignored under %s", async flag => { + await using proc = Bun.spawn({ + cmd: [bunExe(), flag, "-e", IDLE], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const reader = proc.stderr.getReader(); + let stderr = await readStreamUntil(reader, hasBanner); + + process.kill(proc.pid, "SIGUSR1"); + // Any CDP connection works as the "loop has turned" ack; under -wait/-brk + // this is also what lets the target proceed to exit cleanly below. + const ws = await connectInspector(wsUrlFromBanner(stderr)); + try { + const result = await cdpClient(ws)("Runtime.evaluate", { expression: "6 * 7" }); + expect(result.result.result.value).toBe(42); + } finally { + ws.close(); + } + + proc.kill(); + stderr = await readStreamToEnd(reader, stderr); + reader.releaseLock(); + await proc.exited; + + expect({ banners: countBanners(stderr), signalCode: proc.signalCode }).toEqual({ + banners: 1, + signalCode: "SIGTERM", + }); + }); +}); diff --git a/test/js/bun/runtime-inspector/runtime-inspector.test.ts b/test/js/bun/runtime-inspector/runtime-inspector.test.ts new file mode 100644 index 000000000000..3563c7366683 --- /dev/null +++ b/test/js/bun/runtime-inspector/runtime-inspector.test.ts @@ -0,0 +1,169 @@ +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isASAN, isWindows } from "harness"; +import os from "node:os"; +import { + BUSY_LOOP, + cdpClient, + connectInspector, + countBanners, + debugProcess, + hasBanner, + IDLE, + readStreamToEnd, + readStreamUntil, + spawnTarget, + waitForBanner, + withTimeout, + wsUrlFromBanner, +} from "./helpers"; + +// `process._debugProcess(pid)` is the cross-platform entry point: SIGUSR1 on +// POSIX, a named file mapping + CreateRemoteThread on Windows. Everything here +// runs on every platform; SIGUSR1-specific semantics live in the -posix file. +describe.concurrent("process._debugProcess", () => { + test("activates the inspector in an idle target", async () => { + const { proc, pid } = await spawnTarget(IDLE); + await using _ = proc; + + await debugProcess(pid); + const stderr = await waitForBanner(proc); + + expect(stderr).toMatch(/ws:\/\/localhost:\d+\//); + }); + + test("activates the inspector in a target stuck in while(true)", async () => { + const { proc, pid } = await spawnTarget(BUSY_LOOP); + await using _ = proc; + + await debugProcess(pid); + const stderr = await waitForBanner(proc); + + expect(stderr).toMatch(/ws:\/\/localhost:\d+\//); + }); + + test("does not activate a second inspector", async () => { + const { proc, pid } = await spawnTarget(IDLE); + await using _ = proc; + + await debugProcess(pid); + const reader = proc.stderr.getReader(); + let stderr = await readStreamUntil(reader, hasBanner); + + await debugProcess(pid); + // A CDP round trip proves the target's event loop has turned since the + // second request was delivered, so a second banner would be visible by now. + const ws = await connectInspector(wsUrlFromBanner(stderr)); + try { + const result = await cdpClient(ws)("Runtime.evaluate", { expression: "6 * 7" }); + expect(result.result.result.value).toBe(42); + } finally { + ws.close(); + } + + proc.kill(); + stderr = await readStreamToEnd(reader, stderr); + reader.releaseLock(); + + expect(countBanners(stderr)).toBe(1); + }); + + test("CDP works after a client reconnects", async () => { + const { proc, pid } = await spawnTarget(IDLE); + await using _ = proc; + + await debugProcess(pid); + const url = wsUrlFromBanner(await waitForBanner(proc)); + + for (const [expression, expected] of [ + ["1 + 1", 2], + ["2 + 3", 5], + ] as const) { + const ws = await connectInspector(url); + try { + const result = await cdpClient(ws)("Runtime.evaluate", { expression }); + expect(result.result.result.value).toBe(expected); + } finally { + const closed = new Promise(resolve => (ws.onclose = () => resolve())); + ws.close(); + await withTimeout("websocket close", closed); + } + } + }); + + // Times out on the release+ASAN lane waiting for Debugger.paused; the + // while(true) activation test above covers trap delivery there, and the + // non-sanitizer lanes cover the full pause path. + test.skipIf(isASAN)("Debugger.pause interrupts while(true)", async () => { + const { proc, pid } = await spawnTarget(BUSY_LOOP); + await using _ = proc; + + await debugProcess(pid); + const ws = await connectInspector(wsUrlFromBanner(await waitForBanner(proc))); + try { + const paused = Promise.withResolvers(); + const send = cdpClient(ws, msg => { + if (msg.method === "Debugger.paused") paused.resolve(msg); + }); + + await send("Runtime.enable"); + await send("Debugger.enable"); + await send("Debugger.pause"); + const event = await withTimeout("Debugger.paused event", paused.promise); + expect(event.method).toBe("Debugger.paused"); + + await send("Debugger.resume"); + } finally { + ws.close(); + } + }); + + test("rejects a missing pid", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `try { process._debugProcess(); } catch (e) { console.log(e.code); }`], + env: bunEnv, + stdout: "pipe", + }); + expect(await proc.stdout.text()).toBe("ERR_MISSING_ARGS\n"); + }); + + test.skipIf(isWindows)("reports kill() failures as system errors", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`, + ], + env: bunEnv, + stdout: "pipe", + }); + expect(await proc.stdout.text()).toBe("ESRCH kill\n"); + }); + + // Node throws a plain Error carrying the Win32 message here; the vendored + // test/js/node/test/parallel/test-debug-process.js checks the exact text. + test.skipIf(!isWindows)("reports a missing target with the Win32 message", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `try { process._debugProcess(2147483646); } catch (e) { console.log(e.message); }`], + env: bunEnv, + stdout: "pipe", + }); + expect(await proc.stdout.text()).toBe("The system cannot find the file specified.\n"); + }); +}); + +describe.skipIf(isWindows).concurrent("--disable-sigusr1", () => { + test("leaves SIGUSR1 at its default action", async () => { + const { proc, pid } = await spawnTarget(IDLE, ["--disable-sigusr1"]); + await using _ = proc; + + process.kill(pid, "SIGUSR1"); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Exit status of a signal death. Bun's signalCode lookup reports macOS's + // SIGUSR1 (30) under the Linux name for 30, so compare numerically. + expect({ exitCode, banners: countBanners(stderr) }).toEqual({ + exitCode: 128 + os.constants.signals.SIGUSR1, + banners: 0, + }); + }); +}); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 5abad0834d66..91a5e29693a9 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -1251,7 +1251,6 @@ describe.concurrent(() => { // test-process-raw-debug.js. const undefinedStubs = [ "_debugEnd", - "_debugProcess", "_linkedBinding", "_startProfilerIdleNotifier", "_stopProfilerIdleNotifier", From bfa94f42a43431412abe553208650bc6f7cd76dc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:28:39 +0000 Subject: [PATCH 2/8] Pin WebKit to the oven-sh/WebKit#287 preview build To be replaced with the main autobuild once #287 lands. --- scripts/build/deps/webkit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index 37386d50099e..f030950cda15 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -3,7 +3,7 @@ * for local mode. Override via `--webkit-version=` to test a branch. * From https://github.com/oven-sh/WebKit releases. */ -export const WEBKIT_VERSION = "447082ab6897278727b44e1ba3c326ae6e1504c3"; +export const WEBKIT_VERSION = "autobuild-preview-pr-287-d55f967e"; /** * WebKit (JavaScriptCore) — the JS engine. From f366e4eb2667ba8fa8802170dc60db8a5930b02f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:32:11 +0000 Subject: [PATCH 3/8] [autofix.ci] apply automated fixes --- src/jsc/Debugger.rs | 4 +--- test/js/bun/runtime-inspector/runtime-inspector.test.ts | 6 +----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/jsc/Debugger.rs b/src/jsc/Debugger.rs index d5c74e8b7a40..cd89b2e087c3 100644 --- a/src/jsc/Debugger.rs +++ b/src/jsc/Debugger.rs @@ -600,9 +600,7 @@ pub fn start_node_inspector_server(url: &mut BunString, wait_for_connection: boo /// when called off the main thread. pub(crate) fn can_start_at_runtime() -> bool { let this: &VirtualMachine = VirtualMachine::get(); - this.is_main_thread - && this.debugger.is_none() - && !HAS_CREATED_DEBUGGER.load(Ordering::Relaxed) + this.is_main_thread && this.debugger.is_none() && !HAS_CREATED_DEBUGGER.load(Ordering::Relaxed) } /// Shared tail of every "start the inspector after startup" path diff --git a/test/js/bun/runtime-inspector/runtime-inspector.test.ts b/test/js/bun/runtime-inspector/runtime-inspector.test.ts index 3563c7366683..1c713c38ae0a 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector.test.ts @@ -128,11 +128,7 @@ describe.concurrent("process._debugProcess", () => { test.skipIf(isWindows)("reports kill() failures as system errors", async () => { await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - `try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`, - ], + cmd: [bunExe(), "-e", `try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`], env: bunEnv, stdout: "pipe", }); From 5bb5e155608bec643cdd6b8628818196a9de17fd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:39:57 +0000 Subject: [PATCH 4/8] Runtime inspector: review follow-ups - Record the SIGUSR1 disposition chosen at startup (runtime_inspector::Sigusr1) and re-apply that, rather than only the activation handler, when the last user SIGUSR1 listener is removed. Under --inspect* the signal now stays ignored after a listener add/remove cycle instead of falling back to the default action. --disable-sigusr1 leaves the inherited disposition alone, as before this change. - process._debugProcess validates the pid the same way process.kill does, so values toInt32 would wrap (2 ** 32 + 1 -> 1) and fractions are rejected instead of signalling an unrelated process. - Test helper cdpClient rejects on CDP error responses; the user-listener test accumulates stdout across signals; the Debugger.pause test runs on ASAN builds too (passes 12/12 locally on the debug ASAN build). - --inspect-port documents the [host:]port form it shares with --inspect. - Shorter comments throughout. --- docs/runtime/debugger.mdx | 5 +- docs/snippets/cli/run.mdx | 3 +- src/jsc/Debugger.rs | 15 +- src/jsc/RuntimeInspector.rs | 218 +++++++----------- src/jsc/VM.rs | 4 +- src/jsc/VirtualMachine.rs | 11 +- src/jsc/bindings/BunDebugger.cpp | 53 ++--- src/jsc/bindings/BunProcess.cpp | 39 ++-- src/options_types/context.rs | 3 - src/runtime/cli/Arguments.rs | 2 +- src/runtime/jsc_hooks.rs | 40 ++-- test/js/bun/runtime-inspector/helpers.ts | 25 +- .../runtime-inspector-posix.test.ts | 74 ++++-- .../runtime-inspector.test.ts | 28 ++- 14 files changed, 232 insertions(+), 288 deletions(-) diff --git a/docs/runtime/debugger.mdx b/docs/runtime/debugger.mdx index 23a1e97c7bab..9a5222f5c776 100644 --- a/docs/runtime/debugger.mdx +++ b/docs/runtime/debugger.mdx @@ -65,10 +65,11 @@ kill -USR1 bun -e 'process._debugProcess(12345)' ``` -By default the inspector listens on port `6499`. To choose the port ahead of time, start the process with `--inspect-port`; `--inspect-port=0` picks a free port. +By default the inspector listens on port `6499`. To choose where it listens ahead of time, start the process with `--inspect-port`, which accepts the same `port`, `host:port`, and URL prefix forms as `--inspect`; `--inspect-port=0` picks a free port. ```sh icon="terminal" title="terminal" -bun --inspect-port=4000 server.ts # later: kill -USR1 +bun --inspect-port=4000 server.ts # later: kill -USR1 +bun --inspect-port=localhost:4000 server.ts ``` If your program installs its own `process.on("SIGUSR1", ...)` listener, that listener receives the signal instead; the inspector shortcut comes back once the last listener is removed. To keep `SIGUSR1` at its default action (terminating the process) and never start an inspector, pass `--disable-sigusr1`. Processes started with `--inspect`, `--inspect-brk`, or `--inspect-wait` already have an inspector and ignore `SIGUSR1`. diff --git a/docs/snippets/cli/run.mdx b/docs/snippets/cli/run.mdx index 6a02e78bd49c..b67019625460 100644 --- a/docs/snippets/cli/run.mdx +++ b/docs/snippets/cli/run.mdx @@ -139,7 +139,8 @@ bun run - Port for the debugger started later by `SIGUSR1` or `process._debugProcess()` (default `6499`, `0` for a free port) + `[host:]port` for a debugger started later by `SIGUSR1` or `process._debugProcess()` (default `6499`, `0` for a free + port) diff --git a/src/jsc/Debugger.rs b/src/jsc/Debugger.rs index cd89b2e087c3..2c868d48d980 100644 --- a/src/jsc/Debugger.rs +++ b/src/jsc/Debugger.rs @@ -595,24 +595,15 @@ pub fn start_node_inspector_server(url: &mut BunString, wait_for_connection: boo }) } -/// False when an inspector is already configured (CLI `--inspect`, -/// `BUN_INSPECT`, `inspector.open()`, or an earlier runtime activation) or -/// when called off the main thread. +/// False off the main thread or once any inspector exists (CLI flags, env, `inspector.open()`, runtime activation). pub(crate) fn can_start_at_runtime() -> bool { let this: &VirtualMachine = VirtualMachine::get(); this.is_main_thread && this.debugger.is_none() && !HAS_CREATED_DEBUGGER.load(Ordering::Relaxed) } -/// Shared tail of every "start the inspector after startup" path -/// (`inspector.open()`, SIGUSR1 / `process._debugProcess`): install -/// `config` on the main VM, switch the transpiler to debuggable output, spawn -/// the debugger thread, and install Bun's inspector controller. Returns false -/// (leaving `vm.debugger` unset) if the thread could not be started. Caller -/// has checked [`can_start_at_runtime`]. +/// Shared by `inspector.open()` and SIGUSR1 / `process._debugProcess`; the caller has checked [`can_start_at_runtime`]. pub(crate) fn start_at_runtime(config: Debugger) -> bool { - // Short-lived borrows only — `Debugger::create` re-enters JS and forms its - // own `&mut VirtualMachine` (see the aliasing note on - // `wait_for_debugger_if_necessary`). + // `&` only: `Debugger::create` re-enters JS and forms its own `&mut VirtualMachine` (see `wait_for_debugger_if_necessary`). let this: &VirtualMachine = VirtualMachine::get(); debug_assert!(can_start_at_runtime()); this.as_mut().debugger = Some(Box::new(config)); diff --git a/src/jsc/RuntimeInspector.rs b/src/jsc/RuntimeInspector.rs index b2f3c9443424..3e6ab5dd96c3 100644 --- a/src/jsc/RuntimeInspector.rs +++ b/src/jsc/RuntimeInspector.rs @@ -1,22 +1,13 @@ -//! Runtime Inspector Activation (SIGUSR1 / `process._debugProcess`) +//! Starting the inspector in a running process (`kill -USR1 ` / `process._debugProcess(pid)`), as Node does. //! -//! Activates the inspector at runtime, matching Node.js behaviour where -//! `kill -USR1 ` attaches a debugger to a running process. -//! -//! POSIX: a dedicated `SignalInspector` thread sleeps on an async-signal-safe -//! semaphore; the SIGUSR1 handler only posts to it. The woken thread sets a -//! flag, fires `notifyNeedDebuggerBreak` on the main VM (thread-safe; sets a -//! trap bit and starts JSC's SignalSender), and wakes the event loop for the -//! idle case. `VMTraps::handleTraps(NeedDebuggerBreak)` then invokes the -//! per-VM callback registered in `BunDebugger.cpp`, which activates the -//! inspector and (when a frontend has asked for a pause) enters -//! `Debugger::breakProgram()`. -//! -//! Windows: a named file mapping `bun-debug-handler-` holds a function -//! pointer that an external tool invokes via `CreateRemoteThread`, exactly as -//! Node.js does. +//! POSIX: the SIGUSR1 handler posts a semaphore; the `SignalInspector` thread parked on it sets +//! `ACTIVATION_REQUESTED`, fires a `NeedDebuggerBreak` trap on the main VM (reaches a JS thread that +//! never returns to the event loop) and wakes the event loop (reaches an idle one). JSC services the +//! trap by calling the callback in `BunDebugger.cpp`, which calls [`Bun__tryActivateInspector`]. +//! Windows: the named mapping `bun-debug-handler-` holds a function pointer that the signalling +//! process runs in this process via `CreateRemoteThread`, Node's protocol. -use core::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicPtr, AtomicU8, Ordering}; use crate::debugger::{Debugger, Mode, Wait}; use crate::{VM, VirtualMachineRef as VirtualMachine}; @@ -26,18 +17,23 @@ bun_core::declare_scope!(RuntimeInspector, hidden); /// Default port for runtime-activated inspector. Overridden by `--inspect-port`. const DEFAULT_INSPECTOR_PORT: &[u8] = b"6499"; -/// Set once by [`install_if_not_already`] when this process is eligible for -/// runtime activation (not `--disable-sigusr1`, not already `--inspect`ing, -/// GC does not own SIGUSR1). Never cleared; a user SIGUSR1 listener only -/// changes the signal disposition, see [`reinstall_after_user_handler`]. -static ARMED: AtomicBool = AtomicBool::new(false); +/// What SIGUSR1 does while no user `process.on("SIGUSR1")` listener is registered. +#[derive(Clone, Copy, PartialEq, Eq)] +#[repr(u8)] +pub enum Sigusr1 { + /// `--disable-sigusr1`: the disposition is never touched, so the default action (terminate) applies. + Default = 0, + /// An inspector exists already (`--inspect*`), so the signal is ignored. + Ignore = 1, + /// Start the inspector; on Windows this also publishes the `process._debugProcess` mapping. + StartInspector = 2, +} + +/// The [`Sigusr1`] that [`configure`] put in place; re-applied once a user listener gets removed again. +static DISPOSITION: AtomicU8 = AtomicU8::new(Sigusr1::Default as u8); static ACTIVATION_REQUESTED: AtomicBool = AtomicBool::new(false); -/// The main thread's `JSC::VM*`, published by [`on_main_vm_ready`] once -/// `VirtualMachine::init` has written it, with the trap callback already -/// installed. The SignalInspector thread reads this (Acquire) instead of -/// the plain `VirtualMachine::jsc_vm` field so the cross-thread read is -/// properly ordered against the main thread's write. +/// Published by [`on_main_vm_ready`] with the trap callback installed; the only VM pointer the signal thread may read. static MAIN_JSC_VM: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); unsafe extern "C" { @@ -47,14 +43,12 @@ unsafe extern "C" { fn Bun__gcSuspendResumeSignal() -> core::ffi::c_int; } -/// Main thread, from `VirtualMachine::init` after `jsc_vm` is set. Installs -/// the per-VM trap callback and publishes the VM for the signal thread. +/// Main thread, from `VirtualMachine::init` once `jsc_vm` exists; no-op unless [`configure`] chose [`Sigusr1::StartInspector`]. /// /// # Safety -/// `jsc_vm` is the main thread's live `JSC::VM`, which outlives the process -/// (the main VM is never destroyed before exit). +/// `jsc_vm` is the main thread's `JSC::VM`, which is never destroyed before process exit. pub unsafe fn on_main_vm_ready(jsc_vm: *mut VM) { - if !ARMED.load(Ordering::Acquire) || jsc_vm.is_null() { + if disposition() != Sigusr1::StartInspector || jsc_vm.is_null() { return; } // SAFETY: per fn contract. @@ -62,48 +56,27 @@ pub unsafe fn on_main_vm_ready(jsc_vm: *mut VM) { MAIN_JSC_VM.store(jsc_vm, Ordering::Release); } -/// Called from the SignalInspector thread (POSIX) or remote thread (Windows). -/// Runs in normal thread context, so calling thread-safe JSC APIs is fine. +/// Ordinary thread context (the SignalInspector thread, or the thread `_debugProcess` injects on Windows). fn request_inspector_activation() { ACTIVATION_REQUESTED.store(true, Ordering::Release); - // Busy-loop path: fire the trap. `notifyNeedDebuggerBreak` is - // CONCURRENT_SAFE. Null only if the signal raced `VirtualMachine::init`; - // the event-loop wakeup below still activates in that case. + // Busy JS thread: trap it. Null only if the signal raced `VirtualMachine::init`; the wakeup below still works. let jsc_vm = MAIN_JSC_VM.load(Ordering::Acquire); if !jsc_vm.is_null() { VM::opaque_ref(jsc_vm).notify_need_debugger_break(); } - // Idle path: the JS thread is parked in epoll/kqueue and no trap check - // runs until it wakes, so kick the loop; `check_and_activate_inspector` - // picks the request up on the next tick. + // Idle JS thread parked in epoll/kqueue: nothing checks the trap until it wakes, so wake it. if let Some(vm) = VirtualMachine::get_main_thread_vm() { - // SAFETY: main VM pointer is valid for process lifetime and - // `EventLoop::wakeup` is safe to call from any thread. + // SAFETY: the main VM lives until process exit and `EventLoop::wakeup` may be called from any thread. unsafe { (*(*vm).event_loop()).wakeup() }; } } -/// True on platforms where JSC's GC thread-suspend/resume handler owns -/// SIGUSR1 (e.g. FreeBSD); installing our handler there would hang GC. -pub fn gc_owns_sigusr1() -> bool { - #[cfg(unix)] - { - // SAFETY: pure read of g_wtfConfig. - return unsafe { Bun__gcSuspendResumeSignal() } == libc::SIGUSR1; - } - #[allow(unreachable_code)] - false -} - -/// Called on the main thread from the event loop tick. Handles the idle-VM -/// case where the JS thread is blocked in epoll/kqueue and the trap never -/// fires. +/// Every main-thread event-loop tick: picks the request up when the JS thread was idle rather than trapped. #[inline] pub fn check_and_activate_inspector() { - // Hot path: one relaxed load of a flag that only the SignalInspector - // thread ever writes, so no cacheline bouncing in the common case. + // Plain load first: this runs every tick, and only the signal thread ever stores here. if !ACTIVATION_REQUESTED.load(Ordering::Relaxed) { return; } @@ -111,19 +84,15 @@ pub fn check_and_activate_inspector() { return; } if try_activate_inspector() { - // The trap callback itself was installed in `on_main_vm_ready`; this - // just flips BunDebugger.cpp into trap-assisted CDP delivery, same as - // the trap path does after `Bun__tryActivateInspector`. + // Same switch to trap-assisted CDP delivery that the trap path makes after `Bun__tryActivateInspector`. // SAFETY: pure C++ atomic store. unsafe { Bun__activateRuntimeInspectorMode() }; } } -/// Main thread only (event-loop tick or the trap callback). Starts the -/// inspector unless one is already configured or the VM is going away. +/// Main thread only. fn try_activate_inspector() -> bool { - // Short-lived `&` only: `start_at_runtime` re-enters the VM through the - // thread-local accessor. + // `&` only: `start_at_runtime` re-borrows the VM through the thread-local accessor. let vm: &VirtualMachine = VirtualMachine::get(); if vm.is_shutting_down || !crate::debugger::can_start_at_runtime() { bun_core::scoped_log!(RuntimeInspector, "ignoring activation request"); @@ -144,52 +113,39 @@ fn try_activate_inspector() -> bool { started } -/// Arm runtime activation: start the platform delivery mechanism and (POSIX) -/// point SIGUSR1 at it. Idempotent. Must run before `on_main_vm_ready`. -pub fn install_if_not_already() { - if ARMED.swap(true, Ordering::AcqRel) { +/// Main thread at startup, before [`on_main_vm_ready`]. Leaves the signal alone where JSC's GC suspends threads with it (FreeBSD). +pub fn configure(wanted: Sigusr1) { + if wanted == Sigusr1::Default || gc_owns_sigusr1() { return; } - if !platform::install() { - ARMED.store(false, Ordering::Release); + if wanted == Sigusr1::StartInspector && !platform::install() { + return; } + DISPOSITION.store(wanted as u8, Ordering::Release); + platform::apply(wanted); } -/// The last user `process.on("SIGUSR1")` listener was removed. While it was -/// registered, BunProcess.cpp had pointed the signal at its own forwarder; -/// nothing on our side was torn down (the SignalInspector thread stayed -/// parked on its semaphore), so handing the signal back is just re-applying -/// the sigaction. No-op unless this process was armed at startup. +/// BunProcess.cpp removed the last user `process.on("SIGUSR1")` listener and reset the signal to its default action. pub fn reinstall_after_user_handler() { - if !ARMED.load(Ordering::Acquire) { - return; - } - #[cfg(unix)] - platform::install_sigaction(); + platform::apply(disposition()); } -/// Reset SIGUSR1 to default action for `--disable-sigusr1`. -pub fn set_default_sigusr1_action() { - #[cfg(unix)] - // SAFETY: `sigaction` with `SIG_DFL` is always valid. - unsafe { - let mut act: libc::sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = libc::SIG_DFL; - libc::sigemptyset(&raw mut act.sa_mask); - libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); +fn disposition() -> Sigusr1 { + match DISPOSITION.load(Ordering::Acquire) { + 1 => Sigusr1::Ignore, + 2 => Sigusr1::StartInspector, + _ => Sigusr1::Default, } } -/// Ignore SIGUSR1 when the debugger is already enabled via CLI flags. -pub fn ignore_sigusr1() { +fn gc_owns_sigusr1() -> bool { #[cfg(unix)] - // SAFETY: `sigaction` with `SIG_IGN` is always valid. - unsafe { - let mut act: libc::sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = libc::SIG_IGN; - libc::sigemptyset(&raw mut act.sa_mask); - libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); + { + // SAFETY: pure read of g_wtfConfig. + return unsafe { Bun__gcSuspendResumeSignal() } == libc::SIGUSR1; } + #[allow(unreachable_code)] + false } #[cfg(unix)] @@ -198,7 +154,7 @@ mod platform { use core::ffi::c_void; use core::sync::atomic::AtomicPtr; - // Async-signal-safe semaphore (Mach on macOS, POSIX sem_t on Linux). + // `Bun::Semaphore` (vm/Semaphore.cpp): Mach semaphore on macOS, `sem_t` elsewhere; both are async-signal-safe to post. unsafe extern "C" { fn Bun__Semaphore__create(value: core::ffi::c_uint) -> *mut c_void; fn Bun__Semaphore__destroy(sem: *mut c_void); @@ -206,16 +162,14 @@ mod platform { fn Bun__Semaphore__wait(sem: *mut c_void) -> bool; } - /// Live for the rest of the process once `install` succeeds; the thread - /// parked on it is never joined (detaching is fine, it holds nothing). + /// Never destroyed once published: the signal handler and the parked thread use it for the rest of the process. static SEMAPHORE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); extern "C" fn sigusr1_handler(_: libc::c_int) { - // Signal context: only async-signal-safe calls allowed. `sem_post` / - // `semaphore_signal` are. + // Signal context: nothing but the post may happen here. let sem = SEMAPHORE.load(Ordering::Acquire); if !sem.is_null() { - // SAFETY: `sem` is live for the rest of the process (see static doc). + // SAFETY: `sem` is live for the rest of the process (see `SEMAPHORE`). unsafe { Bun__Semaphore__signal(sem) }; } } @@ -223,7 +177,7 @@ mod platform { fn signal_inspector_thread(sem: *mut c_void) { bun_core::output::Source::configure_named_thread(bun_core::zstr!("SignalInspector")); loop { - // SAFETY: `sem` is live for the rest of the process (see static doc). + // SAFETY: `sem` is live for the rest of the process (see `SEMAPHORE`). unsafe { Bun__Semaphore__wait(sem) }; bun_core::scoped_log!(RuntimeInspector, "SignalInspector woke"); request_inspector_activation(); @@ -231,6 +185,9 @@ mod platform { } pub(super) fn install() -> bool { + if !SEMAPHORE.load(Ordering::Acquire).is_null() { + return true; + } // SAFETY: FFI to `new Bun::Semaphore(0)`. let sem = unsafe { Bun__Semaphore__create(0) }; if sem.is_null() { @@ -238,19 +195,15 @@ mod platform { return false; } - // `*mut` is `!Send`; the pointee is a `Bun::Semaphore`, internally - // synchronized and live for the rest of the process, so moving the - // address to the thread is sound. struct SendPtr(*mut c_void); - // SAFETY: see above. + // SAFETY: the pointee is a `Bun::Semaphore`, internally synchronized and live for the rest of the process. unsafe impl Send for SendPtr {} let thread_sem = SendPtr(sem); let spawn = std::thread::Builder::new() .name("SignalInspector".to_string()) .stack_size(512 * 1024) .spawn(move || { - // Rebind the whole wrapper first: edition-2021 closures - // otherwise capture the `!Send` field directly. + // Captures the wrapper rather than its `!Send` field (edition 2021 disjoint captures). let thread_sem = thread_sem; signal_inspector_thread(thread_sem.0) }); @@ -261,18 +214,21 @@ mod platform { return false; } - // Publish for the signal handler only once the consumer thread exists, - // so a post can never be lost. + // Published only once the consumer thread exists, so no post is ever lost. SEMAPHORE.store(sem, Ordering::Release); - install_sigaction(); true } - pub(super) fn install_sigaction() { - // SAFETY: `sigaction` POD; all-zero is valid, fields overwritten below. + pub(super) fn apply(wanted: Sigusr1) { + let handler: libc::sighandler_t = match wanted { + Sigusr1::Default => return, + Sigusr1::Ignore => libc::SIG_IGN, + Sigusr1::StartInspector => sigusr1_handler as *const () as libc::sighandler_t, + }; + // SAFETY: `sigaction` is plain data for which all-zero is valid; `handler` is SIG_IGN or a live fn. unsafe { let mut act: libc::sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = sigusr1_handler as *const () as usize; + act.sa_sigaction = handler; act.sa_flags = libc::SA_RESTART; libc::sigemptyset(&raw mut act.sa_mask); libc::sigaction(libc::SIGUSR1, &raw const act, core::ptr::null_mut()); @@ -319,6 +275,7 @@ mod platform { fn GetCurrentProcessId() -> DWORD; } + /// Keeps the named mapping alive for the rest of the process (the view itself is unmapped right after the write). static MAPPING_HANDLE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); unsafe extern "system" fn start_debug_thread_proc(_: *mut void) -> DWORD { @@ -327,18 +284,14 @@ mod platform { } pub(super) fn install() -> bool { - // SAFETY: plain Win32 calls; all pointers below are either null or - // returned by the kernel. + if !MAPPING_HANDLE.load(Ordering::Acquire).is_null() { + return true; + } + // SAFETY: plain Win32 calls; every pointer below is null, NUL-terminated, or was returned by the kernel. unsafe { - let pid = GetCurrentProcessId(); - let mut name: [u16; 64] = [0; 64]; - let s = format!("bun-debug-handler-{}", pid); - for (i, c) in s.encode_utf16().enumerate() { - if i >= 63 { - break; - } - name[i] = c; - } + let name: Vec = format!("bun-debug-handler-{}\0", GetCurrentProcessId()) + .encode_utf16() + .collect(); let mapping = CreateFileMappingW( INVALID_HANDLE_VALUE, @@ -372,6 +325,8 @@ mod platform { true } } + + pub(super) fn apply(_: Sigusr1) {} } #[cfg(not(any(unix, windows)))] @@ -379,6 +334,8 @@ mod platform { pub(super) fn install() -> bool { false } + + pub(super) fn apply(_: super::Sigusr1) {} } /// Called from BunProcess.cpp when the last user SIGUSR1 listener is removed. @@ -387,8 +344,7 @@ pub extern "C" fn Bun__Sigusr1Handler__reinstall() { reinstall_after_user_handler(); } -/// Called from the C++ debugger-trap callback on the JS thread. -/// Consumes the activation flag and activates the inspector if requested. +/// Called by `onDebuggerTrap` (BunDebugger.cpp) on the JS thread; true if this trap started the inspector. #[unsafe(no_mangle)] pub extern "C" fn Bun__tryActivateInspector() -> bool { if !ACTIVATION_REQUESTED.swap(false, Ordering::AcqRel) { diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index 73e6daff8e48..9b8037d1ba8c 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -119,9 +119,7 @@ impl VM { JSC__VM__notifyNeedTermination(self) } - /// Fires NeedDebuggerBreak Trap. Thread safe. The VM services it at its - /// next safe point by calling the callback installed via - /// `Bun__installDebuggerTrapCallback` (see `runtime_inspector`). + /// Fires NeedDebuggerBreak Trap. Thread safe. Serviced by the callback installed via `Bun__installDebuggerTrapCallback`. pub(crate) fn notify_need_debugger_break(&self) { JSC__VM__notifyNeedDebuggerBreak(self) } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 3438f2d54ecc..02a808c2f787 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -112,10 +112,9 @@ pub struct InitOptions { /// Forwarded as `mini_mode` to `Zig__GlobalObject__create`. For the /// main-thread path this is `smol`; for workers it is `WebWorker::mini`. pub mini_mode: bool, - /// `--disable-sigusr1`: leave SIGUSR1 at its default action instead of - /// arming the runtime-inspector handler. + /// `--disable-sigusr1`: leave SIGUSR1 at its default action instead of starting the inspector on it. pub disable_sigusr1: bool, - /// `--inspect-port`: port for the runtime-activated inspector. + /// `--inspect-port`: where an inspector started by SIGUSR1 / `process._debugProcess` listens. pub inspect_port: Option<&'static [u8]>, } @@ -349,8 +348,7 @@ pub struct VirtualMachine { pub debugger: Option>, pub(crate) has_started_debugger: bool, - /// Port for runtime inspector activation (`--inspect-port`); `None` falls - /// back to the runtime-inspector default. + /// See [`InitOptions::inspect_port`]; `None` means `runtime_inspector`'s default port. pub inspect_port: Option<&'static [u8]>, pub(crate) has_terminated: bool, @@ -2569,9 +2567,6 @@ impl VirtualMachine { if opts.is_main_thread { // SAFETY: `vm` is the freshly-initialised per-thread VM singleton. bun_io::ParentDeathWatchdog::install_on_event_loop(unsafe { Self::event_loop_ctx(vm) }); - // Publishes `jsc_vm` for the SignalInspector thread; must run after - // the `(*vm).jsc_vm` write above. No-op unless the handler was - // armed by `init_runtime_state`. // SAFETY: `jsc_vm` is this (main-thread) VM's live `JSC::VM`. unsafe { crate::runtime_inspector::on_main_vm_ready(jsc_vm) }; } diff --git a/src/jsc/bindings/BunDebugger.cpp b/src/jsc/bindings/BunDebugger.cpp index dc0106325fdd..ba949d50b26b 100644 --- a/src/jsc/bindings/BunDebugger.cpp +++ b/src/jsc/bindings/BunDebugger.cpp @@ -32,10 +32,7 @@ using namespace WebCore; class BunInspectorConnection; static void installRunWhilePausedCallback(JSC::JSGlobalObject*); -// True once the inspector has been activated at runtime via SIGUSR1 / -// process._debugProcess, as opposed to --inspect at startup. When true, -// CDP message delivery additionally fires notifyNeedDebuggerBreak so -// messages reach a VM that never returns to the event loop. +// Set by SIGUSR1 / process._debugProcess activation (never by --inspect): CDP delivery then also traps the JS thread. static std::atomic runtimeInspectorActivated { false }; static WebCore::ScriptExecutionContext* debuggerScriptExecutionContext = nullptr; @@ -158,11 +155,7 @@ class BunInspectorConnection : public ThreadSafeRefCountedinspectorController().connectFrontend(*this, true, false); // waitingForConnection - // On the runtime-activation path the frontend's Debugger.enable may not - // have arrived yet, but we need a debugger attached so breakProgram() - // (from the trap callback) has a Debugger to enter the pause loop on. - // Debugger::attach() is idempotent, so the later Debugger.enable call - // only replays sourceParsed for observers. + // onDebuggerTrap's breakProgram() needs a Debugger attached before Debugger.enable arrives; attach() is idempotent. if (runtimeInspectorActivated.load()) { auto* ctrlDebugger = globalObject->inspectorController().debugger(); if (ctrlDebugger && !globalObject->debugger()) @@ -200,9 +193,7 @@ class BunInspectorConnection : public ThreadSafeRefCountedinPauseLoop.load()) this->globalObject->vm().notifyNeedDebuggerBreak(); } @@ -224,14 +215,12 @@ class BunInspectorConnection : public ThreadSafeRefCounteddoDisconnect(context); }); - // Same reasoning as connect(): on a busy-loop target the posted task - // never runs, so let the trap callback finish the teardown. + // As in connect(): a stuck target is torn down from onDebuggerTrap instead. if (runtimeInspectorActivated.load() && !this->inPauseLoop.load()) this->globalObject->vm().notifyNeedDebuggerBreak(); } - // Runs on the connection's owning JS thread, either from the task posted - // by disconnect() or from the debugger-trap callback. + // JS thread, from the task posted by disconnect() or from onDebuggerTrap. void doDisconnect(ScriptExecutionContext& context) { if (this->status == ConnectionStatus::Disconnected) @@ -281,8 +270,7 @@ class BunInspectorConnection : public ThreadSafeRefCountedget(global->scriptExecutionContext()->identifier())); } - // Mark connections as in the pause loop so interruptForMessageDelivery - // skips firing traps (messages are already pumped by the loop below). + // The loop below pumps messages itself, so interruptForMessageDelivery must not trap meanwhile. for (auto& connection : connections) connection->inPauseLoop.store(true); auto clearInPauseLoop = WTF::makeScopeExit([&] { @@ -538,16 +526,12 @@ class BunInspectorConnection : public ThreadSafeRefCountedinPauseLoop.load()) return; this->globalObject->vm().notifyNeedDebuggerBreak(); @@ -567,8 +551,7 @@ class BunInspectorConnection : public ThreadSafeRefCounted status = ConnectionStatus::Pending; - // True while this connection is inside runWhilePaused. Read from the - // debugger thread to skip redundant debugger-break traps. + // Set while inside runWhilePaused; read from the debugger thread. std::atomic inPauseLoop { false }; bool unrefOnDisconnect = false; @@ -1161,9 +1144,7 @@ extern "C" void Bun__InspectorConnection__disconnectAllOnExit(Zig::GlobalObject* extern "C" bool Bun__tryActivateInspector(); -// Called from VMTraps::handleTraps(NeedDebuggerBreak) on the JS thread at a -// safe point, after invalidateCodeBlocksOnStack. Installed on the main VM -// when the runtime-inspector signal handler is armed. +// JS thread, from VMTraps::handleTraps(NeedDebuggerBreak) at a safe point (code blocks on the stack already jettisoned). static void onDebuggerTrap(JSC::VM& vm) { if (Bun__tryActivateInspector()) @@ -1172,8 +1153,7 @@ static void onDebuggerTrap(JSC::VM& vm) if (!runtimeInspectorActivated.load()) return; - // Hold refs: doDisconnect() below removes the connection from the map, - // which may otherwise drop its last reference mid-iteration. + // Copies of the refs: doDisconnect() below removes entries from the map. Vector, 8> connections; { Locker locker(inspectorConnectionsLock); @@ -1211,9 +1191,7 @@ static void onDebuggerTrap(JSC::VM& vm) conn->receiveMessagesOnInspectorThread(*ctx, static_cast(conn->globalObject), false); } - // runWhilePaused is already pumping messages for this VM; breakProgram() - // would be a no-op (m_isPaused), and re-entering the pause loop from - // inside a CDP dispatch (Runtime.evaluate etc.) would deadlock. + // Already inside runWhilePaused: re-entering the pause loop from a CDP dispatch (Runtime.evaluate etc.) would deadlock. if (anyPaused) return; @@ -1226,12 +1204,7 @@ static void onDebuggerTrap(JSC::VM& vm) auto* debugger = globalObject->debugger(); if (!debugger) continue; - // Force the pause only for an explicit Debugger.pause dispatched during - // the drain above. A step-over/into/out in flight also enables stepping - // mode but must be left to reach its own target frame; pausing it here - // would stop inside the stepped-over callee. On initial SIGUSR1 - // activation with no frontend yet, nothing is requested and we just - // keep running. + // Only an explicit Debugger.pause from the drain above; an in-flight step-over also enables stepping but must reach its own frame. if (debugger->isPauseAtNextOpportunitySet()) { debugger->breakProgram(); return; diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index ff7efdf65ca7..b685767179a1 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -1533,9 +1533,7 @@ extern "C" void Bun__unrefChannelUnlessOverridden(JSC::JSGlobalObject* globalObj extern "C" bool Bun__shouldIgnoreOneDisconnectEventListener(JSC::JSGlobalObject* globalObject); extern "C" void Bun__ensureSignalHandler(); -#ifdef SIGUSR1 extern "C" void Bun__Sigusr1Handler__reinstall(); -#endif extern "C" bool Bun__isMainThreadVM(); extern "C" void Bun__onPosixSignal(int signalNumber); extern "C" void Bun__onSignalListenerCountChanged(int signalNumber, int listenerCount); @@ -1650,8 +1648,7 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e }; #if !OS(WINDOWS) Bun__ensureSignalHandler(); - // For SIGUSR1 this also displaces the runtime-inspector - // activation handler; the removal path below hands it back. + // For SIGUSR1 this displaces the runtime-inspector handler; removal below hands it back. installForwardSignalHandler(signalNumber); #else signal_handle.handle = Bun__UVSignalHandle__init( @@ -1675,14 +1672,9 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e if (void (*oldHandler)(int) = signal(signalNumber, SIG_DFL); oldHandler != forwardSignal) { // Don't uninstall the old handler if it's not the one we installed. signal(signalNumber, oldHandler); - } -#ifdef SIGUSR1 - // Last user listener gone: hand SIGUSR1 back to the - // runtime-inspector handler (no-op if it was never - // armed, e.g. --disable-sigusr1 or --inspect). - else if (signalNumber == SIGUSR1) + } else if (signalNumber == SIGUSR1) { Bun__Sigusr1Handler__reinstall(); -#endif + } #else SignalHandleValue signal_handle = signalToContextIdsMap->get(signalNumber); Bun__UVSignalHandle__close(signal_handle.handle); @@ -4677,11 +4669,17 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDebugProcess, (JSC::JSGlobalObject * gl return Bun::ERR::MISSING_ARGS(scope, globalObject, "The \"pid\" argument must be specified"_s); } - int pid = callFrame->argument(0).toInt32(globalObject); + // Same `pid != (pid | 0)` check as Process_functionKill; rejects fractions and values that toInt32 would wrap. + auto pidValue = callFrame->argument(0); + int pid = pidValue.toInt32(globalObject); RETURN_IF_EXCEPTION(scope, {}); - + bool isInt32 = JSC::JSValue::equal(globalObject, pidValue, jsNumber(pid)); + RETURN_IF_EXCEPTION(scope, {}); + if (!isInt32) { + return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "pid"_s, "number"_s, pidValue); + } if (pid <= 0) { - return Bun::ERR::INVALID_ARG_VALUE(scope, globalObject, "pid"_s, callFrame->argument(0), "must be a positive integer"_s); + return Bun::ERR::INVALID_ARG_VALUE(scope, globalObject, "pid"_s, pidValue, "must be a positive integer"_s); } #if !OS(WINDOWS) @@ -4691,10 +4689,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDebugProcess, (JSC::JSGlobalObject * gl return {}; } #else - // Node.js on Windows throws a plain Error whose message is the - // FormatMessageW string (see winapi_strerror in node.cc), with no .code - // or .syscall. Match that so test/js/node/test/parallel/test-debug-process.js - // passes. + // Node throws a plain Error carrying the FormatMessageW text, with no .code or .syscall (winapi_strerror in node.cc). auto throwWinapiError = [&](DWORD err) { LPWSTR buf = nullptr; DWORD n = FormatMessageW( @@ -4734,10 +4729,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDebugProcess, (JSC::JSGlobalObject * gl UnmapViewOfFile(pFunc); CloseHandle(hMapping); - // The target writes the handler pointer after creating the named mapping; - // a reader that races the install window sees zeroed memory. Treat that - // the same as no mapping so the caller retries instead of CreateRemoteThread - // failing (or worse, succeeding with a bogus entry point). + // Zero means we raced the target between creating the mapping and writing the pointer; report it like a missing mapping. if (!threadProc) { throwWinapiError(ERROR_FILE_NOT_FOUND); return {}; @@ -4757,8 +4749,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDebugProcess, (JSC::JSGlobalObject * gl return {}; } - // Wait briefly so the remote thread finishes signalling the target - // before we close the handle. + // Like Node, return once the injected thread has delivered the request (Node waits unbounded). WaitForSingleObject(hThread, 1000); CloseHandle(hThread); CloseHandle(hProcess); diff --git a/src/options_types/context.rs b/src/options_types/context.rs index 85e2856839cb..20da6b775aab 100644 --- a/src/options_types/context.rs +++ b/src/options_types/context.rs @@ -560,10 +560,7 @@ pub struct RuntimeOptions { pub cron_period: Box<[u8]>, pub cpu_prof: CpuProf, pub heap_prof: HeapProf, - /// `--disable-sigusr1`: leave SIGUSR1 at its default action instead of - /// arming the runtime-inspector handler. pub disable_sigusr1: bool, - /// `--inspect-port`: port for the runtime-activated inspector. pub inspect_port: Option>, } diff --git a/src/runtime/cli/Arguments.rs b/src/runtime/cli/Arguments.rs index e4cc2667fd0d..992d6a18e1a1 100644 --- a/src/runtime/cli/Arguments.rs +++ b/src/runtime/cli/Arguments.rs @@ -189,7 +189,7 @@ const RUNTIME_PARAMS_: &[ParamType] = &[ "--inspect-brk ? Activate Bun's debugger, set breakpoint on first line of code and wait" ), parse_param!( - "--inspect-port Port for the debugger started by SIGUSR1 / process._debugProcess() (default 6499, 0 for a free port)" + "--inspect-port [host:]port for the debugger started by SIGUSR1 / process._debugProcess() (default 6499, 0 for a free port)" ), parse_param!( "--disable-sigusr1 Do not start the debugger on SIGUSR1; leave the signal at its default action" diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index f097d8830378..1ab7689208e9 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -553,48 +553,36 @@ unsafe fn init_runtime_state( if opts.worker_ptr.is_null() { // SAFETY: `vm` is the freshly-boxed unique VM on this thread. unsafe { configure_debugger(vm, &opts.debugger) }; - // SAFETY: `vm` unique; `jsc_vm`/`debugger` written above. + // SAFETY: `vm` is unique here; `debugger` was just written above. unsafe { configure_sigusr1_handler(vm, opts) }; } Ok(state.cast()) } -/// Install (or suppress) the SIGUSR1 runtime-inspector handler on the main -/// thread. Must run after [`configure_debugger`] so the `vm.debugger.is_some()` -/// check reflects `--inspect*` flags. -/// -/// The per-VM trap callback is NOT installed here: `init_runtime_state` fires -/// before `vm.jsc_vm` is set (same ordering as the `ParentDeathWatchdog` note -/// above). `VirtualMachine::init` calls `runtime_inspector::on_main_vm_ready` -/// once `jsc_vm` exists, which installs it if this function armed the handler. +/// Runs after [`configure_debugger`] so that `--inspect*` is visible in `vm.debugger`. `vm.jsc_vm` does not +/// exist yet (see the `ParentDeathWatchdog` note above); `VirtualMachine::init` finishes the job with +/// `runtime_inspector::on_main_vm_ready`. /// /// # Safety /// `vm` is the freshly-boxed unique VM on this thread. unsafe fn configure_sigusr1_handler(vm: *mut VirtualMachine, opts: &InitOptions) { + use bun_jsc::runtime_inspector::{self, Sigusr1}; // SAFETY: per fn contract. - if !unsafe { (*vm).is_main_thread } { + let (is_main_thread, has_debugger) = + unsafe { ((*vm).is_main_thread, (*vm).debugger.is_some()) }; + if !is_main_thread { return; } - use bun_jsc::runtime_inspector; // SAFETY: per fn contract. unsafe { (*vm).inspect_port = opts.inspect_port }; - // On platforms where JSC's GC uses SIGUSR1 for thread suspend/resume - // (e.g. FreeBSD), replacing that handler would hang the first - // conservative stack scan. Leave the GC handler in place; runtime - // activation via `process._debugProcess` (Windows-style file mapping is - // not available here either) is simply unsupported on those platforms. - if runtime_inspector::gc_owns_sigusr1() { - return; - } - if opts.disable_sigusr1 { - runtime_inspector::set_default_sigusr1_action(); - // SAFETY: per fn contract; `debugger` written by `configure_debugger`. - } else if unsafe { (*vm).debugger.is_some() } { - runtime_inspector::ignore_sigusr1(); + runtime_inspector::configure(if opts.disable_sigusr1 { + Sigusr1::Default + } else if has_debugger { + Sigusr1::Ignore } else { - runtime_inspector::install_if_not_already(); - } + Sigusr1::StartInspector + }); } /// Translate the CLI flag / diff --git a/test/js/bun/runtime-inspector/helpers.ts b/test/js/bun/runtime-inspector/helpers.ts index b3cefc47ca14..4ded65e9d38b 100644 --- a/test/js/bun/runtime-inspector/helpers.ts +++ b/test/js/bun/runtime-inspector/helpers.ts @@ -20,7 +20,10 @@ export async function readStreamUntil( let timer!: ReturnType; const timeout = new Promise((_, reject) => { timer = setTimeout( - () => reject(new Error(`Timed out after ${timeoutMs}ms waiting for stream condition. Got: ${JSON.stringify(output)}`)), + () => + reject( + new Error(`Timed out after ${timeoutMs}ms waiting for stream condition. Got: ${JSON.stringify(output)}`), + ), timeoutMs, ); timer.unref(); @@ -126,25 +129,27 @@ export function wsUrlFromBanner(stderr: string): string { return match![0]; } -/** Minimal request/response CDP client over `ws`; `onEvent` sees notifications. */ +/** Minimal request/response CDP client over `ws`; `onEvent` sees notifications, protocol errors reject. */ export function cdpClient(ws: WebSocket, onEvent?: (msg: any) => void) { let nextId = 1; - const pending = new Map void>(); + const pending = new Map>(); ws.onmessage = event => { const msg = JSON.parse(event.data as string); - if (msg.id !== undefined) { - pending.get(msg.id)?.(msg); - pending.delete(msg.id); - } else { + if (msg.id === undefined) { onEvent?.(msg); + return; } + const request = pending.get(msg.id); + pending.delete(msg.id); + if (msg.error) request?.reject(new Error(`CDP error: ${JSON.stringify(msg.error)}`)); + else request?.resolve(msg); }; return function send(method: string, params: Record = {}): Promise { const id = nextId++; - const { promise, resolve } = Promise.withResolvers(); - pending.set(id, resolve); + const request = Promise.withResolvers(); + pending.set(id, request); ws.send(JSON.stringify({ id, method, params })); - return withTimeout(`response to ${method}`, promise); + return withTimeout(`response to ${method}`, request.promise); }; } diff --git a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts index e275be1ee676..ceaf10192062 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts @@ -1,3 +1,4 @@ +import type { Subprocess } from "bun"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows } from "harness"; import { @@ -80,8 +81,10 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { let stdout = ""; for (let i = 1; i <= 3; i++) { process.kill(pid, "SIGUSR1"); - stdout = await readStreamUntil(reader, s => s.includes(`user ${i}`)); + stdout += await readStreamUntil(reader, s => s.includes(`user ${i}`)); } + // The third signal makes the target exit, so stdout reaches EOF. + stdout = await readStreamToEnd(reader, stdout); reader.releaseLock(); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); @@ -122,28 +125,55 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { stdout: "pipe", stderr: "pipe", }); - const reader = proc.stderr.getReader(); - let stderr = await readStreamUntil(reader, hasBanner); - - process.kill(proc.pid, "SIGUSR1"); - // Any CDP connection works as the "loop has turned" ack; under -wait/-brk - // this is also what lets the target proceed to exit cleanly below. - const ws = await connectInspector(wsUrlFromBanner(stderr)); - try { - const result = await cdpClient(ws)("Runtime.evaluate", { expression: "6 * 7" }); - expect(result.result.result.value).toBe(42); - } finally { - ws.close(); - } - - proc.kill(); - stderr = await readStreamToEnd(reader, stderr); - reader.releaseLock(); - await proc.exited; + await expectSigusr1Ignored(proc); + }); - expect({ banners: countBanners(stderr), signalCode: proc.signalCode }).toEqual({ - banners: 1, - signalCode: "SIGTERM", + test("SIGUSR1 is still ignored under --inspect after a user listener is added and removed", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "--inspect=0", + "-e", + `const onSignal = () => {}; + process.on("SIGUSR1", onSignal); + process.off("SIGUSR1", onSignal); + ${IDLE}`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", }); + // IDLE prints the pid, so a stdout line means the listener has come and gone. + const stdoutReader = proc.stdout.getReader(); + await readStreamUntil(stdoutReader, s => s.includes("\n")); + stdoutReader.releaseLock(); + + await expectSigusr1Ignored(proc); }); }); + +async function expectSigusr1Ignored(proc: Subprocess) { + const reader = proc.stderr.getReader(); + let stderr = await readStreamUntil(reader, hasBanner); + + process.kill(proc.pid, "SIGUSR1"); + // Any CDP connection works as the "loop has turned" ack; under -wait/-brk + // this is also what lets the target proceed to exit cleanly below. + const ws = await connectInspector(wsUrlFromBanner(stderr)); + try { + const result = await cdpClient(ws)("Runtime.evaluate", { expression: "6 * 7" }); + expect(result.result.result.value).toBe(42); + } finally { + ws.close(); + } + + proc.kill(); + stderr = await readStreamToEnd(reader, stderr); + reader.releaseLock(); + await proc.exited; + + expect({ banners: countBanners(stderr), signalCode: proc.signalCode }).toEqual({ + banners: 1, + signalCode: "SIGTERM", + }); +} diff --git a/test/js/bun/runtime-inspector/runtime-inspector.test.ts b/test/js/bun/runtime-inspector/runtime-inspector.test.ts index 1c713c38ae0a..5da00d4658eb 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isWindows } from "harness"; +import { bunEnv, bunExe, isWindows } from "harness"; import os from "node:os"; import { BUSY_LOOP, @@ -90,10 +90,7 @@ describe.concurrent("process._debugProcess", () => { } }); - // Times out on the release+ASAN lane waiting for Debugger.paused; the - // while(true) activation test above covers trap delivery there, and the - // non-sanitizer lanes cover the full pause path. - test.skipIf(isASAN)("Debugger.pause interrupts while(true)", async () => { + test("Debugger.pause interrupts while(true)", async () => { const { proc, pid } = await spawnTarget(BUSY_LOOP); await using _ = proc; @@ -126,6 +123,27 @@ describe.concurrent("process._debugProcess", () => { expect(await proc.stdout.text()).toBe("ERR_MISSING_ARGS\n"); }); + test("rejects pids that are not positive int32s", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `for (const pid of [0, -1, 1.5, 2 ** 32 + 1]) { + try { process._debugProcess(pid); console.log(pid, "no error"); } catch (e) { console.log(pid, e.code); } + }`, + ], + env: bunEnv, + stdout: "pipe", + }); + expect(await proc.stdout.text()).toMatchInlineSnapshot(` + "0 ERR_INVALID_ARG_VALUE + -1 ERR_INVALID_ARG_VALUE + 1.5 ERR_INVALID_ARG_TYPE + 4294967297 ERR_INVALID_ARG_TYPE + " + `); + }); + test.skipIf(isWindows)("reports kill() failures as system errors", async () => { await using proc = Bun.spawn({ cmd: [bunExe(), "-e", `try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`], From 30ef98eb72d88f2636ce08183de3a4ce08b56d7d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:08:23 +0000 Subject: [PATCH 5/8] Runtime inspector tests: keep BUN_JSC_validateExceptionChecks off the inspectee On the ASAN lanes the runner sets BUN_JSC_validateExceptionChecks, and a process answering Runtime.evaluate (or pausing) aborts inside JSC's InjectedScript on the unchecked getOwnNonIndexPropertyNames scope in the prebuilt WebKit, the same gap that keeps test/cli/inspect/inspect.test.ts in test/no-validate-exceptions.txt. Every CDP round trip in these files timed out there as a result. Strip the flag for the inspected processes only, as test/js/node/inspector/inspector.test.ts does; the test process, the signalling children and the error-path children still run with it. spawnTarget now kills the child if it fails before handing it to the caller. --- test/js/bun/runtime-inspector/helpers.ts | 35 ++++++++++++++----- .../runtime-inspector-posix.test.ts | 5 +-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/test/js/bun/runtime-inspector/helpers.ts b/test/js/bun/runtime-inspector/helpers.ts index 4ded65e9d38b..2b239b52aa14 100644 --- a/test/js/bun/runtime-inspector/helpers.ts +++ b/test/js/bun/runtime-inspector/helpers.ts @@ -73,6 +73,18 @@ export async function waitForBanner(proc: Subprocess): Promise } } +/** + * Environment for the process being inspected. Answering `Runtime.evaluate` (or pausing) runs + * JSC's InjectedScript, whose unchecked getOwnNonIndexPropertyNames exception scope in the prebuilt + * WebKit aborts the process under BUN_JSC_validateExceptionChecks, which the ASAN lanes set (the + * reason test/cli/inspect/inspect.test.ts is in test/no-validate-exceptions.txt). Same workaround + * as test/js/node/inspector/inspector.test.ts; ASAN/LSAN and the signalling processes keep bunEnv. + */ +export const inspecteeEnv = (() => { + const { BUN_JSC_validateExceptionChecks, BUN_JSC_dumpSimulatedThrows, ...env } = bunEnv; + return env; +})(); + /** * Spawns bun running `script` with a random inspector port and returns it once * it has printed its pid on the first stdout line, i.e. once JS is executing. @@ -81,20 +93,25 @@ export async function waitForBanner(proc: Subprocess): Promise export async function spawnTarget(script: string, extraArgs: string[] = []) { const proc = Bun.spawn({ cmd: [bunExe(), "--inspect-port=0", ...extraArgs, "-e", script], - env: bunEnv, + env: inspecteeEnv, stdout: "pipe", stderr: "pipe", }); - const reader = proc.stdout.getReader(); - let first: string; try { - first = await readStreamUntil(reader, s => s.includes("\n")); - } finally { - reader.releaseLock(); + const reader = proc.stdout.getReader(); + let first: string; + try { + first = await readStreamUntil(reader, s => s.includes("\n")); + } finally { + reader.releaseLock(); + } + const pid = parseInt(first, 10); + expect(pid).toBeGreaterThan(0); + return { proc, pid }; + } catch (error) { + proc.kill(); + throw error; } - const pid = parseInt(first, 10); - expect(pid).toBeGreaterThan(0); - return { proc, pid }; } /** Runs `process._debugProcess(pid)` in a separate bun and asserts it succeeded. */ diff --git a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts index ceaf10192062..b7311af48f49 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts @@ -7,6 +7,7 @@ import { countBanners, hasBanner, IDLE, + inspecteeEnv, readStreamToEnd, readStreamUntil, spawnTarget, @@ -121,7 +122,7 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { test.each(["--inspect=0", "--inspect-wait=0", "--inspect-brk=0"])("SIGUSR1 is ignored under %s", async flag => { await using proc = Bun.spawn({ cmd: [bunExe(), flag, "-e", IDLE], - env: bunEnv, + env: inspecteeEnv, stdout: "pipe", stderr: "pipe", }); @@ -139,7 +140,7 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { process.off("SIGUSR1", onSignal); ${IDLE}`, ], - env: bunEnv, + env: inspecteeEnv, stdout: "pipe", stderr: "pipe", }); From 388ab9ee7013636469141dee8bf12ba8c7b6ba63 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:49:19 +0000 Subject: [PATCH 6/8] Runtime inspector tests: install the user SIGUSR1 listener before reporting ready The two user-listener targets printed their pid before calling process.on, so a signal sent right after spawnTarget returned could still reach the activation handler. debugProcess drains stdout as well, and the banner helpers type the stderr pipe rather than stdout. --- test/js/bun/runtime-inspector/helpers.ts | 17 +++++++++++------ .../runtime-inspector-posix.test.ts | 10 +++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/test/js/bun/runtime-inspector/helpers.ts b/test/js/bun/runtime-inspector/helpers.ts index 2b239b52aa14..40def04faff7 100644 --- a/test/js/bun/runtime-inspector/helpers.ts +++ b/test/js/bun/runtime-inspector/helpers.ts @@ -64,7 +64,7 @@ export function hasBanner(stderr: string): boolean { } /** Reads the target's stderr until one full banner has been printed. */ -export async function waitForBanner(proc: Subprocess): Promise { +export async function waitForBanner(proc: Subprocess): Promise { const reader = proc.stderr.getReader(); try { return await readStreamUntil(reader, hasBanner); @@ -86,9 +86,10 @@ export const inspecteeEnv = (() => { })(); /** - * Spawns bun running `script` with a random inspector port and returns it once - * it has printed its pid on the first stdout line, i.e. once JS is executing. - * `script` must `console.log(process.pid)` first. + * Spawns bun running `script` with a random inspector port and returns once the + * script has printed its pid, which must be its first stdout line. The script + * should print it only after any setup the test relies on (signal listeners), + * since the caller may signal the process as soon as this returns. */ export async function spawnTarget(script: string, extraArgs: string[] = []) { const proc = Bun.spawn({ @@ -122,8 +123,12 @@ export async function debugProcess(pid: number): Promise { stdout: "pipe", stderr: "pipe", }); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect({ exitCode, hasError: stderr.includes("error:") }).toEqual({ exitCode: 0, hasError: false }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, exitCode, hasError: stderr.includes("error:") }).toEqual({ + stdout: "", + exitCode: 0, + hasError: false, + }); } /** diff --git a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts index b7311af48f49..c514b61f5d09 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts @@ -71,9 +71,9 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { test("a user SIGUSR1 listener takes precedence", async () => { const { proc, pid } = await spawnTarget( - `console.log(process.pid); - let n = 0; + `let n = 0; process.on("SIGUSR1", () => { console.log("user " + ++n); if (n === 3) process.exit(0); }); + console.log(process.pid); setInterval(() => {}, 1000);`, ); await using _ = proc; @@ -98,9 +98,9 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { test("removing the last user listener hands SIGUSR1 back to the inspector", async () => { const { proc, pid } = await spawnTarget( - `console.log(process.pid); - const onSignal = () => { console.log("user"); process.off("SIGUSR1", onSignal); console.log("removed"); }; + `const onSignal = () => { console.log("user"); process.off("SIGUSR1", onSignal); console.log("removed"); }; process.on("SIGUSR1", onSignal); + console.log(process.pid); setInterval(() => {}, 1000);`, ); await using _ = proc; @@ -153,7 +153,7 @@ describe.skipIf(isWindows).concurrent("SIGUSR1 activation", () => { }); }); -async function expectSigusr1Ignored(proc: Subprocess) { +async function expectSigusr1Ignored(proc: Subprocess) { const reader = proc.stderr.getReader(); let stderr = await readStreamUntil(reader, hasBanner); From 9a5d8dd4e7a39b1926f88b1ea00657cf39d94ff9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:02:02 +0000 Subject: [PATCH 7/8] Runtime inspector tests: assert the full result of the helper processes runSnippet drains stdout and stderr and returns them with the exit code, so debugProcess and the argument-validation tests assert on all three instead of stdout alone. --- test/js/bun/runtime-inspector/helpers.ts | 17 ++--- .../runtime-inspector.test.ts | 62 ++++++++----------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/test/js/bun/runtime-inspector/helpers.ts b/test/js/bun/runtime-inspector/helpers.ts index 40def04faff7..509d49995a01 100644 --- a/test/js/bun/runtime-inspector/helpers.ts +++ b/test/js/bun/runtime-inspector/helpers.ts @@ -115,20 +115,21 @@ export async function spawnTarget(script: string, extraArgs: string[] = []) { } } -/** Runs `process._debugProcess(pid)` in a separate bun and asserts it succeeded. */ -export async function debugProcess(pid: number): Promise { +/** Runs `code` with `bun -e` to completion. Assert on the whole result so a failure shows everything. */ +export async function runSnippet(code: string) { await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", `process._debugProcess(${pid})`], + cmd: [bunExe(), "-e", code], env: bunEnv, stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, exitCode, hasError: stderr.includes("error:") }).toEqual({ - stdout: "", - exitCode: 0, - hasError: false, - }); + return { stdout, stderr, exitCode }; +} + +/** Runs `process._debugProcess(pid)` in a separate bun and asserts it succeeded. */ +export async function debugProcess(pid: number): Promise { + expect(await runSnippet(`process._debugProcess(${pid})`)).toEqual({ stdout: "", stderr: "", exitCode: 0 }); } /** diff --git a/test/js/bun/runtime-inspector/runtime-inspector.test.ts b/test/js/bun/runtime-inspector/runtime-inspector.test.ts index 5da00d4658eb..8d1b7dc375e7 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { isWindows } from "harness"; import os from "node:os"; import { BUSY_LOOP, @@ -11,6 +11,7 @@ import { IDLE, readStreamToEnd, readStreamUntil, + runSnippet, spawnTarget, waitForBanner, withTimeout, @@ -115,53 +116,44 @@ describe.concurrent("process._debugProcess", () => { }); test("rejects a missing pid", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", `try { process._debugProcess(); } catch (e) { console.log(e.code); }`], - env: bunEnv, - stdout: "pipe", + expect(await runSnippet(`try { process._debugProcess(); } catch (e) { console.log(e.code); }`)).toEqual({ + stdout: "ERR_MISSING_ARGS\n", + stderr: "", + exitCode: 0, }); - expect(await proc.stdout.text()).toBe("ERR_MISSING_ARGS\n"); }); test("rejects pids that are not positive int32s", async () => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - `for (const pid of [0, -1, 1.5, 2 ** 32 + 1]) { - try { process._debugProcess(pid); console.log(pid, "no error"); } catch (e) { console.log(pid, e.code); } - }`, - ], - env: bunEnv, - stdout: "pipe", + const result = await runSnippet( + `for (const pid of [0, -1, 1.5, 2 ** 32 + 1]) { + try { process._debugProcess(pid); console.log(pid, "no error"); } catch (e) { console.log(pid, e.code); } + }`, + ); + expect(result).toEqual({ + stdout: [ + "0 ERR_INVALID_ARG_VALUE", + "-1 ERR_INVALID_ARG_VALUE", + "1.5 ERR_INVALID_ARG_TYPE", + "4294967297 ERR_INVALID_ARG_TYPE", + "", + ].join("\n"), + stderr: "", + exitCode: 0, }); - expect(await proc.stdout.text()).toMatchInlineSnapshot(` - "0 ERR_INVALID_ARG_VALUE - -1 ERR_INVALID_ARG_VALUE - 1.5 ERR_INVALID_ARG_TYPE - 4294967297 ERR_INVALID_ARG_TYPE - " - `); }); test.skipIf(isWindows)("reports kill() failures as system errors", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", `try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`], - env: bunEnv, - stdout: "pipe", - }); - expect(await proc.stdout.text()).toBe("ESRCH kill\n"); + expect( + await runSnippet(`try { process._debugProcess(2147483646); } catch (e) { console.log(e.code, e.syscall); }`), + ).toEqual({ stdout: "ESRCH kill\n", stderr: "", exitCode: 0 }); }); // Node throws a plain Error carrying the Win32 message here; the vendored // test/js/node/test/parallel/test-debug-process.js checks the exact text. test.skipIf(!isWindows)("reports a missing target with the Win32 message", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", `try { process._debugProcess(2147483646); } catch (e) { console.log(e.message); }`], - env: bunEnv, - stdout: "pipe", - }); - expect(await proc.stdout.text()).toBe("The system cannot find the file specified.\n"); + expect( + await runSnippet(`try { process._debugProcess(2147483646); } catch (e) { console.log(e.message); }`), + ).toEqual({ stdout: "The system cannot find the file specified.\n", stderr: "", exitCode: 0 }); }); }); From 192eb9ea8cb56423ca838701103190647ba4ca41 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:09:18 +0000 Subject: [PATCH 8/8] ci: run the darwin lane once for the SIGUSR1 and Mach semaphore paths [macos tests]