From 88f16236b14e66ac951707c0cbf5b2bc41c6610f Mon Sep 17 00:00:00 2001 From: snek Date: Wed, 9 Jul 2025 09:01:27 +0200 Subject: [PATCH] fix: new signal handling (#30029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set up a new centralized signal handling path, which emulates the default unless something is explicitly registered to prevent it. Fixes: https://github.com/denoland/deno/issues/29590 --------- Co-authored-by: Bartek Iwańczuk --- Cargo.lock | 14 ++- Cargo.toml | 3 + ext/http/http_next.rs | 18 ++++ ext/net/Cargo.toml | 1 + ext/net/tunnel.rs | 63 +++--------- ext/os/Cargo.toml | 6 +- ext/os/lib.rs | 6 +- ext/os/ops/signal.rs | 204 ++++----------------------------------- ext/signals/Cargo.toml | 18 ++++ ext/signals/README.md | 1 + ext/signals/lib.rs | 164 +++++++++++++++++++++++++++++++ ext/telemetry/Cargo.toml | 1 + ext/telemetry/lib.rs | 2 + 13 files changed, 253 insertions(+), 248 deletions(-) create mode 100644 ext/signals/Cargo.toml create mode 100644 ext/signals/README.md create mode 100644 ext/signals/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 222c443ce4..9921a87c34 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2366,6 +2366,7 @@ dependencies = [ "deno_error", "deno_features", "deno_permissions", + "deno_signals", "deno_tls", "hickory-proto", "hickory-resolver", @@ -2609,14 +2610,12 @@ dependencies = [ "deno_error", "deno_path_util", "deno_permissions", - "deno_telemetry", + "deno_signals", "libc", "netif", "ntapi", "once_cell", "serde", - "signal-hook", - "signal-hook-registry", "thiserror 2.0.12", "tokio", "winapi", @@ -2842,6 +2841,14 @@ dependencies = [ "url", ] +[[package]] +name = "deno_signals" +version = "0.1.0" +dependencies = [ + "signal-hook", + "winapi", +] + [[package]] name = "deno_snapshots" version = "0.25.0" @@ -2888,6 +2895,7 @@ dependencies = [ "deno_core", "deno_error", "deno_net", + "deno_signals", "deno_tls", "http-body-util", "hyper 1.6.0", diff --git a/Cargo.toml b/Cargo.toml index 904f9091e6..de3799804a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ members = [ "ext/net", "ext/node", "ext/rt_helper", + "ext/signals", "ext/telemetry", "ext/url", "ext/web", @@ -101,6 +102,7 @@ deno_net = { version = "0.202.0", path = "./ext/net" } deno_node = { version = "0.148.0", path = "./ext/node" } deno_os = { version = "0.27.0", path = "./ext/os" } deno_process = { version = "0.25.0", path = "./ext/process" } +deno_signals = { version = "0.1.0", path = "./ext/signals" } deno_telemetry = { version = "0.32.0", path = "./ext/telemetry" } deno_tls = { version = "0.197.0", path = "./ext/tls" } deno_url = { version = "0.210.0", path = "./ext/url" } @@ -242,6 +244,7 @@ serde-value = "0.7" serde_bytes = "0.11" serde_json = "1.0.85" serde_repr = "=0.1.19" +signal-hook = "0.3" simd-json = "0.14.0" slab = "0.4" smallvec = "1.8" diff --git a/ext/http/http_next.rs b/ext/http/http_next.rs index 3729cc9c8f..71b597d612 100644 --- a/ext/http/http_next.rs +++ b/ext/http/http_next.rs @@ -30,6 +30,7 @@ use deno_core::serde_v8::from_v8; use deno_core::unsync::JoinHandle; use deno_core::unsync::spawn; use deno_core::v8; +use deno_error::JsErrorClass; use deno_net::ops_tls::TlsStream; use deno_net::raw::NetworkStream; use deno_websocket::ws_create_server_stream; @@ -1299,6 +1300,23 @@ pub async fn op_http_wait( } } + if let HttpNextError::Other(err) = &err { + if let Some(err) = err.as_any().downcast_ref::() { + if let Some(err) = err.get_ref() { + if let Some(err) = + err.downcast_ref::() + { + if matches!( + err, + deno_net::tunnel::quinn::ConnectionError::LocallyClosed + ) { + return Ok(null()); + } + } + } + } + } + return Err(err); } diff --git a/ext/net/Cargo.toml b/ext/net/Cargo.toml index aebb99a45d..1a03a0ebf5 100644 --- a/ext/net/Cargo.toml +++ b/ext/net/Cargo.toml @@ -18,6 +18,7 @@ deno_core.workspace = true deno_error.workspace = true deno_features.workspace = true deno_permissions.workspace = true +deno_signals.workspace = true deno_tls.workspace = true hickory-proto.workspace = true hickory-resolver.workspace = true diff --git a/ext/net/tunnel.rs b/ext/net/tunnel.rs index f0e873834c..e579babb32 100644 --- a/ext/net/tunnel.rs +++ b/ext/net/tunnel.rs @@ -21,6 +21,7 @@ use deno_tls::SocketUse; use deno_tls::TlsKeys; use deno_tls::create_client_config; use deno_tls::rustls::RootCertStore; +pub use quinn; use quinn::ConnectionError; use quinn::crypto::rustls::QuicClientConfig; use tokio::io::AsyncRead; @@ -61,7 +62,16 @@ static TUNNEL: OnceLock = OnceLock::new(); pub fn set_tunnel(tunnel: crate::tunnel::TunnelListener) { if TUNNEL.set(tunnel).is_ok() { - setup_signal_handlers(); + deno_signals::before_exit(before_exit); + } +} + +fn before_exit() { + if let Some(tunnel) = get_tunnel() { + tunnel.connection.close(1u32.into(), b""); + // stay alive long enough to actually send the close frame, since + // we can't rely on the linux kernel to close this like with tcp. + deno_core::futures::executor::block_on(tunnel.endpoint.wait_idle()); } } @@ -69,57 +79,6 @@ pub fn get_tunnel() -> Option<&'static crate::tunnel::TunnelListener> { TUNNEL.get() } -#[cfg(unix)] -fn setup_signal_handlers() { - use tokio::signal::unix::SignalKind; - - let signals_to_handle = [ - SignalKind::hangup(), - SignalKind::interrupt(), - SignalKind::terminate(), - ]; - - for signal_kind in signals_to_handle { - tokio::spawn(async move { - let Ok(mut signal_fut) = tokio::signal::unix::signal(signal_kind) else { - return; - }; - - loop { - signal_fut.recv().await; - if let Some(tunnel) = get_tunnel() { - tunnel.connection.close(1u32.into(), b""); - } - } - }); - } -} - -#[cfg(windows)] -fn setup_signal_handlers() { - macro_rules! handle_signal { - ($handler:expr) => { - tokio::spawn(async { - let Ok(mut signal_fut) = $handler() else { - return; - }; - loop { - signal_fut.recv().await; - if let Some(tunnel) = get_tunnel() { - tunnel.connection.close(1u32.into(), b""); - } - } - }); - }; - } - - handle_signal!(tokio::signal::windows::ctrl_break); - handle_signal!(tokio::signal::windows::ctrl_c); - handle_signal!(tokio::signal::windows::ctrl_close); - handle_signal!(tokio::signal::windows::ctrl_logoff); - handle_signal!(tokio::signal::windows::ctrl_shutdown); -} - /// Essentially a SocketAddr, except we prefer a human /// readable hostname to identify the remote endpoint. #[derive(Debug, Clone)] diff --git a/ext/os/Cargo.toml b/ext/os/Cargo.toml index 63d018b158..5fc6ca8dd2 100644 --- a/ext/os/Cargo.toml +++ b/ext/os/Cargo.toml @@ -18,7 +18,7 @@ deno_core.workspace = true deno_error.workspace = true deno_path_util.workspace = true deno_permissions.workspace = true -deno_telemetry.workspace = true +deno_signals.workspace = true libc.workspace = true netif.workspace = true once_cell.workspace = true @@ -29,7 +29,3 @@ tokio.workspace = true [target.'cfg(windows)'.dependencies] winapi = { workspace = true, features = ["commapi", "knownfolders", "mswsock", "objbase", "psapi", "shlobj", "sysinfoapi", "tlhelp32", "winbase", "winerror", "winuser", "winsock2"] } ntapi = "0.4.0" - -[target.'cfg(unix)'.dependencies] -signal-hook = "0.3.17" -signal-hook-registry = "1.4.2" diff --git a/ext/os/lib.rs b/ext/os/lib.rs index 7c2d1de94a..4ec4d6fddf 100644 --- a/ext/os/lib.rs +++ b/ext/os/lib.rs @@ -42,7 +42,7 @@ impl ExitCode { } pub fn exit(code: i32) -> ! { - deno_telemetry::flush(); + deno_signals::run_exit(); #[allow(clippy::disallowed_methods)] std::process::exit(code); } @@ -81,10 +81,6 @@ deno_core::extension!( if let Some(exit_code) = options.exit_code { state.put::(exit_code); } - #[cfg(unix)] - { - state.put(ops::signal::SignalState::default()); - } } ); diff --git a/ext/os/ops/signal.rs b/ext/os/ops/signal.rs index 488bee326c..f2e2946003 100644 --- a/ext/os/ops/signal.rs +++ b/ext/os/ops/signal.rs @@ -1,35 +1,15 @@ // Copyright 2018-2025 the Deno authors. MIT license. use std::borrow::Cow; use std::cell::RefCell; -#[cfg(unix)] -use std::collections::BTreeMap; use std::rc::Rc; -#[cfg(unix)] -use std::sync::Arc; -#[cfg(unix)] -use std::sync::atomic::AtomicBool; use deno_core::AsyncRefCell; -use deno_core::CancelFuture; -use deno_core::CancelHandle; use deno_core::OpState; use deno_core::RcRef; use deno_core::Resource; use deno_core::ResourceId; use deno_core::error::ResourceError; use deno_core::op2; -#[cfg(unix)] -use tokio::signal::unix::Signal; -#[cfg(unix)] -use tokio::signal::unix::SignalKind; -#[cfg(unix)] -use tokio::signal::unix::signal; -#[cfg(windows)] -use tokio::signal::windows::CtrlBreak; -#[cfg(windows)] -use tokio::signal::windows::CtrlC; -#[cfg(windows)] -use tokio::signal::windows::CtrlClose; #[derive(Debug, thiserror::Error, deno_error::JsError)] pub enum SignalError { @@ -47,117 +27,22 @@ pub enum SignalError { Io(#[from] std::io::Error), } -#[cfg(unix)] -#[derive(Default)] -pub struct SignalState { - enable_default_handlers: BTreeMap>, -} - -#[cfg(unix)] -impl SignalState { - /// Disable the default signal handler for the given signal. - /// - /// Returns the shared flag to enable the default handler later, and whether a default handler already existed. - fn disable_default_handler( - &mut self, - signo: libc::c_int, - ) -> (Arc, bool) { - use std::collections::btree_map::Entry; - - match self.enable_default_handlers.entry(signo) { - Entry::Occupied(entry) => { - let enable = entry.get(); - enable.store(false, std::sync::atomic::Ordering::Release); - (enable.clone(), true) - } - Entry::Vacant(entry) => { - let enable = Arc::new(AtomicBool::new(false)); - entry.insert(enable.clone()); - (enable, false) - } - } - } -} - -#[cfg(unix)] -/// The resource for signal stream. -/// The second element is the waker of polling future. struct SignalStreamResource { - signal: AsyncRefCell, - enable_default_handler: Arc, - cancel: CancelHandle, + signo: i32, + id: u32, + rx: AsyncRefCell>, } -#[cfg(unix)] impl Resource for SignalStreamResource { fn name(&self) -> Cow { "signal".into() } fn close(self: Rc) { - self.cancel.cancel(); + deno_signals::unregister(self.signo, self.id); } } -// TODO: CtrlClose could be mapped to SIGHUP but that needs a -// tokio::windows::signal::CtrlClose type, or something from a different crate -#[cfg(windows)] -enum WindowsSignal { - Sigint(CtrlC), - Sigbreak(CtrlBreak), - Sighup(CtrlClose), -} - -#[cfg(windows)] -impl From for WindowsSignal { - fn from(ctrl_c: CtrlC) -> Self { - Self::Sigint(ctrl_c) - } -} - -#[cfg(windows)] -impl From for WindowsSignal { - fn from(ctrl_break: CtrlBreak) -> Self { - Self::Sigbreak(ctrl_break) - } -} - -#[cfg(windows)] -impl From for WindowsSignal { - fn from(ctrl_close: CtrlClose) -> Self { - Self::Sighup(ctrl_close) - } -} - -#[cfg(windows)] -impl WindowsSignal { - pub async fn recv(&mut self) -> Option<()> { - match self { - Self::Sigint(ctrl_c) => ctrl_c.recv().await, - Self::Sigbreak(ctrl_break) => ctrl_break.recv().await, - Self::Sighup(ctrl_close) => ctrl_close.recv().await, - } - } -} - -#[cfg(windows)] -struct SignalStreamResource { - signal: AsyncRefCell, - cancel: CancelHandle, -} - -#[cfg(windows)] -impl Resource for SignalStreamResource { - fn name(&self) -> Cow { - "signal".into() - } - - fn close(self: Rc) { - self.cancel.cancel(); - } -} - -#[cfg(unix)] #[op2(fast)] #[smi] pub fn op_signal_bind( @@ -165,60 +50,25 @@ pub fn op_signal_bind( #[string] sig: &str, ) -> Result { let signo = crate::signal::signal_str_to_int(sig)?; - if signal_hook_registry::FORBIDDEN.contains(&signo) { + if deno_signals::is_forbidden(signo) { return Err(SignalError::SignalNotAllowed(sig.to_string())); } - let signal = AsyncRefCell::new(signal(SignalKind::from_raw(signo))?); - - let (enable_default_handler, has_default_handler) = state - .borrow_mut::() - .disable_default_handler(signo); - - let resource = SignalStreamResource { - signal, - cancel: Default::default(), - enable_default_handler: enable_default_handler.clone(), - }; - let rid = state.resource_table.add(resource); - - if !has_default_handler { - // restore default signal handler when the signal is unbound - // this can error if the signal is not supported, if so let's just leave it as is - let _ = signal_hook::flag::register_conditional_default( - signo, - enable_default_handler, - ); - } - - Ok(rid) -} - -#[cfg(windows)] -#[op2(fast)] -#[smi] -pub fn op_signal_bind( - state: &mut OpState, - #[string] sig: &str, -) -> Result { - use tokio::signal::windows::ctrl_break; - use tokio::signal::windows::ctrl_c; - use tokio::signal::windows::ctrl_close; - - let resource = SignalStreamResource { - signal: AsyncRefCell::new(match sig { - "SIGHUP" => ctrl_close()?.into(), - "SIGINT" => ctrl_c()?.into(), - "SIGBREAK" => ctrl_break()?.into(), - _ => { - return Err( - crate::signal::InvalidSignalStrError(sig.to_string()).into(), - ); - } + let (tx, rx) = tokio::sync::watch::channel(()); + let id = deno_signals::register( + signo, + true, + Box::new(move || { + let _ = tx.send(()); }), - cancel: Default::default(), - }; - let rid = state.resource_table.add(resource); + ); + + let rid = state.resource_table.add(SignalStreamResource { + signo, + id, + rx: AsyncRefCell::new(rx), + }); + Ok(rid) } @@ -232,13 +82,9 @@ pub async fn op_signal_poll( .resource_table .get::(rid)?; - let cancel = RcRef::map(&resource, |r| &r.cancel); - let mut signal = RcRef::map(&resource, |r| &r.signal).borrow_mut().await; + let mut rx = RcRef::map(&resource, |r| &r.rx).borrow_mut().await; - match signal.recv().or_cancel(cancel).await { - Ok(result) => Ok(result.is_none()), - Err(_) => Ok(true), - } + Ok(rx.changed().await.is_err()) } #[op2(fast)] @@ -247,14 +93,6 @@ pub fn op_signal_unbind( #[smi] rid: ResourceId, ) -> Result<(), ResourceError> { let resource = state.resource_table.take::(rid)?; - - #[cfg(unix)] - { - resource - .enable_default_handler - .store(true, std::sync::atomic::Ordering::Release); - } - resource.close(); Ok(()) } diff --git a/ext/signals/Cargo.toml b/ext/signals/Cargo.toml new file mode 100644 index 0000000000..e7ee2c845c --- /dev/null +++ b/ext/signals/Cargo.toml @@ -0,0 +1,18 @@ +# Copyright 2018-2025 the Deno authors. MIT license. + +[package] +name = "deno_signals" +version = "0.1.0" +authors.workspace = true +edition.workspace = true +license.workspace = true +readme = "README.md" +repository.workspace = true +description = "Signals for Deno" + +[lib] +path = "lib.rs" + +[dependencies] +signal-hook.workspace = true +winapi.workspace = true diff --git a/ext/signals/README.md b/ext/signals/README.md new file mode 100644 index 0000000000..dbba93d90e --- /dev/null +++ b/ext/signals/README.md @@ -0,0 +1 @@ +Signal handling for Deno diff --git a/ext/signals/lib.rs b/ext/signals/lib.rs new file mode 100644 index 0000000000..7a82a0e1c0 --- /dev/null +++ b/ext/signals/lib.rs @@ -0,0 +1,164 @@ +// Copyright 2018-2025 the Deno authors. MIT license. + +use std::collections::HashMap; +use std::sync::Mutex; +use std::sync::OnceLock; +use std::sync::atomic::AtomicU32; +use std::sync::atomic::Ordering; + +use signal_hook::consts::*; + +#[cfg(windows)] +static SIGHUP: i32 = 1; + +static COUNTER: AtomicU32 = AtomicU32::new(0); + +type Handler = Box; +type Handlers = HashMap>; +static HANDLERS: OnceLock<(Handle, Mutex)> = OnceLock::new(); + +#[cfg(unix)] +struct Handle(signal_hook::iterator::Handle); + +#[cfg(windows)] +struct Handle; + +fn handle_signal(signal: i32) -> bool { + let Some((_, handlers)) = HANDLERS.get() else { + return false; + }; + let handlers = handlers.lock().unwrap(); + let Some(handlers) = handlers.get(&signal) else { + return false; + }; + + let mut handled = false; + + for (_, prevent_default, f) in handlers { + if *prevent_default { + handled = true; + } + + f(); + } + + handled +} + +#[cfg(unix)] +fn init() -> Handle { + use signal_hook::iterator::Signals; + + let mut signals = Signals::new::<[i32; 0], i32>([]).unwrap(); + let handle = signals.handle(); + + std::thread::spawn(move || { + for signal in signals.forever() { + let handled = handle_signal(signal); + if !handled { + signal_hook::low_level::emulate_default_handler(signal).unwrap(); + } + } + }); + + Handle(handle) +} + +#[cfg(windows)] +fn init() -> Handle { + unsafe extern "system" fn handle(ctrl_type: u32) -> i32 { + let signal = match ctrl_type { + 0 => SIGINT, + 1 => SIGBREAK, + 2 => SIGHUP, + 5 => SIGTERM, + 6 => SIGTERM, + _ => return 0, + }; + let handled = handle_signal(signal); + handled as _ + } + + // SAFETY: Registering handler + unsafe { + winapi::um::consoleapi::SetConsoleCtrlHandler(Some(handle), 1); + } + + Handle +} + +pub fn register( + signal: i32, + prevent_default: bool, + f: Box, +) -> u32 { + let (handle, handlers) = HANDLERS.get_or_init(|| { + let handle = init(); + + let handlers = Mutex::new(HashMap::new()); + + (handle, handlers) + }); + + let id = COUNTER.fetch_add(1, Ordering::Relaxed); + let mut handlers = handlers.lock().unwrap(); + match handlers.entry(signal) { + std::collections::hash_map::Entry::Occupied(mut v) => { + v.get_mut().push((id, prevent_default, f)) + } + std::collections::hash_map::Entry::Vacant(v) => { + v.insert(vec![(id, prevent_default, f)]); + + #[cfg(unix)] + { + handle.0.add_signal(signal).unwrap(); + } + #[cfg(windows)] + { + let _ = handle; + } + } + } + + id +} + +pub fn unregister(signal: i32, id: u32) { + let Some((_, handlers)) = HANDLERS.get() else { + return; + }; + let mut handlers = handlers.lock().unwrap(); + let Some(handlers) = handlers.get_mut(&signal) else { + return; + }; + let Some(index) = handlers.iter().position(|v| v.0 == id) else { + return; + }; + let _ = handlers.swap_remove(index); +} + +static BEFORE_EXIT: OnceLock>> = OnceLock::new(); + +pub fn before_exit(f: fn()) { + register(SIGHUP, false, Box::new(f)); + register(SIGTERM, false, Box::new(f)); + register(SIGINT, false, Box::new(f)); + BEFORE_EXIT + .get_or_init(|| Mutex::new(vec![])) + .lock() + .unwrap() + .push(Box::new(f)); +} + +pub fn run_exit() { + if let Some(fns) = BEFORE_EXIT.get() { + let fns = fns.lock().unwrap(); + for f in fns.iter() { + f(); + } + } +} + +pub fn is_forbidden(signo: i32) -> bool { + FORBIDDEN.contains(&signo) +} diff --git a/ext/telemetry/Cargo.toml b/ext/telemetry/Cargo.toml index ff5724c6be..813aba5cf7 100644 --- a/ext/telemetry/Cargo.toml +++ b/ext/telemetry/Cargo.toml @@ -18,6 +18,7 @@ async-trait.workspace = true deno_core.workspace = true deno_error.workspace = true deno_net.workspace = true +deno_signals.workspace = true deno_tls.workspace = true http-body-util.workspace = true hyper.workspace = true diff --git a/ext/telemetry/lib.rs b/ext/telemetry/lib.rs index b60a7824ec..eb921d1ff8 100644 --- a/ext/telemetry/lib.rs +++ b/ext/telemetry/lib.rs @@ -927,6 +927,8 @@ pub fn init( }) .map_err(|_| deno_core::anyhow::anyhow!("failed to set otel globals"))?; + deno_signals::before_exit(flush); + Ok(()) }