From 22c7683c78324e55b1ac0d32d94e3d0278526b3f Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 02:40:16 +0000 Subject: [PATCH 01/11] repl: interrupt a running evaluation with Ctrl+C The REPL stays in raw mode while it evaluates, so Ctrl+C is delivered as a byte nobody reads. After `while (true) {}` the session is unrecoverable: two Ctrl+Cs do nothing, the process has to be killed from another terminal, and termios is left in raw mode. Hand the terminal back to the line discipline for the duration of an evaluation (what node's REPL does, so Ctrl+C arrives as SIGINT) and arm the existing SigintWatcher, which raises a JSC termination trap. Synchronous code unwinds and the REPL reports ERR_SCRIPT_EXECUTION_INTERRUPTED, matching node. This also removes the old interrupt path, which broke out of a promise wait by calling `setExecutionForbidden()`. That is one-way in JSC, so a single Ctrl+C during an `await` left the VM silently dropping every microtask for the rest of the session. SigintReceiver's flag is written by the watcher thread and read by the VM thread, so it becomes atomic. The REPL's terminal tests now spawn through `Bun.spawn`'s inline `terminal` option: handing it an already-created `Bun.Terminal` skips the setsid + TIOCSCTTY setup, so the child has no controlling terminal and never sees SIGINT. --- src/jsc/bindings/bindings.cpp | 87 ++++++ src/jsc/bindings/vm/SigintReceiver.h | 12 +- src/runtime/cli/repl.rs | 415 ++++++++++++--------------- test/js/bun/repl/repl.test.ts | 81 ++++-- 4 files changed, 345 insertions(+), 250 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index e694c218f38b..cc37968ab216 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -12,9 +12,11 @@ #include "JavaScriptCore/ErrorType.h" #include "JavaScriptCore/TopExceptionScope.h" #include "JavaScriptCore/Exception.h" +#include "JavaScriptCore/VMTraps.h" #include "ErrorCode+List.h" #include "ErrorCode.h" #include "JavaScriptCore/ThrowScope.h" +#include "../vm/SigintWatcher.h" #include "JavaScriptCore/JSCast.h" #include "JavaScriptCore/JSType.h" @@ -6266,6 +6268,83 @@ CPP_DECL [[ZIG_EXPORT(nothrow)]] unsigned int Bun__CallFrame__getLineNumber(JSC: return lineColumn.line; } +// Armed around one REPL evaluation. The watcher thread flips the receiver flag +// before raising the termination trap, and that flag is the only durable record +// of the signal: JSC clears both the trap bit and `hasTerminationRequest()` by +// the time the outermost VM entry scope has unwound. +namespace { +class ReplSigintScope final : public Bun::SigintReceiver { +public: + explicit ReplSigintScope(JSC::JSGlobalObject* globalObject) + : m_holder(Bun::SigintWatcher::hold(globalObject, this)) + { + } + +private: + Bun::SigintWatcher::GlobalObjectHolder m_holder; +}; +} + +// Drops whatever termination state a SIGINT left behind so the VM can run +// JavaScript again. +static void replClearTermination(JSC::VM& vm) +{ + vm.traps().clearTrap(JSC::VMTraps::NeedTermination); + if (vm.hasPendingTerminationException()) { + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + } + vm.clearHasTerminationRequest(); +} + +// Arms the SIGINT watcher for `globalObject`. While armed, a SIGINT raises a +// JSC termination trap, which unwinds synchronous JavaScript (`while (true) {}`) +// the same way node's `breakOnSigint` does. The returned scope must be passed to +// `Bun__REPL__disarmSigint`. +extern "C" void* Bun__REPL__armSigint(JSC::JSGlobalObject* globalObject) +{ + auto& vm = JSC::getVM(globalObject); + // Allocate the termination exception up front: the trap handler runs at a + // point where allocating one is not allowed. + vm.ensureTerminationException(); + return new ReplSigintScope(globalObject); +} + +extern "C" void Bun__REPL__disarmSigint(JSC::JSGlobalObject* globalObject, void* scope) +{ + // Tears down the watcher thread, so no further signal can reach us. + delete static_cast(scope); + + auto& vm = JSC::getVM(globalObject); + // A signal that raced the disarm leaves a trap bit nobody will service; + // the next evaluation would terminate the instant it entered the VM. + if (vm.traps().hasTrapBit(JSC::VMTraps::NeedTermination)) [[unlikely]] { + replClearTermination(vm); + } +} + +extern "C" bool Bun__REPL__sigintRequested(void* scope) +{ + return scope && static_cast(scope)->getSigintReceived(); +} + +// Clears the SIGINT termination state and returns the error to report, or an +// empty JSValue when no interrupt is pending. +extern "C" JSC::EncodedJSValue Bun__REPL__takeSigintError(JSC::JSGlobalObject* globalObject, void* scope) +{ + if (!Bun__REPL__sigintRequested(scope)) { + return JSC::JSValue::encode({}); + } + + auto& vm = JSC::getVM(globalObject); + static_cast(scope)->setSigintReceived(false); + replClearTermination(vm); + + JSC::JSObject* error = Bun::createError(globalObject, Bun::ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, + "Script execution was interrupted by `SIGINT`"_s); + globalObject->putDirect(vm, JSC::Identifier::fromString(vm, "_error"_s), error); + return JSC::JSValue::encode(error); +} + // REPL evaluation function - evaluates JavaScript code in the global scope // Returns the result value, or undefined if an exception was thrown // If an exception is thrown, the exception value is stored in *exception @@ -6296,6 +6375,14 @@ extern "C" JSC::EncodedJSValue Bun__REPL__evaluate( WTF::NakedPtr evalException; JSC::JSValue result = JSC::evaluate(globalObject, sourceCode, globalObject->globalThis(), evalException); + // SIGINT unwound the script, and `evalException` is the internal + // TerminatedExecutionError. The caller reports + // ERR_SCRIPT_EXECUTION_INTERRUPTED via `Bun__REPL__takeSigintError` instead. + if (evalException && vm.isTerminationException(evalException.get())) [[unlikely]] { + *exception = JSC::JSValue::encode(JSC::jsUndefined()); + return JSC::JSValue::encode(JSC::jsUndefined()); + } + if (evalException) { *exception = JSC::JSValue::encode(evalException->value()); // Set _error on the globalObject directly (not globalThis proxy) diff --git a/src/jsc/bindings/vm/SigintReceiver.h b/src/jsc/bindings/vm/SigintReceiver.h index fc66d43b3c58..303c72ae81a9 100644 --- a/src/jsc/bindings/vm/SigintReceiver.h +++ b/src/jsc/bindings/vm/SigintReceiver.h @@ -1,23 +1,27 @@ #pragma once +#include + namespace Bun { +// `m_sigintReceived` is written by the SigintWatcher thread and read by the VM +// thread, so it has to be atomic. class SigintReceiver { public: SigintReceiver() = default; void setSigintReceived(bool value = true) { - m_sigintReceived = value; + m_sigintReceived.store(value, std::memory_order_relaxed); } - bool getSigintReceived() + bool getSigintReceived() const { - return m_sigintReceived; + return m_sigintReceived.load(std::memory_order_relaxed); } protected: - bool m_sigintReceived = false; + std::atomic m_sigintReceived = false; }; } // namespace Bun diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index 96c1486cf41f..56406ced9529 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -11,8 +11,7 @@ //! //! This replaces the TypeScript-based REPL for faster startup and better integration. -#[cfg(unix)] -use core::ffi::c_int; +use core::ffi::c_void; use core::fmt::Arguments; use std::io::Write as _; @@ -54,6 +53,34 @@ unsafe extern "C" { prefixPtr: *const u8, prefixLen: usize, ) -> JSValue; + + /// Arms the SIGINT watcher; the returned scope must be passed to + /// `Bun__REPL__disarmSigint` exactly once. + fn Bun__REPL__armSigint(globalObject: *const JSGlobalObject) -> *mut c_void; + fn Bun__REPL__disarmSigint(globalObject: *const JSGlobalObject, scope: *mut c_void); + fn Bun__REPL__sigintRequested(scope: *mut c_void) -> bool; + /// Clears the VM's SIGINT termination state and hands back the error to + /// report, or an empty `JSValue` when no interrupt is pending. + fn Bun__REPL__takeSigintError( + globalObject: *const JSGlobalObject, + scope: *mut c_void, + ) -> JSValue; +} + +/// Ctrl+C interruption armed for the duration of one evaluation. +struct SigintScope { + scope: *mut c_void, +} + +/// What one REPL evaluation produced. +enum EvalOutcome { + /// The value of the input, already unwrapped from the `{ value: ... }` + /// wrapper the REPL transform adds. + Value(JSValue), + /// A thrown exception, a rejected promise, or a Ctrl+C interrupt. + Error(JSValue), + /// A top-level await that never settled. Nothing to print. + Pending, } // ============================================================================ @@ -967,51 +994,66 @@ impl<'a> Repl<'a> { } } - /// Temporarily enable SIGINT delivery during blocking promise waits - fn enable_signals_during_wait(&mut self) { - if let Some(vm) = self.vm { - // Cleared in disable_signals_during_wait; Release pairs with the - // Acquire load in `sigint_handler`. - SIGINT_VM.store(vm.jsc_vm, core::sync::atomic::Ordering::Release); + /// Make Ctrl+C interrupt the evaluation that is about to run. + /// + /// Like node's REPL, hand the terminal back to the line discipline so Ctrl+C + /// arrives as SIGINT rather than a byte nobody reads, and arm the watcher so + /// the signal raises a JSC termination trap, which unwinds `while (true) {}`. + /// `None` when not attached to a terminal: SIGINT keeps killing the process. + fn begin_interruptible_eval(&mut self) -> Option { + let global = self.global?; + if !self.is_tty { + return None; } + // Arm before handing the terminal back, so the window where Ctrl+C still + // means "kill the process" stays closed. + // SAFETY: `global` is a live opaque `JSGlobalObject` handle; the scope is + // released exactly once in `end_interruptible_eval`. + let scope = unsafe { Bun__REPL__armSigint(global) }; + #[cfg(unix)] - { - // Switch to normal terminal mode (has ISIG) so Ctrl+C generates SIGINT - let _ = self.tty_state.set_mode(0, tty::Mode::Normal); + let _ = self.tty_state.set_mode(0, tty::Mode::Normal); + // On Windows, ENABLE_PROCESSED_INPUT is already set, so Ctrl+C already + // arrives as a console control event. - // Install SIGINT handler - // SAFETY: zeroed `sigaction` is a valid empty mask + null restorer; we set - // sa_sigaction/sa_flags below. `act` is valid for the duration of the call. - unsafe { - let mut act: bun_sys::posix::Sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = sigint_handler as *const () as usize; - act.sa_flags = 0; - bun_sys::posix::sigaction(libc::SIGINT, &raw const act, core::ptr::null_mut()); - } - } - // On Windows, ENABLE_PROCESSED_INPUT is already set so Ctrl+C works + Some(SigintScope { scope }) } - /// Restore raw terminal mode after promise wait - fn disable_signals_during_wait(&mut self) { - SIGINT_VM.store(core::ptr::null_mut(), core::sync::atomic::Ordering::Release); + fn end_interruptible_eval(&mut self, scope: Option) { + let Some(global) = self.global else { + return; + }; + let Some(scope) = scope else { + return; + }; #[cfg(unix)] - { - // Back to raw mode - let _ = self.tty_state.set_mode(0, tty::Mode::Raw); + let _ = self.tty_state.set_mode(0, tty::Mode::Raw); - // Restore default SIGINT handling - // SAFETY: zeroed `sigaction` is a valid empty mask + null restorer; SIG_DFL - // restores the default disposition. `act` is valid for the duration of the call. - unsafe { - let mut act: bun_sys::posix::Sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = libc::SIG_DFL; - act.sa_flags = 0; - bun_sys::posix::sigaction(libc::SIGINT, &raw const act, core::ptr::null_mut()); - } - } + // SAFETY: `scope.scope` came from `Bun__REPL__armSigint` and is consumed + // once; `global` is a live opaque `JSGlobalObject` handle. + unsafe { Bun__REPL__disarmSigint(global, scope.scope) }; + } + + /// Whether a Ctrl+C has asked the VM to stop running JavaScript. + fn sigint_requested(scope: Option<&SigintScope>) -> bool { + let Some(scope) = scope else { + return false; + }; + // SAFETY: `scope.scope` is live until `end_interruptible_eval`. + unsafe { Bun__REPL__sigintRequested(scope.scope) } + } + + /// Clear the VM's termination state and take the error to report, if the + /// evaluation was interrupted by Ctrl+C. + fn take_interrupt_error(&self, scope: Option<&SigintScope>) -> Option { + let global = self.global?; + let scope = scope?; + // SAFETY: `global` is a live opaque `JSGlobalObject` handle and + // `scope.scope` is live until `end_interruptible_eval`. + let error = unsafe { Bun__REPL__takeSigintError(global, scope.scope) }; + (!error.is_empty()).then_some(error) } fn write(&self, data: &[u8]) { @@ -1261,134 +1303,136 @@ impl<'a> Repl<'a> { // JavaScript Evaluation // ======================================================================== - fn evaluate_and_print(&mut self, code: &[u8]) { + /// Evaluate already-transformed REPL input, resolving the async IIFE the + /// transform emits for top-level `await`. Ctrl+C interrupts both the + /// synchronous evaluation and the wait for the promise. + fn evaluate_transformed(&mut self, code: &[u8]) -> EvalOutcome { let Some(global) = self.global else { - return; + return EvalOutcome::Pending; }; let Some(vm) = self.vm else { - return; + return EvalOutcome::Pending; }; - // Transform the code using REPL mode (hoists declarations, wraps result in { value: expr }) - let Some(transformed_code) = self.transform_for_repl(code) else { - // Transform failed, try evaluating raw code (for syntax errors, etc.) - self.evaluate_raw(code); - return; - }; + let sigint = self.begin_interruptible_eval(); - // Evaluate the transformed code let mut exception: JSValue = JSValue::UNDEFINED; // SAFETY: `global` is a live opaque `JSGlobalObject` handle; slice ptr/len pairs // are valid for the duration of the call; `exception` is a stack local. let result = unsafe { Bun__REPL__evaluate( global, - transformed_code.as_ptr(), - transformed_code.len(), + code.as_ptr(), + code.len(), b"[repl]".as_ptr(), b"[repl]".len(), &raw mut exception, ) }; - // Check for exception - if !exception.is_undefined() && !exception.is_null() { - self.set_last_error(exception); - self.print_js_error(exception); - return; - } - - // Handle async IIFE results - wait for promise to resolve - let mut resolved_result = result; - if let Some(promise) = result.as_promise() { + let mut outcome = if !exception.is_undefined() && !exception.is_null() { + EvalOutcome::Error(exception) + } else if let Some(promise) = result.as_promise() { // Mark as handled BEFORE waiting to prevent unhandled rejection output jsc::JSPromise::opaque_mut(promise).set_handled(); - - // Temporarily re-enable signal delivery so Ctrl+C can interrupt - // the blocking waitForPromise call - self.enable_signals_during_wait(); - // Note: reshaped for borrowck — call disable_signals_during_wait() explicitly on each return path below - - // Wait for the promise to settle - vm.as_mut() - .wait_for_promise(jsc::AnyPromise::Normal(promise)); - - // If execution was forbidden by SIGINT, clear it and report - if vm.jsc_vm().execution_forbidden() { - vm.jsc_vm().set_execution_forbidden(false); - global.clear_termination_exception(); - self.print(format_args!("\n")); - self.disable_signals_during_wait(); - return; - } - + self.wait_for_promise_interruptible(promise, sigint.as_ref()); // SAFETY: `vm.jsc_vm` is the live JSC VM handle for this thread. let jsc_vm_ref = vm.jsc_vm(); - // Check promise status after waiting match jsc::JSPromise::opaque_mut(promise).status() { PromiseStatus::Fulfilled => { - resolved_result = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref); + EvalOutcome::Value(jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref)) } PromiseStatus::Rejected => { - let rejection = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref); - self.set_last_error(rejection); - // Set _error on the global object - let global_this = global.to_js_value(); - global_this.put(global, b"_error", rejection); - self.print_js_error(rejection); - self.disable_signals_during_wait(); - return; - } - PromiseStatus::Pending => { - // Interrupted by signal or timed out - self.print(format_args!("\n")); - self.disable_signals_during_wait(); - return; + EvalOutcome::Error(jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref)) } + PromiseStatus::Pending => EvalOutcome::Pending, } - self.disable_signals_during_wait(); - } + } else { + EvalOutcome::Value(result) + }; - // Extract the value from the result wrapper { value: expr } - // The REPL transform wraps the last expression in { value: expr } - let mut actual_result = resolved_result; - if resolved_result.is_object() { - // Wrapper is REPL-built { __proto__: null, value: ... } so getOwn shouldn't throw, - // but if it does, propagate as a REPL error. - let maybe_value = - match resolved_result.get_own(global, &bun_core::String::static_("value")) { - Ok(v) => v, - Err(err) => { - let exc = global.take_exception(err); - self.set_last_error(exc); - self.print_js_error(exc); - vm.as_mut().tick(); - return; - } + // A Ctrl+C supersedes whatever the evaluation left behind, and the VM + // has to come out of its terminated state before it runs JS again. + if let Some(error) = self.take_interrupt_error(sigint.as_ref()) { + outcome = EvalOutcome::Error(error); + } + self.end_interruptible_eval(sigint); + + // Unwrap the `{ value: expr }` wrapper the REPL transform adds. It is a + // REPL-built `{ __proto__: null, value: ... }` so getOwn shouldn't throw, + // but if it does, propagate as a REPL error. + if let EvalOutcome::Value(value) = outcome { + if value.is_object() { + outcome = match value.get_own(global, &bun_core::String::static_("value")) { + Ok(Some(inner)) => EvalOutcome::Value(inner), + Ok(None) => EvalOutcome::Value(value), + Err(err) => EvalOutcome::Error(global.take_exception(err)), }; - if let Some(value) = maybe_value { - actual_result = value; } } + outcome + } - // Store and print result - self.set_last_result(actual_result); - - // Set _ to the last result (only if not undefined) - // Use the global object as JSValue and put the property on it - if !actual_result.is_undefined() { - let global_this = global.to_js_value(); - global_this.put(global, b"_", actual_result); + /// Wait for `promise` to settle while running the event loop, returning + /// early when Ctrl+C asks the VM to stop. + fn wait_for_promise_interruptible( + &mut self, + promise: *mut jsc::JSPromise, + sigint: Option<&SigintScope>, + ) { + let Some(vm) = self.vm else { + return; + }; + let pending = + |promise| jsc::JSPromise::opaque_mut(promise).status() == PromiseStatus::Pending; + while pending(promise) && !Self::sigint_requested(sigint) { + vm.as_mut().tick(); + if !pending(promise) || Self::sigint_requested(sigint) { + return; + } + // SIGINT is delivered without SA_RESTART, so this wakes with EINTR. + vm.as_mut().auto_tick(); } + } - if actual_result.is_undefined() { - if self.use_colors { - self.print(format_args!("{}undefined{}\n", Color::DIM, Color::RESET)); - } else { - self.print(format_args!("undefined\n")); + fn evaluate_and_print(&mut self, code: &[u8]) { + let Some(global) = self.global else { + return; + }; + let Some(vm) = self.vm else { + return; + }; + + // Transform the code using REPL mode (hoists declarations, wraps result in { value: expr }) + let Some(transformed_code) = self.transform_for_repl(code) else { + // Transform failed, try evaluating raw code (for syntax errors, etc.) + self.evaluate_raw(code); + return; + }; + + match self.evaluate_transformed(&transformed_code) { + EvalOutcome::Error(error) => { + self.set_last_error(error); + let global_this = global.to_js_value(); + global_this.put(global, b"_error", error); + self.print_js_error(error); + } + EvalOutcome::Pending => self.print(format_args!("\n")), + EvalOutcome::Value(value) => { + self.set_last_result(value); + if value.is_undefined() { + if self.use_colors { + self.print(format_args!("{}undefined{}\n", Color::DIM, Color::RESET)); + } else { + self.print(format_args!("undefined\n")); + } + } else { + // Set `_` to the last result (only if not undefined) + let global_this = global.to_js_value(); + global_this.put(global, b"_", value); + self.print_formatted_value(value); + } } - } else { - self.print_formatted_value(actual_result); } // Tick the event loop to handle any pending work @@ -1584,90 +1628,24 @@ impl<'a> Repl<'a> { return; }; - let mut exception: JSValue = JSValue::UNDEFINED; - // SAFETY: `global` is a live opaque `JSGlobalObject` handle; slice ptr/len pairs - // are valid for the duration of the call; `exception` is a stack local. - let result = unsafe { - Bun__REPL__evaluate( - global, - transformed_code.as_ptr(), - transformed_code.len(), - b"[repl]".as_ptr(), - b"[repl]".len(), - &raw mut exception, - ) - }; - - if !exception.is_undefined() && !exception.is_null() { - self.set_last_error(exception); - self.print_js_error(exception); - return; - } - - let mut resolved_result = result; - if let Some(promise) = result.as_promise() { - // SAFETY: `promise` is a live JSC heap cell; `vm.jsc_vm` is the - // owning JSC VM handle for this thread. - jsc::JSPromise::opaque_mut(promise).set_handled(); - self.enable_signals_during_wait(); - // Note: reshaped for borrowck — disable_signals_during_wait called on each path - vm.as_mut() - .wait_for_promise(jsc::AnyPromise::Normal(promise)); - if vm.jsc_vm().execution_forbidden() { - vm.jsc_vm().set_execution_forbidden(false); - global.clear_termination_exception(); - self.print(format_args!("\n")); - self.disable_signals_during_wait(); - return; + match self.evaluate_transformed(&transformed_code) { + EvalOutcome::Error(error) => { + self.set_last_error(error); + self.print_js_error(error); } - let jsc_vm_ref = vm.jsc_vm(); - match jsc::JSPromise::opaque_mut(promise).status() { - PromiseStatus::Fulfilled => { - resolved_result = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref) - } - PromiseStatus::Rejected => { - let rejection = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref); - self.set_last_error(rejection); - self.print_js_error(rejection); - self.disable_signals_during_wait(); - return; + EvalOutcome::Pending => {} + EvalOutcome::Value(value) => { + self.set_last_result(value); + if !value.is_undefined() { + let global_this = global.to_js_value(); + global_this.put(global, b"_", value); } - PromiseStatus::Pending => { - self.disable_signals_during_wait(); - return; + if let Err(err) = self.copy_value_to_clipboard(value) { + let exc = global.take_exception(err); + self.set_last_error(exc); + self.print_js_error(exc); } } - self.disable_signals_during_wait(); - } - - let mut actual_result = resolved_result; - if resolved_result.is_object() { - let maybe_value = - match resolved_result.get_own(global, &bun_core::String::static_("value")) { - Ok(v) => v, - Err(err) => { - let exc = global.take_exception(err); - self.set_last_error(exc); - self.print_js_error(exc); - vm.as_mut().tick(); - return; - } - }; - if let Some(value) = maybe_value { - actual_result = value; - } - } - - self.set_last_result(actual_result); - if !actual_result.is_undefined() { - let global_this = global.to_js_value(); - global_this.put(global, b"_", actual_result); - } - - if let Err(err) = self.copy_value_to_clipboard(actual_result) { - let exc = global.take_exception(err); - self.set_last_error(exc); - self.print_js_error(exc); } vm.as_mut().tick(); } @@ -2435,23 +2413,6 @@ impl<'a> Drop for Repl<'a> { } } -/// Global pointer for signal handler to access the VM. -// PORTING.md §Global mutable state: read from a signal handler → AtomicPtr. -// Atomics are async-signal-safe; the previous raw-global `Option<*mut>` was -// not. `null` encodes `None`. -static SIGINT_VM: core::sync::atomic::AtomicPtr = - core::sync::atomic::AtomicPtr::new(core::ptr::null_mut()); - -#[cfg(unix)] -extern "C" fn sigint_handler(_: c_int) { - let vm = SIGINT_VM.load(core::sync::atomic::Ordering::Acquire); - if !vm.is_null() { - // `vm` was a valid `*mut jsc::VM` when stored (JS thread is - // blocked in wait while the handler runs, so it stays valid). - jsc::VM::opaque_ref(vm).set_execution_forbidden(true); - } -} - fn is_incomplete_code(code: &[u8]) -> bool { let mut brace_count: i32 = 0; let mut bracket_count: i32 = 0; diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 0d8e2ce79af9..f87965f62292 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -51,27 +51,30 @@ async function withTerminalRepl( let cursor = 0; let resolveWaiter: (() => void) | null = null; - await using terminal = new Bun.Terminal({ - cols: 120, - rows: 40, - data(_term, data) { - const str = Buffer.from(data).toString(); - received.push(str); - if (resolveWaiter) { - resolveWaiter(); - resolveWaiter = null; - } - }, - }); - + // The inline `terminal` option is what makes the child a session leader with + // the pty as its controlling terminal, so Ctrl+C reaches it as SIGINT. + // Handing `Bun.spawn` an already-created `Bun.Terminal` skips that setup. await using proc = Bun.spawn({ cmd: [bunExe(), "repl"], - terminal, env: { ...bunEnv, TERM: "xterm-256color", }, + terminal: { + cols: 120, + rows: 40, + data(_term, data) { + const str = Buffer.from(data).toString(); + received.push(str); + if (resolveWaiter) { + resolveWaiter(); + resolveWaiter = null; + } + }, + }, }); + const terminal = proc.terminal!; + using closeTerminal = { [Symbol.dispose]: () => terminal.close() }; const send = (text: string) => terminal.write(text); @@ -91,11 +94,14 @@ async function withTerminalRepl( `Timed out waiting for pattern: ${pattern}\nReceived so far:\n${stripAnsi(received.join("").slice(cursor))}`, ); } - // Wait for the next chunk of terminal data (or time out). - - await new Promise(resolve => { - resolveWaiter = resolve; - }); + // Wait for the next chunk of terminal data, or for the deadline: without + // the race this sleeps forever when the child goes quiet. + await Promise.race([ + new Promise(resolve => { + resolveWaiter = resolve; + }), + Bun.sleep(remaining), + ]); resolveWaiter = null; } }; @@ -993,6 +999,43 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { }); }); + // The REPL used to stay in raw mode while evaluating, so Ctrl+C was delivered + // as a byte nobody read and a synchronous loop could only be escaped by + // killing the process (which left the terminal in raw mode). + test("Ctrl+C interrupts a synchronous infinite loop", async () => { + await withTerminalRepl(async ({ send, waitFor }) => { + // Print from inside the evaluation so the interrupt is sent only once the + // loop is actually running. Split the literal so the echoed input line + // can't satisfy the wait. + send(`process.stdout.write("RUN" + "NING\\n"); while (true) {}\n`); + await waitFor("RUNNING"); + send("\x03"); // Ctrl+C + const output = await waitFor(/interrupted/); + expect(stripAnsi(output)).toContain("Script execution was interrupted by `SIGINT`"); + + // And the session keeps working. + send("111 + 222\n"); + await waitFor(/\n\s*333\b/); + }); + }); + + test("Ctrl+C during a never-settling await leaves the REPL usable", async () => { + await withTerminalRepl(async ({ send, waitFor }) => { + // The executor runs synchronously, so printing from it marks the point + // where the REPL starts waiting. The long timer keeps the loop alive, so + // the wait parks in the poller exactly like a real pending `await` would. + send(`await new Promise(() => { process.stdout.write("WAIT" + "ING\\n"); setTimeout(() => {}, 600000); })\n`); + await waitFor("WAITING"); + send("\x03"); // Ctrl+C + await waitFor(/interrupted/); + + // Interrupting used to leave the VM permanently execution-forbidden, + // which silently dropped every microtask from then on. + send("await Promise.resolve(1234 * 1000)\n"); + await waitFor(/\n\s*1234000\b/); + }); + }); + test("require works in terminal", async () => { await withTerminalRepl(async ({ send, waitFor }) => { send("typeof require\n"); From 716661b6c1162ade5c3df943b9d84ff545b35ad3 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 03:04:48 +0000 Subject: [PATCH 02/11] repl: restore raw mode before disarming, so no exit path can leave the prompt cooked --- src/runtime/cli/repl.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index 56406ced9529..7d529ed2059f 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -1021,16 +1021,18 @@ impl<'a> Repl<'a> { } fn end_interruptible_eval(&mut self, scope: Option) { - let Some(global) = self.global else { - return; - }; let Some(scope) = scope else { return; }; + // Before anything fallible: leaving the prompt in cooked mode is worse + // than any error we could hit below. #[cfg(unix)] let _ = self.tty_state.set_mode(0, tty::Mode::Raw); + // A scope is only handed out once `begin_interruptible_eval` has seen a + // global, and `self.global` is set once per session. + let global = self.global.expect("sigint scope armed without a global"); // SAFETY: `scope.scope` came from `Bun__REPL__armSigint` and is consumed // once; `global` is a live opaque `JSGlobalObject` handle. unsafe { Bun__REPL__disarmSigint(global, scope.scope) }; From 83bcf86899f6bf0b2b9134d0d0e22c9d50fe6839 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 05:14:55 +0000 Subject: [PATCH 03/11] repl: survive a nested node:vm call, and drop the dead execution-forbidden setter Arming the SIGINT watcher around every REPL evaluation made the REPL an outer `SigintWatcher` holder, which exposed two bugs in code the REPL now reaches: A nested `vm.runInThisContext(src)` with no `breakOnSigint` aborted the process on Ctrl+C. The inner script's termination reached `checkForTermination` with neither a SIGINT flag of its own nor a timeout, and fell into `RELEASE_ASSERT_NOT_REACHED`. An enclosing scope's SIGINT is still a SIGINT, so report it as one. `NodeVMModule` had the same shape. A nested `vm.runInThisContext(src, { breakOnSigint: true })` silently disarmed the REPL: `registerGlobalObject` skipped the duplicate while `unregisterGlobalObject` removed one entry unconditionally, so the inner holder took the outer holder's registration with it. Append unconditionally to balance the two. `JSC::VM::setExecutionForbidden` lost its last caller when the old interrupt path went away. The C++ shim ignored its `bool`, so `set_execution_forbidden(false)` never cleared anything and was a trap for the next caller. The getter stays. Also corrects the promise-wait comment: every poll path in `us_loop_run_bun_tick` re-enters on EINTR, so a signal does not wake a parked loop. The wait is only prompt because an idle loop polls without blocking, which is what the test now exercises. --- src/jsc/VM.rs | 5 ---- src/jsc/bindings/NodeVMModule.cpp | 5 +++- src/jsc/bindings/NodeVMScript.cpp | 5 +++- src/jsc/bindings/bindings.cpp | 5 ---- src/jsc/bindings/headers.h | 1 - src/jsc/bindings/vm/SigintWatcher.cpp | 7 +++++- src/runtime/cli/repl.rs | 5 +++- test/js/bun/repl/repl.test.ts | 36 ++++++++++++++++++++++++--- 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index bfe379e7e722..8ace60a90577 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -32,7 +32,6 @@ unsafe extern "C" { safe fn JSC__VM__runGC(vm: &VM, sync: bool) -> usize; safe fn JSC__VM__heapSize(vm: &VM) -> usize; safe fn JSC__VM__collectAsync(vm: &VM); - safe fn JSC__VM__setExecutionForbidden(vm: &VM, forbidden: bool); safe fn JSC__VM__setExecutionTimeLimit(vm: &VM, timeout: f64); safe fn JSC__VM__clearExecutionTimeLimit(vm: &VM); safe fn JSC__VM__executionForbidden(vm: &VM) -> bool; @@ -130,10 +129,6 @@ impl VM { JSC__VM__collectAsync(self) } - pub fn set_execution_forbidden(&self, forbidden: bool) { - JSC__VM__setExecutionForbidden(self, forbidden) - } - pub fn set_execution_time_limit(&self, timeout: f64) { JSC__VM__setExecutionTimeLimit(self, timeout) } diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 23292142dc32..9dedfd49779b 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -254,7 +254,10 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b } else if (timeout != 0) { throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } else { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.SourceTextModule evaluation terminated due neither to SIGINT nor to timeout"); + // Terminated by an enclosing scope that armed the watcher itself — + // the REPL's Ctrl+C, say. Still a SIGINT, so report it as one + // rather than asserting on a reachable state. + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); } } else { setSigintReceived(false); diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 90d1df51b524..ab1e610dcc3e 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -297,7 +297,10 @@ static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, } else if (timeout) { throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); } else { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.Script terminated due neither to SIGINT nor to timeout"); + // Terminated by an enclosing scope that armed the watcher itself — + // the REPL's Ctrl+C, say. Still a SIGINT, so report it as one + // rather than asserting on a reachable state. + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); } return true; } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index cc37968ab216..8b4d3fcdef51 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -4831,11 +4831,6 @@ bool JSC__VM__hasTerminationRequest(JSC::VM* vm) return vm->hasTerminationRequest(); } -void JSC__VM__setExecutionForbidden(JSC::VM* arg0, bool arg1) -{ - (*arg0).setExecutionForbidden(); -} - // These may be called concurrently from another thread. void JSC__VM__notifyNeedTermination(JSC::VM* arg0) { diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index 750b3ac81d6b..099821bbdf36 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -318,7 +318,6 @@ CPP_DECL void JSC__VM__notifyNeedWatchdogCheck(JSC::VM* arg0); CPP_DECL void JSC__VM__releaseWeakRefs(JSC::VM* arg0); CPP_DECL size_t JSC__VM__runGC(JSC::VM* arg0, bool arg1); CPP_DECL void JSC__VM__setControlFlowProfiler(JSC::VM* arg0, bool arg1); -CPP_DECL void JSC__VM__setExecutionForbidden(JSC::VM* arg0, bool arg1); CPP_DECL void JSC__VM__setExecutionTimeLimit(JSC::VM* arg0, double arg1); CPP_DECL void JSC__VM__shrinkFootprint(JSC::VM* arg0); CPP_DECL void JSC__VM__throwError(JSC::VM* arg0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2); diff --git a/src/jsc/bindings/vm/SigintWatcher.cpp b/src/jsc/bindings/vm/SigintWatcher.cpp index 0c2c99a1aa7e..fb1c67eae906 100644 --- a/src/jsc/bindings/vm/SigintWatcher.cpp +++ b/src/jsc/bindings/vm/SigintWatcher.cpp @@ -119,7 +119,12 @@ void SigintWatcher::registerGlobalObject(JSGlobalObject* globalObject) } WTF::Locker lock(m_globalObjectsMutex); - m_globalObjects.appendIfNotContains(globalObject); + // Append unconditionally: `unregisterGlobalObject` removes exactly one + // entry, so skipping a duplicate would let a nested holder (an inner + // `runInThisContext({ breakOnSigint: true })`) unregister the outer + // holder's global on the way out. `signalAll` tolerates duplicates — + // `notifyNeedTermination` only re-sets an already-set trap bit. + m_globalObjects.append(globalObject); } void SigintWatcher::unregisterGlobalObject(JSGlobalObject* globalObject) diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index 7d529ed2059f..5db63cfc9b44 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -1392,7 +1392,10 @@ impl<'a> Repl<'a> { if !pending(promise) || Self::sigint_requested(sigint) { return; } - // SIGINT is delivered without SA_RESTART, so this wakes with EINTR. + // Non-blocking while the loop is idle, so the flag above is seen + // promptly. With live handles this parks in the poller, which every + // path in `us_loop_run_bun_tick` re-enters on EINTR, so a Ctrl+C that + // lands after the park waits for whatever wakes the loop next. vm.as_mut().auto_tick(); } } diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index f87965f62292..dc63b2bfc202 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -1022,9 +1022,9 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { test("Ctrl+C during a never-settling await leaves the REPL usable", async () => { await withTerminalRepl(async ({ send, waitFor }) => { // The executor runs synchronously, so printing from it marks the point - // where the REPL starts waiting. The long timer keeps the loop alive, so - // the wait parks in the poller exactly like a real pending `await` would. - send(`await new Promise(() => { process.stdout.write("WAIT" + "ING\\n"); setTimeout(() => {}, 600000); })\n`); + // where the REPL starts waiting. Nothing keeps the loop alive, so the wait + // never parks in the poller and the interrupt is picked up immediately. + send(`await new Promise(() => { process.stdout.write("WAIT" + "ING\\n"); })\n`); await waitFor("WAITING"); send("\x03"); // Ctrl+C await waitFor(/interrupted/); @@ -1036,6 +1036,36 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { }); }); + // The inner script's termination reached `checkForTermination` with neither a + // SIGINT flag of its own nor a timeout, which used to abort the process. + test("Ctrl+C during a nested vm.runInThisContext does not abort", async () => { + await withTerminalRepl(async ({ send, waitFor }) => { + // No `\n` escape inside the nested string: the REPL mishandles one there + // (pre-existing, reproduces on release bun), and the marker doesn't need it. + send(`require('vm').runInThisContext('process.stdout.write("IN"+"NER"); while (true) {}')\n`); + await waitFor("INNER"); + send("\x03"); // Ctrl+C + await waitFor(/interrupted/); + + // Reaching a result at all proves the process survived the interrupt. + send("111 + 222\n"); + await waitFor(/\n\s*333\b/); + }); + }); + + // The inner `breakOnSigint` holder used to unregister the REPL's own global on + // the way out, leaving the rest of the evaluation uninterruptible. + test("Ctrl+C still interrupts after a nested breakOnSigint script", async () => { + await withTerminalRepl(async ({ send, waitFor }) => { + send( + `require('vm').runInThisContext('1', { breakOnSigint: true }); process.stdout.write("AFT"+"ER\\n"); while (true) {}\n`, + ); + await waitFor("AFTER"); + send("\x03"); // Ctrl+C + await waitFor(/interrupted/); + }); + }); + test("require works in terminal", async () => { await withTerminalRepl(async ({ send, waitFor }) => { send("typeof require\n"); From c48383e3344524f530fa08e370e1682c3031beff Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 06:23:44 +0000 Subject: [PATCH 04/11] node:vm: let the scope that armed a termination classify it The branch added for a nested script terminated from the outside reported every such termination as a SIGINT. It is also reached when an *outer* `runInThisContext({ timeout: N })` watchdog fires while a nested no-options script is on the stack, where no signal is involved at all: const vm = require("vm"); globalThis.inner = () => vm.runInThisContext("while (true) {}"); vm.runInThisContext("inner()", { timeout: 100 }); // was: ERR_SCRIPT_EXECUTION_INTERRUPTED, "interrupted by `SIGINT`" // node: ERR_SCRIPT_EXECUTION_TIMEOUT, "timed out after 100ms" A nested script cannot classify a termination it did not request: it only knows its own SIGINT flag and its own timeout. So when neither is set, re-raise the termination instead of converting it, leaving `hasTerminationRequest()` intact. The enclosing scope's `checkForTermination` then picks the right branch with its own limit and receiver flag, and the REPL's `Bun__REPL__evaluate` still sees a termination exception and reports the interrupt. `NodeVMModule` had the same shape. Now matches node on all three paths. --- src/jsc/bindings/NodeVMModule.cpp | 15 ++++++----- src/jsc/bindings/NodeVMScript.cpp | 43 +++++++++++++++++-------------- test/js/node/vm/vm.test.ts | 17 ++++++++++++ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 9dedfd49779b..92d58401f98c 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -245,19 +245,22 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b // so the exception-check validator is satisfied before the TOP scope. std::ignore = scope.exception(); if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { + // Neither this module's own SIGINT nor its own timeout: an enclosing + // scope asked for the termination — an outer `timeout`, or the REPL's + // Ctrl+C watcher. Only that scope can classify it, so re-raise and let + // it report. + if (!getSigintReceived() && timeout == 0) { + JSC::throwException(globalObject, scope, vm.ensureTerminationException()); + return {}; + } vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); vm.clearHasTerminationRequest(); if (getSigintReceived()) { setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else if (timeout != 0) { - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } else { - // Terminated by an enclosing scope that armed the watcher itself — - // the REPL's Ctrl+C, say. Still a SIGINT, so report it as one - // rather than asserting on a reachable state. - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } } else { setSigintReceived(false); diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index ab1e610dcc3e..d9ac97f6fbf9 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -283,29 +283,32 @@ void NodeVMScript::destroy(JSCell* cell) static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional timeout) { - if (vm.hasTerminationRequest()) { - vm.drainMicrotasksForGlobalObject(globalObject); - // The termination may have fired inside an afterEvaluate microtask - // checkpoint, leaving the termination exception pending; clear it so - // the ERR_SCRIPT_EXECUTION_* error below replaces it. - if (vm.hasPendingTerminationException()) - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); - if (script->getSigintReceived()) { - script->setSigintReceived(false); - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else if (timeout) { - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); - } else { - // Terminated by an enclosing scope that armed the watcher itself — - // the REPL's Ctrl+C, say. Still a SIGINT, so report it as one - // rather than asserting on a reachable state. - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } + if (!vm.hasTerminationRequest()) + return false; + + // Neither this script's own SIGINT nor its own timeout: an enclosing scope + // asked for the termination — an outer `timeout`, or the REPL's Ctrl+C + // watcher. Only that scope can classify it, so re-raise and let its own + // `checkForTermination` (or `Bun__REPL__evaluate`) report it. + if (!script->getSigintReceived() && !timeout) { + JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return true; } - return false; + vm.drainMicrotasksForGlobalObject(globalObject); + // The termination may have fired inside an afterEvaluate microtask + // checkpoint, leaving the termination exception pending; clear it so + // the ERR_SCRIPT_EXECUTION_* error below replaces it. + if (vm.hasPendingTerminationException()) + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); + if (script->getSigintReceived()) { + script->setSigintReceived(false); + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); + } else { + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); + } + return true; } void setupWatchdog(VM& vm, double timeout, double* oldTimeout, double* newTimeout) diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 81e49e2e84b3..cc9fc37c340c 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -86,6 +86,23 @@ describe("vm", () => { }); expect(result).toBe(2); }); + + // An outer watchdog firing while a nested no-options script is on the stack + // used to abort the process, and then to report it as a SIGINT. Only the + // scope that armed the termination can classify it. + test("an outer timeout reports ERR_SCRIPT_EXECUTION_TIMEOUT through a nested script", () => { + (globalThis as any).__nestedSpin = () => runInThisContext("while (true) {}"); + try { + expect(() => runInThisContext("__nestedSpin()", { timeout: 100 })).toThrow( + expect.objectContaining({ + code: "ERR_SCRIPT_EXECUTION_TIMEOUT", + message: "Script execution timed out after 100ms", + }), + ); + } finally { + delete (globalThis as any).__nestedSpin; + } + }); }); describe("compileFunction()", () => { From 7f264e79c707412d4099086f7e83deb8a0be080e Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 07:55:53 +0000 Subject: [PATCH 05/11] node:vm: lock in that a re-raised termination isn't stored on the module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Falling through to VM_RETURN_IF_EXCEPTION in the re-raise branch looks like the tidier shape — it is what the sibling SIGINT/timeout branches do — but it stores the VM's singleton TerminationException in `m_evaluationException`. Re-throwing that once the request has been cleared trips ASSERTION FAILED: !isTerminationException(exception) || hasTerminationRequest() JSC::VM::setException(Exception *) and aborts. `reconcileEvaluationState` settles the status lazily instead, wrapping the error *value* in a fresh Exception that is safe to re-throw, so the module still ends up `errored`. Record why the early return is load-bearing, and add a regression test that re-evaluates the errored module — the step where the stored exception is re-thrown. The test exits 134 (SIGABRT) against the fall-through shape. --- src/jsc/bindings/NodeVMModule.cpp | 8 ++++++ test/js/node/vm/vm.test.ts | 46 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 92d58401f98c..b41cbdccc6f7 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -249,6 +249,14 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b // scope asked for the termination — an outer `timeout`, or the REPL's // Ctrl+C watcher. Only that scope can classify it, so re-raise and let // it report. + // + // Returning here instead of falling through to VM_RETURN_IF_EXCEPTION is + // load-bearing: that macro would store the VM's singleton + // TerminationException on the module, and re-throwing it once the request + // is cleared trips `VM::setException`'s + // `!isTerminationException(e) || hasTerminationRequest()` assertion. + // `reconcileEvaluationState` settles the status instead, wrapping the + // error *value* in a fresh Exception that is safe to re-throw. if (!getSigintReceived() && timeout == 0) { JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return {}; diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index cc9fc37c340c..d0cc4c3de3ae 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -958,6 +958,52 @@ test("Loader is not defined in vm context", () => { expect(runInContext("typeof Loader.registry;", customContext)).toBe("undefined"); }); +// Re-raising an enclosing scope's termination must not hand the VM's singleton +// TerminationException to the module: storing it and re-throwing it once the +// request is cleared trips `VM::setException`'s +// `!isTerminationException(e) || hasTerminationRequest()` assertion. +test("a module terminated by an enclosing scope ends up errored without aborting", async () => { + const fixture = ` + const vm = require("node:vm"); + const m = new vm.SourceTextModule("while (true) {}"); + await m.link(() => {}); + globalThis.__run = () => { + const p = m.evaluate(); + p.catch(() => {}); + return p; + }; + try { + vm.runInThisContext("__run()", { timeout: 100 }); + console.log("outer=NO_THROW"); + } catch (e) { + console.log("outer=" + e.code); + } + console.log("status=" + m.status); + try { + m.error; + console.log("error=readable"); + } catch (e) { + console.log("error=threw:" + e.code); + } + // Re-evaluating an errored module re-throws the error it stored. That is + // where a stored TerminationException trips the assertion. + try { + const again = m.evaluate(); + again.catch(() => {}); + console.log("reeval=ok"); + } catch (e) { + console.log("reeval=threw:" + (e.code ?? e.name)); + } + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim().split("\n"), exitCode }).toEqual({ + stdout: ["outer=ERR_SCRIPT_EXECUTION_TIMEOUT", "status=errored", "error=readable", "reeval=ok"], + exitCode: 0, + }); + capture(stderr); +}); + test("node:vm native Module prototype methods reject non-module receivers", async () => { // The native NodeVMModule prototype (reachable via the kNative own-symbol on a // vm.SourceTextModule instance) must validate its receiver. Calling its methods From 357988075b4a65130ec8b020f4f4fbe0f22b34e7 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 09:00:46 +0000 Subject: [PATCH 06/11] repl: stop swallowing an external SIGINT, and address review nits `SigintWatcher::uninstall()` hardcoded `Bun__onPosixSignal` + `SA_RESTART` instead of restoring what `install()` displaced. Arming the watcher around every REPL evaluation made that hot: after the first command, `kill -INT` on a `bun repl` was enqueued, dropped for want of a listener, and the startup handler that restores the terminal on the way out never ran again. release: kill -INT at the prompt -> dies, terminal restored before: kill -INT at the prompt -> swallowed, terminal left raw `install()` now saves the displaced disposition and `uninstall()` puts it back, which also stops a plain `vm.runInThisContext(x, { breakOnSigint: true })` from leaking the same change into the rest of the process. `registerReceiver` had the same `appendIfNotContains`-vs-unconditional-remove asymmetry as `registerGlobalObject`: nested holders on one `NodeVMScript` let the inner unregister the outer's receiver. `setSigintReceived` is an idempotent atomic store, so appending is safe. Also from review: - Root the promise across `wait_for_promise_interruptible`; ticking can collect. - Leave the promise wait once the loop has no work, rather than burning a core on an `await` nothing can ever settle (201 CPU ticks/2s -> 0). - `drainMicrotasksForGlobalObject(globalObject)`: `nodeVmGlobalObject` is nullable and `globalObject` already resolves to it when present. - Keep `_error` in sync on `.copy`'s failure paths. - Trim four comments to the 3-line limit; capture stderr before asserting. --- src/jsc/bindings/NodeVMModule.cpp | 19 ++++++------------ src/jsc/bindings/NodeVMScript.cpp | 7 +++---- src/jsc/bindings/bindings.cpp | 14 ++++++-------- src/jsc/bindings/vm/SigintWatcher.cpp | 28 +++++++++++++-------------- src/jsc/bindings/vm/SigintWatcher.h | 4 ++++ src/runtime/cli/repl.rs | 16 +++++++++++---- test/js/bun/repl/repl.test.ts | 25 ++++++++++++++++++++---- test/js/node/vm/vm.test.ts | 3 ++- 8 files changed, 68 insertions(+), 48 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index b41cbdccc6f7..23d5361dad56 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -245,23 +245,16 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b // so the exception-check validator is satisfied before the TOP scope. std::ignore = scope.exception(); if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { - // Neither this module's own SIGINT nor its own timeout: an enclosing - // scope asked for the termination — an outer `timeout`, or the REPL's - // Ctrl+C watcher. Only that scope can classify it, so re-raise and let - // it report. - // - // Returning here instead of falling through to VM_RETURN_IF_EXCEPTION is - // load-bearing: that macro would store the VM's singleton - // TerminationException on the module, and re-throwing it once the request - // is cleared trips `VM::setException`'s - // `!isTerminationException(e) || hasTerminationRequest()` assertion. - // `reconcileEvaluationState` settles the status instead, wrapping the - // error *value* in a fresh Exception that is safe to re-throw. + // An enclosing scope asked for the termination; only it can classify it. + // Returning rather than falling through is load-bearing: VM_RETURN_IF_EXCEPTION + // would store the singleton TerminationException, whose later re-throw trips + // `VM::setException`. `reconcileEvaluationState` settles the status safely. if (!getSigintReceived() && timeout == 0) { JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return {}; } - vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); + // `globalObject` is the NodeVM global when there is one, non-null otherwise. + vm.drainMicrotasksForGlobalObject(globalObject); DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); vm.clearHasTerminationRequest(); if (getSigintReceived()) { diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index d9ac97f6fbf9..48a70a1f7d3b 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -286,10 +286,9 @@ static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, if (!vm.hasTerminationRequest()) return false; - // Neither this script's own SIGINT nor its own timeout: an enclosing scope - // asked for the termination — an outer `timeout`, or the REPL's Ctrl+C - // watcher. Only that scope can classify it, so re-raise and let its own - // `checkForTermination` (or `Bun__REPL__evaluate`) report it. + // Neither this script's own SIGINT nor its own timeout, so an enclosing scope + // asked for the termination. Only that scope can classify it: re-raise and let + // its `checkForTermination` (or `Bun__REPL__evaluate`) report. if (!script->getSigintReceived() && !timeout) { JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return true; diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 8b4d3fcdef51..7727f6ecbc7a 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -6263,10 +6263,9 @@ CPP_DECL [[ZIG_EXPORT(nothrow)]] unsigned int Bun__CallFrame__getLineNumber(JSC: return lineColumn.line; } -// Armed around one REPL evaluation. The watcher thread flips the receiver flag -// before raising the termination trap, and that flag is the only durable record -// of the signal: JSC clears both the trap bit and `hasTerminationRequest()` by -// the time the outermost VM entry scope has unwound. +// Armed around one REPL evaluation. The receiver flag is the only durable record +// of the signal: JSC clears the trap bit and `hasTerminationRequest()` by the time +// the outermost VM entry scope has unwound. namespace { class ReplSigintScope final : public Bun::SigintReceiver { public: @@ -6291,10 +6290,9 @@ static void replClearTermination(JSC::VM& vm) vm.clearHasTerminationRequest(); } -// Arms the SIGINT watcher for `globalObject`. While armed, a SIGINT raises a -// JSC termination trap, which unwinds synchronous JavaScript (`while (true) {}`) -// the same way node's `breakOnSigint` does. The returned scope must be passed to -// `Bun__REPL__disarmSigint`. +// Arms SIGINT watching for `globalObject`: a SIGINT then raises a JSC termination +// trap that unwinds synchronous JS, the way node's `breakOnSigint` does. Pass the +// returned scope to `Bun__REPL__disarmSigint`. extern "C" void* Bun__REPL__armSigint(JSC::JSGlobalObject* globalObject) { auto& vm = JSC::getVM(globalObject); diff --git a/src/jsc/bindings/vm/SigintWatcher.cpp b/src/jsc/bindings/vm/SigintWatcher.cpp index fb1c67eae906..aa4cf8d07c2b 100644 --- a/src/jsc/bindings/vm/SigintWatcher.cpp +++ b/src/jsc/bindings/vm/SigintWatcher.cpp @@ -51,7 +51,9 @@ void SigintWatcher::install() sigaddset(&action.sa_mask, SIGINT); action.sa_flags = 0; - sigaction(SIGINT, &action, nullptr); + // Save what we displace. `ref()` only reaches install() on the 0 -> 1 + // transition, so this cannot capture our own handler. + sigaction(SIGINT, &action, &m_previousAction); #endif if (m_installed.exchange(true)) { @@ -90,13 +92,10 @@ void SigintWatcher::uninstall() #if OS(WINDOWS) SetConsoleCtrlHandler(WindowsCtrlHandler, false); #else - struct sigaction action; - memset(&action, 0, sizeof(struct sigaction)); - action.sa_handler = Bun__onPosixSignal; - sigemptyset(&action.sa_mask); - sigaddset(&action.sa_mask, SIGINT); - action.sa_flags = SA_RESTART; - sigaction(SIGINT, &action, nullptr); + // Put back exactly what install() displaced. Hardcoding Bun__onPosixSignal + // here would swallow SIGINT for a process that never registered a + // listener, clobbering the startup handler that restores the terminal. + sigaction(SIGINT, &m_previousAction, nullptr); #endif m_semaphore.signal(); @@ -119,11 +118,9 @@ void SigintWatcher::registerGlobalObject(JSGlobalObject* globalObject) } WTF::Locker lock(m_globalObjectsMutex); - // Append unconditionally: `unregisterGlobalObject` removes exactly one - // entry, so skipping a duplicate would let a nested holder (an inner - // `runInThisContext({ breakOnSigint: true })`) unregister the outer - // holder's global on the way out. `signalAll` tolerates duplicates — - // `notifyNeedTermination` only re-sets an already-set trap bit. + // Append unconditionally so a nested holder unregisters only its own entry. + // `signalAll` tolerates duplicates: `notifyNeedTermination` just re-sets an + // already-set trap bit. m_globalObjects.append(globalObject); } @@ -151,7 +148,10 @@ void SigintWatcher::registerReceiver(SigintReceiver* module) } WTF::Locker lock(m_receiversMutex); - m_receivers.appendIfNotContains(module); + // Append unconditionally, for the same reason as registerGlobalObject: + // `unregisterReceiver` removes one entry. `setSigintReceived` is an + // idempotent atomic store, so duplicates are harmless to `signalAll`. + m_receivers.append(module); } void SigintWatcher::unregisterReceiver(SigintReceiver* module) diff --git a/src/jsc/bindings/vm/SigintWatcher.h b/src/jsc/bindings/vm/SigintWatcher.h index 4b5ff92deb44..9fabe94fa476 100644 --- a/src/jsc/bindings/vm/SigintWatcher.h +++ b/src/jsc/bindings/vm/SigintWatcher.h @@ -106,6 +106,10 @@ class SigintWatcher { WTF::Vector m_globalObjects; WTF::Vector m_receivers; uint32_t m_refCount = 0; +#if !OS(WINDOWS) + // The disposition install() displaced, so uninstall() can put it back. + struct sigaction m_previousAction {}; +#endif bool signalAll(); }; diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index 5db63cfc9b44..a32248ca4931 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -1335,6 +1335,8 @@ impl<'a> Repl<'a> { let mut outcome = if !exception.is_undefined() && !exception.is_null() { EvalOutcome::Error(exception) } else if let Some(promise) = result.as_promise() { + // The wait ticks the VM, which can collect; keep the promise rooted. + let _rooted = result.protected(); // Mark as handled BEFORE waiting to prevent unhandled rejection output jsc::JSPromise::opaque_mut(promise).set_handled(); self.wait_for_promise_interruptible(promise, sigint.as_ref()); @@ -1392,10 +1394,14 @@ impl<'a> Repl<'a> { if !pending(promise) || Self::sigint_requested(sigint) { return; } - // Non-blocking while the loop is idle, so the flag above is seen - // promptly. With live handles this parks in the poller, which every - // path in `us_loop_run_bun_tick` re-enters on EINTR, so a Ctrl+C that - // lands after the park waits for whatever wakes the loop next. + // With no live handles `auto_tick` never blocks, so nothing can settle + // the promise and looping would just burn a core. + if !vm.is_event_loop_alive() { + return; + } + // Parks in the poller. Every path in `us_loop_run_bun_tick` re-enters + // it on EINTR, so a Ctrl+C landing after the park is only seen once + // something else wakes the loop. vm.as_mut().auto_tick(); } } @@ -1636,6 +1642,7 @@ impl<'a> Repl<'a> { match self.evaluate_transformed(&transformed_code) { EvalOutcome::Error(error) => { self.set_last_error(error); + global.to_js_value().put(global, b"_error", error); self.print_js_error(error); } EvalOutcome::Pending => {} @@ -1648,6 +1655,7 @@ impl<'a> Repl<'a> { if let Err(err) = self.copy_value_to_clipboard(value) { let exc = global.take_exception(err); self.set_last_error(exc); + global.to_js_value().put(global, b"_error", exc); self.print_js_error(exc); } } diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index dc63b2bfc202..84d4b20af21c 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -999,6 +999,23 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { }); }); + // Arming the SIGINT watcher around every evaluation must put the previous + // disposition back, or an external SIGINT is swallowed from then on and the + // startup handler that restores the terminal never runs. + test("an external SIGINT at the prompt still terminates the REPL", async () => { + await withTerminalRepl(async ({ send, waitFor, proc }) => { + send("1 + 1\n"); + await waitFor(/\n\s*2\b/); + + proc.kill("SIGINT"); + const exited = await Promise.race([ + proc.exited.then(() => "exited"), + Bun.sleep(4000).then(() => "still running"), + ]); + expect(exited).toBe("exited"); + }); + }); + // The REPL used to stay in raw mode while evaluating, so Ctrl+C was delivered // as a byte nobody read and a synchronous loop could only be escaped by // killing the process (which left the terminal in raw mode). @@ -1021,10 +1038,10 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { test("Ctrl+C during a never-settling await leaves the REPL usable", async () => { await withTerminalRepl(async ({ send, waitFor }) => { - // The executor runs synchronously, so printing from it marks the point - // where the REPL starts waiting. Nothing keeps the loop alive, so the wait - // never parks in the poller and the interrupt is picked up immediately. - send(`await new Promise(() => { process.stdout.write("WAIT" + "ING\\n"); })\n`); + // The executor runs synchronously, so printing from it marks the point where + // the REPL starts waiting. The short interval keeps the loop alive, so the + // wait parks in the poller but wakes often enough to notice the interrupt. + send(`await new Promise(() => { process.stdout.write("WAIT" + "ING\\n"); setInterval(() => {}, 10); })\n`); await waitFor("WAITING"); send("\x03"); // Ctrl+C await waitFor(/interrupted/); diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index d0cc4c3de3ae..58c0e9236071 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -997,11 +997,12 @@ test("a module terminated by an enclosing scope ends up errored without aborting `; await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Before the assertion: a native abort makes stderr the useful diagnostic. + capture(stderr); expect({ stdout: stdout.trim().split("\n"), exitCode }).toEqual({ stdout: ["outer=ERR_SCRIPT_EXECUTION_TIMEOUT", "status=errored", "error=readable", "reeval=ok"], exitCode: 0, }); - capture(stderr); }); test("node:vm native Module prototype methods reject non-module receivers", async () => { From f346c1ac9a530864304674a86f49dc085b94fb1b Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 10:03:13 +0000 Subject: [PATCH 07/11] SigintWatcher: only undo our own SIGINT handler on uninstall Saving the displaced disposition and restoring it unconditionally fixed the swallowed external SIGINT, but it also discarded any handler installed *while* the watcher was armed. `process.on("SIGINT", h)` inside a REPL evaluation installs BunProcess's handler and records the signal in `signalToContextIdsMap`; the disarm then threw that handler away, and because the map entry survives, BunProcess never reinstalls it. The listener was stranded for the session. release: process.on("SIGINT") then kill -INT -> repl alive, listener honored before: process.on("SIGINT") then kill -INT -> repl dies, listener discarded `uninstall()` now restores only when the handler still installed is the watcher's own, so it undoes itself and leaves anything else alone. The lambda becomes a named function so it can be compared. Two tests hold both halves: with no listener an external SIGINT must still terminate, with a listener the session must survive. The unconditional restore passes the first and fails the second. The nested-timeout vm test now runs its input as a spawned fixture, matching its sibling, so a regression back to the abort is an attributable failure rather than a dead test runner. --- src/jsc/bindings/vm/SigintWatcher.cpp | 22 ++++++++++++------ test/js/bun/repl/repl.test.ts | 14 ++++++++++++ test/js/node/vm/vm.test.ts | 33 ++++++++++++++++----------- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/jsc/bindings/vm/SigintWatcher.cpp b/src/jsc/bindings/vm/SigintWatcher.cpp index aa4cf8d07c2b..1c16edd6a5ea 100644 --- a/src/jsc/bindings/vm/SigintWatcher.cpp +++ b/src/jsc/bindings/vm/SigintWatcher.cpp @@ -20,6 +20,11 @@ static BOOL WindowsCtrlHandler(DWORD signal) return false; } +#else +static void sigintWatcherHandler(int) +{ + SigintWatcher::get().signalReceived(); +} #endif SigintWatcher::SigintWatcher() @@ -43,9 +48,7 @@ void SigintWatcher::install() struct sigaction action; memset(&action, 0, sizeof(struct sigaction)); - action.sa_handler = [](int signalNumber) { - get().signalReceived(); - }; + action.sa_handler = sigintWatcherHandler; sigemptyset(&action.sa_mask); sigaddset(&action.sa_mask, SIGINT); @@ -92,10 +95,15 @@ void SigintWatcher::uninstall() #if OS(WINDOWS) SetConsoleCtrlHandler(WindowsCtrlHandler, false); #else - // Put back exactly what install() displaced. Hardcoding Bun__onPosixSignal - // here would swallow SIGINT for a process that never registered a - // listener, clobbering the startup handler that restores the terminal. - sigaction(SIGINT, &m_previousAction, nullptr); + // Undo only our own handler. Code that ran while we were armed may have + // installed its own (`process.on("SIGINT")`), and clobbering that would + // strand the listener for good: BunProcess installs it exactly once. + struct sigaction current; + if (sigaction(SIGINT, nullptr, ¤t) == 0 + && !(current.sa_flags & SA_SIGINFO) + && current.sa_handler == sigintWatcherHandler) { + sigaction(SIGINT, &m_previousAction, nullptr); + } #endif m_semaphore.signal(); diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 84d4b20af21c..695d2d8850f8 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -1016,6 +1016,20 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { }); }); + // The disarm must not clobber a handler installed while the watcher was armed: + // BunProcess registers its SIGINT handler exactly once, so discarding it would + // strand the listener for the rest of the session. + test("a SIGINT listener registered in the REPL survives the evaluation", async () => { + await withTerminalRepl(async ({ send, waitFor, proc }) => { + send("process.on('SIGINT', () => {}); 1 + 1\n"); + await waitFor(/\n\s*2\b/); + + proc.kill("SIGINT"); + const outcome = await Promise.race([proc.exited.then(() => "exited"), Bun.sleep(2000).then(() => "alive")]); + expect(outcome).toBe("alive"); + }); + }); + // The REPL used to stay in raw mode while evaluating, so Ctrl+C was delivered // as a byte nobody read and a synchronous loop could only be escaped by // killing the process (which left the terminal in raw mode). diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 58c0e9236071..84b460276f64 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -89,19 +89,26 @@ describe("vm", () => { // An outer watchdog firing while a nested no-options script is on the stack // used to abort the process, and then to report it as a SIGINT. Only the - // scope that armed the termination can classify it. - test("an outer timeout reports ERR_SCRIPT_EXECUTION_TIMEOUT through a nested script", () => { - (globalThis as any).__nestedSpin = () => runInThisContext("while (true) {}"); - try { - expect(() => runInThisContext("__nestedSpin()", { timeout: 100 })).toThrow( - expect.objectContaining({ - code: "ERR_SCRIPT_EXECUTION_TIMEOUT", - message: "Script execution timed out after 100ms", - }), - ); - } finally { - delete (globalThis as any).__nestedSpin; - } + // scope that armed the termination can classify it. Spawned, so a regression + // back to the abort is an attributable failure rather than a dead runner. + test("an outer timeout reports ERR_SCRIPT_EXECUTION_TIMEOUT through a nested script", async () => { + const fixture = ` + const vm = require("node:vm"); + globalThis.__nestedSpin = () => vm.runInThisContext("while (true) {}"); + try { + vm.runInThisContext("__nestedSpin()", { timeout: 100 }); + console.log("NO_THROW"); + } catch (e) { + console.log(e.code + " " + e.message); + } + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + capture(stderr); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ + stdout: "ERR_SCRIPT_EXECUTION_TIMEOUT Script execution timed out after 100ms", + exitCode: 0, + }); }); }); From 0b269700b1471dac2686dc43fb8e15565a43f35b Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 6 Jul 2026 13:42:28 +0000 Subject: [PATCH 08/11] node:vm: keep the terminated-module microtask clear scoped to its own context `drainMicrotasksForGlobalObject` does not drain, it *clears*: void VM::drainMicrotasksForGlobalObject(JSGlobalObject* g) { m_defaultMicrotaskQueue->clearForGlobalObject(g); } So it has to stay scoped to the terminated context's global. `nodeVmGlobalObject` is deliberately null when the module has no context, meaning there is nothing to clear. Passing the caller's `globalObject` instead discarded the *main* thread's pending microtasks, so every parked `await` continuation vanished and the process wedged. `new SourceTextModule("while (true) {}")` with `evaluate({ timeout })` and no context is exactly that shape, which is why `test/js/node/test/parallel/test-vm-module-basic.js` timed out on every platform. drain target = nodeVmGlobalObject -> exit 0 (x3) drain target = globalObject -> exit 124 (x3) Reverted, with a comment naming the trap, plus a regression test that parks a continuation in the main queue across a terminated context-less evaluation. It fails against the `globalObject` target. --- src/jsc/bindings/NodeVMModule.cpp | 7 +++++-- test/js/node/vm/vm.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 23d5361dad56..c45f34c21055 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -253,8 +253,11 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return {}; } - // `globalObject` is the NodeVM global when there is one, non-null otherwise. - vm.drainMicrotasksForGlobalObject(globalObject); + // Despite the name this *clears* the queue, so it must stay scoped to the + // terminated context's global. `nodeVmGlobalObject` is null when there is + // no context, which means there is nothing to clear -- passing the caller's + // `globalObject` instead would discard the main thread's microtasks. + vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); vm.clearHasTerminationRequest(); if (getSigintReceived()) { diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 84b460276f64..47a81d851471 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -965,6 +965,33 @@ test("Loader is not defined in vm context", () => { expect(runInContext("typeof Loader.registry;", customContext)).toBe("undefined"); }); +// `drainMicrotasksForGlobalObject` *clears* rather than drains, so a terminated +// module must only clear its own context's global. Passing the caller's global +// discarded the main thread's pending microtasks and wedged the process. +test("a terminated context-less module does not discard the main microtask queue", async () => { + const fixture = ` + const vm = require("node:vm"); + const m = new vm.SourceTextModule("while (true) {}"); + await m.link(() => {}); + + const { promise, resolve } = Promise.withResolvers(); + const chain = promise.then(() => "survived"); + resolve(); // the continuation is now parked in the main microtask queue + + const guard = setTimeout(() => { console.log("HUNG"); process.exit(3); }, 8000); + await m.evaluate({ timeout: 100 }).then(() => console.log("NO_THROW"), e => console.log("threw=" + e.code)); + console.log("chain=" + (await chain)); + clearTimeout(guard); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + capture(stderr); + expect({ stdout: stdout.trim().split("\n"), exitCode }).toEqual({ + stdout: ["threw=ERR_SCRIPT_EXECUTION_TIMEOUT", "chain=survived"], + exitCode: 0, + }); +}); + // Re-raising an enclosing scope's termination must not hand the VM's singleton // TerminationException to the module: storing it and re-throwing it once the // request is cleared trips `VM::setException`'s From 81f5ef99adcbe04fdce340e17db0a3ac682440ad Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:25:55 +0000 Subject: [PATCH 09/11] node:vm: keep the runInThisContext microtask clear out of the caller's queue `drainMicrotasksForGlobalObject` clears rather than drains, so a terminated script may only clear the queue of the context it ran in. `runInThisContext` has no context of its own, and `checkForTermination` was handing it the caller's global: const { promise, resolve } = Promise.withResolvers(); const chain = promise.then(() => "survived"); resolve(); try { vm.runInThisContext("while (true) {}", { timeout: 100 }); } catch {} chain.then(v => console.log(v)); // never prints; node prints "survived" Pass the contextified global separately: `runInContext` keeps its own `NodeVMGlobalObject`, `runInThisContext` passes null (nothing to clear), matching what `NodeVMModule::evaluate` already does. --- src/jsc/bindings/NodeVMModule.cpp | 12 +++++------- src/jsc/bindings/NodeVMScript.cpp | 13 +++++++++---- test/js/node/vm/vm.test.ts | 30 ++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index c45f34c21055..301b832c8424 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -246,17 +246,15 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b std::ignore = scope.exception(); if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { // An enclosing scope asked for the termination; only it can classify it. - // Returning rather than falling through is load-bearing: VM_RETURN_IF_EXCEPTION - // would store the singleton TerminationException, whose later re-throw trips - // `VM::setException`. `reconcileEvaluationState` settles the status safely. + // Returning is load-bearing: falling through would store the singleton + // TerminationException, whose re-throw later trips `VM::setException`. if (!getSigintReceived() && timeout == 0) { JSC::throwException(globalObject, scope, vm.ensureTerminationException()); return {}; } - // Despite the name this *clears* the queue, so it must stay scoped to the - // terminated context's global. `nodeVmGlobalObject` is null when there is - // no context, which means there is nothing to clear -- passing the caller's - // `globalObject` instead would discard the main thread's microtasks. + // Despite the name this *clears* the queue, so scope it to the terminated + // context. `nodeVmGlobalObject` is null when there is no context (nothing + // to clear); passing `globalObject` would discard the main queue. vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); vm.clearHasTerminationRequest(); diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 48a70a1f7d3b..d2ba3421a279 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -281,7 +281,7 @@ void NodeVMScript::destroy(JSCell* cell) static_cast(cell)->NodeVMScript::~NodeVMScript(); } -static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional timeout) +static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, NodeVMGlobalObject* contextGlobalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional timeout) { if (!vm.hasTerminationRequest()) return false; @@ -294,7 +294,10 @@ static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, return true; } - vm.drainMicrotasksForGlobalObject(globalObject); + // Despite the name this *clears* the queue, so scope it to the terminated + // context. `runInThisContext` has none of its own (null, nothing to clear); + // passing `globalObject` there would discard the caller's microtasks. + vm.drainMicrotasksForGlobalObject(contextGlobalObject); // The termination may have fired inside an afterEvaluate microtask // checkpoint, leaving the termination exception pending; clear it so // the ERR_SCRIPT_EXECUTION_* error below replaces it. @@ -388,7 +391,7 @@ static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVM vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit)); } - if (checkForTermination(vm, globalObject, scope, script, newLimit)) { + if (checkForTermination(vm, globalObject, globalObject, scope, script, newLimit)) { return {}; } @@ -453,7 +456,9 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInThisContext, (JSGlobalObject * globalObject, vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit)); } - if (checkForTermination(vm, globalObject, scope, script, newLimit)) { + // `runInThisContext` evaluates in the caller's global, so there is no + // contextified global whose microtask queue the termination may clear. + if (checkForTermination(vm, globalObject, nullptr, scope, script, newLimit)) { return {}; } diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 47a81d851471..0174956f48da 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -992,6 +992,36 @@ test("a terminated context-less module does not discard the main microtask queue }); }); +// Same clear, reached from the script side. `runInThisContext` evaluates in the +// caller's global, so there is no context queue to clear and the caller's one is +// not ours to discard. Node keeps the continuation here too. +test("a timed-out runInThisContext does not discard the main microtask queue", async () => { + const fixture = ` + const vm = require("node:vm"); + + const { promise, resolve } = Promise.withResolvers(); + const chain = promise.then(() => "survived"); + resolve(); // the continuation is now parked in the main microtask queue + + try { + vm.runInThisContext("while (true) {}", { timeout: 100 }); + console.log("NO_THROW"); + } catch (e) { + console.log("threw=" + e.code); + } + // Clearing the queue drops the continuation, so this never prints and the + // process exits with nothing left to do. + chain.then(v => console.log("chain=" + v)); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + capture(stderr); + expect({ stdout: stdout.trim().split("\n"), exitCode }).toEqual({ + stdout: ["threw=ERR_SCRIPT_EXECUTION_TIMEOUT", "chain=survived"], + exitCode: 0, + }); +}); + // Re-raising an enclosing scope's termination must not hand the VM's singleton // TerminationException to the module: storing it and re-throwing it once the // request is cleared trips `VM::setException`'s From bd2f7b6bee93161aa2d322b7a498f34a1355f97d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:27:21 +0000 Subject: [PATCH 10/11] SigintWatcher: own the SIGINT disposition while armed BunProcess and the watcher each install their own SIGINT handler and each refuse to uninstall one they did not install, which leaves the disposition wrong in both directions once the REPL arms the watcher around every evaluation. Removing the last listener: BunProcess's `signal(SIGINT, SIG_DFL)` finds the watcher's handler, so it reinstates it; the disarm then sees its own handler intact and restores the snapshot it took at arm time, which is the forwarder BunProcess just asked to drop. An external `kill -INT` is swallowed from then on. Adding one: BunProcess installs the forwarder over the watcher's handler, so Ctrl+C during that same evaluation only queues a JS event, which the loop it is supposed to interrupt never drains. While armed the watcher owns the disposition, so BunProcess hands it the action to apply on disarm instead of installing over it. When nothing is armed `deferSigintDisposition` returns false immediately and BunProcess is unchanged. --- src/jsc/bindings/BunProcess.cpp | 24 +++++++++++++++--- src/jsc/bindings/vm/SigintWatcher.cpp | 26 +++++++++++++++++--- src/jsc/bindings/vm/SigintWatcher.h | 9 ++++++- test/js/bun/repl/repl.test.ts | 35 +++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index 498fdcf9278f..0d2b51b2f3b6 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -59,6 +59,7 @@ #include "NodeValidator.h" #include "NodeModuleModule.h" #include "JSX509Certificate.h" +#include "vm/SigintWatcher.h" #include "AsyncContextFrame.h" #include "ErrorCode.h" @@ -1568,7 +1569,12 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e sigaddset(&action.sa_mask, signalNumber); action.sa_flags = SA_RESTART; - sigaction(signalNumber, &action, nullptr); + // The SIGINT watcher (`node:vm` breakOnSigint, the REPL) holds the + // disposition while the code doing this runs, so hand it the action + // rather than installing over its handler and disarming it. + if (signalNumber != SIGINT || !Bun::SigintWatcher::get().deferSigintDisposition(action)) { + sigaction(signalNumber, &action, nullptr); + } #else signal_handle.handle = Bun__UVSignalHandle__init( eventEmitter.scriptExecutionContext()->jsGlobalObject(), @@ -1585,9 +1591,19 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e if (signalToContextIdsMap->find(signalNumber) != signalToContextIdsMap->end() && eventEmitter.listenerCount(eventName) == 0) { #if !OS(WINDOWS) - 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); + struct sigaction action; + memset(&action, 0, sizeof(struct sigaction)); + action.sa_handler = SIG_DFL; + sigemptyset(&action.sa_mask); + + // Same as above: while the watcher is armed, the default we want + // back takes effect when it disarms, not now. Without this its + // handler is what `signal()` below would find and reinstate. + if (signalNumber != SIGINT || !Bun::SigintWatcher::get().deferSigintDisposition(action)) { + 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); + } } #else SignalHandleValue signal_handle = signalToContextIdsMap->get(signalNumber); diff --git a/src/jsc/bindings/vm/SigintWatcher.cpp b/src/jsc/bindings/vm/SigintWatcher.cpp index 1c16edd6a5ea..81918793c430 100644 --- a/src/jsc/bindings/vm/SigintWatcher.cpp +++ b/src/jsc/bindings/vm/SigintWatcher.cpp @@ -95,9 +95,9 @@ void SigintWatcher::uninstall() #if OS(WINDOWS) SetConsoleCtrlHandler(WindowsCtrlHandler, false); #else - // Undo only our own handler. Code that ran while we were armed may have - // installed its own (`process.on("SIGINT")`), and clobbering that would - // strand the listener for good: BunProcess installs it exactly once. + // Undo only our own handler: a native addon may have installed its own + // while we were armed, and clobbering it would strand that handler. + // `process.on("SIGINT")` instead routes through deferSigintDisposition. struct sigaction current; if (sigaction(SIGINT, nullptr, ¤t) == 0 && !(current.sa_flags & SA_SIGINFO) @@ -111,6 +111,26 @@ void SigintWatcher::uninstall() } } +#if !OS(WINDOWS) +bool SigintWatcher::deferSigintDisposition(const struct sigaction& action) +{ + WTF::Locker locker { m_refCountMutex }; + if (!m_installed.load()) { + return false; + } + + struct sigaction current; + if (sigaction(SIGINT, nullptr, ¤t) != 0 + || (current.sa_flags & SA_SIGINFO) + || current.sa_handler != sigintWatcherHandler) { + return false; + } + + m_previousAction = action; + return true; +} +#endif + void SigintWatcher::signalReceived() { if (!m_waiting.test_and_set()) { diff --git a/src/jsc/bindings/vm/SigintWatcher.h b/src/jsc/bindings/vm/SigintWatcher.h index 9fabe94fa476..9a475e9ca984 100644 --- a/src/jsc/bindings/vm/SigintWatcher.h +++ b/src/jsc/bindings/vm/SigintWatcher.h @@ -20,6 +20,12 @@ class SigintWatcher { void install(); void uninstall(); void signalReceived(); +#if !OS(WINDOWS) + /** While armed we own the SIGINT disposition, so anything that wants to change + * it (`process.on("SIGINT")` and its removal) hands us the action to apply when + * we disarm. Returns false if we are not armed: install it yourself. */ + bool deferSigintDisposition(const struct sigaction& action); +#endif void registerGlobalObject(JSC::JSGlobalObject* globalObject); void unregisterGlobalObject(JSC::JSGlobalObject* globalObject); void registerReceiver(SigintReceiver* module); @@ -107,7 +113,8 @@ class SigintWatcher { WTF::Vector m_receivers; uint32_t m_refCount = 0; #if !OS(WINDOWS) - // The disposition install() displaced, so uninstall() can put it back. + // What uninstall() puts back: the disposition install() displaced, unless + // deferSigintDisposition() has since replaced it. Guarded by m_refCountMutex. struct sigaction m_previousAction {}; #endif diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 695d2d8850f8..917f66434df3 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -1030,6 +1030,41 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { }); }); + // And the mirror: dropping the last listener has to put the default action back. + // BunProcess reinstates the watcher's own handler rather than uninstalling it, + // so the disarm used to see its handler intact and restore the stale snapshot. + test("removing the last SIGINT listener restores the default disposition", async () => { + await withTerminalRepl(async ({ send, waitFor, proc }) => { + send("globalThis.h = () => {}; process.on('SIGINT', globalThis.h); 1 + 1\n"); + await waitFor(/\n\s*2\b/); + + send("process.removeListener('SIGINT', globalThis.h); 3 + 4\n"); + await waitFor(/\n\s*7\b/); + + proc.kill("SIGINT"); + const outcome = await Promise.race([ + proc.exited.then(() => "exited"), + Bun.sleep(4000).then(() => "still running"), + ]); + expect(outcome).toBe("exited"); + }); + }); + + // Adding a listener used to install over the watcher's handler, downgrading + // Ctrl+C to a JS event that the loop it is meant to break out of never drains. + test("Ctrl+C interrupts a loop that installed a SIGINT listener first", async () => { + await withTerminalRepl(async ({ send, waitFor }) => { + send(`process.on("SIGINT", () => {}); process.stdout.write("LOOP" + "ING\\n"); while (true) {}\n`); + await waitFor("LOOPING"); + send("\x03"); // Ctrl+C + const output = await waitFor(/interrupted/); + expect(stripAnsi(output)).toContain("Script execution was interrupted by `SIGINT`"); + + send("111 + 222\n"); + await waitFor(/\n\s*333\b/); + }); + }); + // The REPL used to stay in raw mode while evaluating, so Ctrl+C was delivered // as a byte nobody read and a synchronous loop could only be escaped by // killing the process (which left the terminal in raw mode). From 92c7b6fcbea6d04c558e4943ee19cf215c6210a7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:26:05 +0000 Subject: [PATCH 11/11] SigintWatcher: restore the disposition before going unarmed `uninstall()` cleared `m_installed` while its handler was still the SIGINT disposition. A signal landing in that window runs the handler, which posts the semaphore, and the watcher thread then bails on `if (!m_installed) return;` before forwarding it, so the signal reaches nobody at all. It also strands `m_waiting` set, which deafens the watcher for the rest of the process. Hand the disposition back first, then go unarmed. The restore is already guarded on the handler still being ours, so the second caller of a racing `uninstall()` (only the destructor; `deref` holds the mutex) finds nothing to undo and the exchange still gates the thread teardown to one caller. Also: prove the REPL's SIGINT listener actually runs rather than only that the process stayed alive, and drop the watchdog timer from the module microtask fixture now that the continuation is a `.then` rather than an `await`, so the cleared-queue case fails on the missing line instead of on a timer. --- src/jsc/bindings/vm/SigintWatcher.cpp | 41 +++++++++++++++++---------- test/js/bun/repl/repl.test.ts | 13 +++++++-- test/js/node/vm/vm.test.ts | 8 +++--- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/jsc/bindings/vm/SigintWatcher.cpp b/src/jsc/bindings/vm/SigintWatcher.cpp index 81918793c430..53e0e516dcfc 100644 --- a/src/jsc/bindings/vm/SigintWatcher.cpp +++ b/src/jsc/bindings/vm/SigintWatcher.cpp @@ -88,27 +88,38 @@ void SigintWatcher::install() void SigintWatcher::uninstall() { - if (m_installed.exchange(false)) { - WTF::Thread* currentThread = WTF::Thread::currentMayBeNull(); - ASSERT(!currentThread || m_thread->uid() != currentThread->uid()); + if (!m_installed.load()) { + return; + } + WTF::Thread* currentThread = WTF::Thread::currentMayBeNull(); + ASSERT(!currentThread || m_thread->uid() != currentThread->uid()); + + // Hand the disposition back while still armed. Clearing m_installed first + // leaves a window where our handler is live but the watcher thread bails out + // of `signalAll`, so a signal landing in it reaches nobody at all. #if OS(WINDOWS) - SetConsoleCtrlHandler(WindowsCtrlHandler, false); + SetConsoleCtrlHandler(WindowsCtrlHandler, false); #else - // Undo only our own handler: a native addon may have installed its own - // while we were armed, and clobbering it would strand that handler. - // `process.on("SIGINT")` instead routes through deferSigintDisposition. - struct sigaction current; - if (sigaction(SIGINT, nullptr, ¤t) == 0 - && !(current.sa_flags & SA_SIGINFO) - && current.sa_handler == sigintWatcherHandler) { - sigaction(SIGINT, &m_previousAction, nullptr); - } + // Undo only our own handler: a native addon may have installed its own + // while we were armed, and clobbering it would strand that handler. + // `process.on("SIGINT")` instead routes through deferSigintDisposition. + struct sigaction current; + if (sigaction(SIGINT, nullptr, ¤t) == 0 + && !(current.sa_flags & SA_SIGINFO) + && current.sa_handler == sigintWatcherHandler) { + sigaction(SIGINT, &m_previousAction, nullptr); + } #endif - m_semaphore.signal(); - m_thread->waitForCompletion(); + // The restore above is idempotent, so a second caller that lost this race + // (only the destructor can, `deref` holds m_refCountMutex) stops here. + if (!m_installed.exchange(false)) { + return; } + + m_semaphore.signal(); + m_thread->waitForCompletion(); } #if !OS(WINDOWS) diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 917f66434df3..0fc0143a589d 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -1021,12 +1021,19 @@ describe.todoIf(isWindows)("Bun REPL (Terminal)", () => { // strand the listener for the rest of the session. test("a SIGINT listener registered in the REPL survives the evaluation", async () => { await withTerminalRepl(async ({ send, waitFor, proc }) => { - send("process.on('SIGINT', () => {}); 1 + 1\n"); + // Split the literal so the echoed input line can't satisfy the wait. + send("process.on('SIGINT', () => console.log('CAUGHT_' + 'SIGINT')); 1 + 1\n"); await waitFor(/\n\s*2\b/); proc.kill("SIGINT"); - const outcome = await Promise.race([proc.exited.then(() => "exited"), Bun.sleep(2000).then(() => "alive")]); - expect(outcome).toBe("alive"); + // The loop only runs during an evaluation, so the queued signal is handed + // to the listener on the next one. The signal is already pending when the + // keystrokes are written, and a handler runs before the read they satisfy + // returns to userspace, so the marker can't be missed. + send("3 + 4\n"); + // Getting the marker at all proves the process survived and the listener, + // rather than the watcher's handler, is what the disposition points at. + await waitFor("CAUGHT_SIGINT"); }); }); diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 0174956f48da..2d6560861f2f 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -978,10 +978,10 @@ test("a terminated context-less module does not discard the main microtask queue const chain = promise.then(() => "survived"); resolve(); // the continuation is now parked in the main microtask queue - const guard = setTimeout(() => { console.log("HUNG"); process.exit(3); }, 8000); - await m.evaluate({ timeout: 100 }).then(() => console.log("NO_THROW"), e => console.log("threw=" + e.code)); - console.log("chain=" + (await chain)); - clearTimeout(guard); + m.evaluate({ timeout: 100 }).then(() => console.log("NO_THROW"), e => console.log("threw=" + e.code)); + // Clearing the queue drops the continuation, so this never prints and the + // process exits with nothing left to do. + chain.then(v => console.log("chain=" + v)); `; await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);