fix: use 'deno_signals' crate for signal handling (#30204)

Follow up to https://github.com/denoland/deno/pull/30029.

Definition of signal numbers/names were moved from `ext/os` to
`ext/signals`.

All occurrences of `tokio::signal` API were replaced with helpers from
`deno_signals` helpers. Additionally clippy lints were added to ensure `tokio::signal`
is not used by accident.

Closes https://github.com/denoland/deno/issues/30223

Co-authored-by: snek <the@snek.dev>

Cargo.lock

@@ -1576,6 +1576,7 @@ dependencies = [
  "deno_resolver",
  "deno_runtime",
  "deno_semver",
+ "deno_signals",
  "deno_snapshots",
  "deno_subprocess_windows",
  "deno_task_shell",
@@ -2715,6 +2716,7 @@ dependencies = [
  "deno_os",
  "deno_path_util 0.5.2",
  "deno_permissions",
+ "deno_signals",
  "deno_subprocess_windows",
  "libc",
  "log",
@@ -2805,6 +2807,7 @@ dependencies = [
  "deno_permissions",
  "deno_process",
  "deno_resolver",
+ "deno_signals",
  "deno_telemetry",
  "deno_terminal 0.2.2",
  "deno_tls",
@@ -2862,7 +2865,11 @@ dependencies = [
 name = "deno_signals"
 version = "0.2.0"
 dependencies = [
+ "deno_error",
+ "libc",
  "signal-hook",
+ "thiserror 2.0.12",
+ "tokio",
  "winapi",
 ]
 

cli/Cargo.toml

@@ -89,6 +89,7 @@ deno_path_util.workspace = true
 deno_resolver = { workspace = true, features = ["deno_ast", "graph", "sync"] }
 deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_semver.workspace = true
+deno_signals.workspace = true
 deno_snapshots.workspace = true
 deno_task_shell.workspace = true
 deno_telemetry.workspace = true

cli/clippy.toml

@@ -2,10 +2,24 @@ disallowed-methods = [
   { path = "reqwest::Client::new", reason = "create an HttpClient via an HttpClientProvider instead" },
   { path = "std::process::exit", reason = "use deno_runtime::exit instead" },
   { path = "clap::Arg::env", reason = "ensure environment variables are resolved after loading the .env file instead" },
+  { path = "tokio::signal::ctrl_c", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::unix::signal", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::ctrl_break", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::ctrl_c", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::ctrl_close", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::ctrl_logoff", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::ctrl_shutdown", reason = "use deno_signals crate instead" },
 ]
 disallowed-types = [
   { path = "reqwest::Client", reason = "use crate::http_util::HttpClient instead" },
   { path = "sys_traits::impls::RealSys", reason = "use crate::sys::CliSys instead" },
+  { path = "tokio::signal::unix::Signal", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::unix::SignalKind", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::CtrlBreak", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::CtrlC", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::CtrlClose", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::CtrlLogoff", reason = "use deno_signals crate instead" },
+  { path = "tokio::signal::windows::CtrlShutdown", reason = "use deno_signals crate instead" },
 ]
 ignore-interior-mutability = [
   "lsp_types::Uri",

cli/task_runner.rs

@@ -36,6 +36,7 @@ pub fn get_script_with_args(script: &str, argv: &[String]) -> String {
     .map(|a| format!("\"{}\"", a.replace('"', "\\\"").replace('$', "\\$")))
     .collect::<Vec<_>>()
     .join(" ");
+
   let script = format!("{script} {additional_args}");
   script.trim().to_owned()
 }
@@ -632,7 +633,7 @@ pub async fn run_future_forwarding_signals<TOutput>(
 }
 
 async fn listen_ctrl_c(kill_signal: KillSignal) {
-  while let Ok(()) = tokio::signal::ctrl_c().await {
+  while let Ok(()) = deno_signals::ctrl_c().await {
     // On windows, ctrl+c is sent to the process group, so the signal would
     // have already been sent to the child process. We still want to listen
     // for ctrl+c here to keep the process alive when receiving it, but no
@@ -646,7 +647,7 @@ async fn listen_ctrl_c(kill_signal: KillSignal) {
 #[cfg(unix)]
 async fn listen_and_forward_all_signals(kill_signal: KillSignal) {
   use deno_core::futures::FutureExt;
-  use deno_runtime::deno_os::signal::SIGNAL_NUMS;
+  use deno_signals::SIGNAL_NUMS;
 
   // listen and forward every signal we support
   let mut futures = Vec::with_capacity(SIGNAL_NUMS.len());
@@ -658,9 +659,7 @@ async fn listen_and_forward_all_signals(kill_signal: KillSignal) {
     let kill_signal = kill_signal.clone();
     futures.push(
       async move {
-        let Ok(mut stream) = tokio::signal::unix::signal(
+        let Ok(mut stream) = deno_signals::signal_stream(signo) else {
-          tokio::signal::unix::SignalKind::from_raw(signo),
-        ) else {
           return;
         };
         let signal_kind: deno_task_shell::SignalKind = signo.into();

cli/tools/test/mod.rs

@@ -66,7 +66,6 @@ use rand::rngs::SmallRng;
 use rand::seq::SliceRandom;
 use regex::Regex;
 use serde::Deserialize;
-use tokio::signal;
 use tokio::sync::mpsc::UnboundedSender;
 
 use crate::args::CliOptions;
@@ -1282,7 +1281,7 @@ async fn test_specifiers(
 
   let mut cancel_sender = test_event_sender_factory.weak_sender();
   let sigint_handler_handle = spawn(async move {
-    signal::ctrl_c().await.unwrap();
+    deno_signals::ctrl_c().await.unwrap();
     cancel_sender.send(TestEvent::Sigint).ok();
   });
   HAS_TEST_RUN_SIGINT_HANDLER.store(true, Ordering::Relaxed);
@@ -1734,7 +1733,7 @@ pub async fn run_tests_with_watch(
   // once a user adds one.
   spawn(async move {
     loop {
-      signal::ctrl_c().await.unwrap();
+      deno_signals::ctrl_c().await.unwrap();
       if !HAS_TEST_RUN_SIGINT_HANDLER.load(Ordering::Relaxed) {
         #[allow(clippy::disallowed_methods)]
         std::process::exit(130);

ext/os/lib.rs

@@ -18,7 +18,6 @@ use once_cell::sync::Lazy;
 use serde::Serialize;
 
 mod ops;
-pub mod signal;
 pub mod sys_info;
 
 pub use ops::signal::SignalError;

ext/os/ops/signal.rs

@@ -15,10 +15,10 @@ use deno_core::op2;
 pub enum SignalError {
   #[class(type)]
   #[error(transparent)]
-  InvalidSignalStr(#[from] crate::signal::InvalidSignalStrError),
+  InvalidSignalStr(#[from] deno_signals::InvalidSignalStrError),
   #[class(type)]
   #[error(transparent)]
-  InvalidSignalInt(#[from] crate::signal::InvalidSignalIntError),
+  InvalidSignalInt(#[from] deno_signals::InvalidSignalIntError),
   #[class(type)]
   #[error("Binding to signal '{0}' is not allowed")]
   SignalNotAllowed(String),
@@ -49,7 +49,7 @@ pub fn op_signal_bind(
   state: &mut OpState,
   #[string] sig: &str,
 ) -> Result<ResourceId, SignalError> {
-  let signo = crate::signal::signal_str_to_int(sig)?;
+  let signo = deno_signals::signal_str_to_int(sig)?;
   if deno_signals::is_forbidden(signo) {
     return Err(SignalError::SignalNotAllowed(sig.to_string()));
   }
@@ -61,7 +61,7 @@ pub fn op_signal_bind(
     Box::new(move || {
       let _ = tx.send(());
     }),
-  );
+  )?;
 
   let rid = state.resource_table.add(SignalStreamResource {
     signo,

ext/process/Cargo.toml

@@ -21,6 +21,7 @@ deno_io.workspace = true
 deno_os.workspace = true
 deno_path_util.workspace = true
 deno_permissions.workspace = true
+deno_signals.workspace = true
 deno_subprocess_windows.workspace = true
 libc.workspace = true
 log.workspace = true

ext/process/lib.rs

@@ -313,7 +313,7 @@ impl TryFrom<ExitStatus> for ChildStatus {
         success: false,
         code: 128 + signal,
         #[cfg(unix)]
-        signal: Some(deno_os::signal::signal_int_to_str(signal)?.to_string()),
+        signal: Some(deno_signals::signal_int_to_str(signal)?.to_string()),
         #[cfg(not(unix))]
         signal: None,
       }
@@ -1263,7 +1263,7 @@ mod deprecated {
 
   #[cfg(unix)]
   pub fn kill(pid: i32, signal: &str) -> Result<(), ProcessError> {
-    let signo = deno_os::signal::signal_str_to_int(signal)
+    let signo = deno_signals::signal_str_to_int(signal)
       .map_err(SignalError::InvalidSignalStr)?;
     use nix::sys::signal::Signal;
     use nix::sys::signal::kill as unix_kill;
@@ -1291,7 +1291,7 @@ mod deprecated {
 
     if !matches!(signal, "SIGKILL" | "SIGTERM") {
       Err(
-        SignalError::InvalidSignalStr(deno_os::signal::InvalidSignalStrError(
+        SignalError::InvalidSignalStr(deno_signals::InvalidSignalStrError(
           signal.to_string(),
         ))
         .into(),

ext/signals/Cargo.toml

@@ -14,5 +14,9 @@ description = "Signals for Deno"
 path = "lib.rs"
 
 [dependencies]
+deno_error.workspace = true
+libc.workspace = true
 signal-hook.workspace = true
-winapi.workspace = true
+thiserror.workspace = true
+tokio.workspace = true
+winapi = { workspace = true, features = ["consoleapi"] }

ext/signals/lib.rs

@@ -7,6 +7,10 @@ use std::sync::atomic::AtomicU32;
 use std::sync::atomic::Ordering;
 
 use signal_hook::consts::*;
+use tokio::sync::watch;
+
+mod dict;
+pub use dict::*;
 
 #[cfg(windows)]
 static SIGHUP: i32 = 1;
@@ -91,7 +95,14 @@ pub fn register(
   signal: i32,
   prevent_default: bool,
   f: Box<dyn Fn() + Send>,
-) -> u32 {
+) -> Result<u32, std::io::Error> {
+  if is_forbidden(signal) {
+    return Err(std::io::Error::new(
+      std::io::ErrorKind::Other,
+      format!("Refusing to register signal {signal}"),
+    ));
+  }
+
   let (handle, handlers) = HANDLERS.get_or_init(|| {
     let handle = init();
 
@@ -120,7 +131,7 @@ pub fn register(
     }
   }
 
-  id
+  Ok(id)
 }
 
 pub fn unregister(signal: i32, id: u32) {
@@ -140,9 +151,9 @@ pub fn unregister(signal: i32, id: u32) {
 static BEFORE_EXIT: OnceLock<Mutex<Vec<Handler>>> = OnceLock::new();
 
 pub fn before_exit(f: fn()) {
-  register(SIGHUP, false, Box::new(f));
+  register(SIGHUP, false, Box::new(f)).unwrap();
-  register(SIGTERM, false, Box::new(f));
+  register(SIGTERM, false, Box::new(f)).unwrap();
-  register(SIGINT, false, Box::new(f));
+  register(SIGINT, false, Box::new(f)).unwrap();
   BEFORE_EXIT
     .get_or_init(|| Mutex::new(vec![]))
     .lock()
@@ -162,3 +173,33 @@ pub fn run_exit() {
 pub fn is_forbidden(signo: i32) -> bool {
   FORBIDDEN.contains(&signo)
 }
+
+pub struct SignalStream {
+  rx: watch::Receiver<()>,
+}
+
+impl SignalStream {
+  pub async fn recv(&mut self) -> Option<()> {
+    self.rx.changed().await.ok()
+  }
+}
+
+pub fn signal_stream(signo: i32) -> Result<SignalStream, std::io::Error> {
+  let (tx, rx) = watch::channel(());
+  let cb = Box::new(move || {
+    tx.send_replace(());
+  });
+  register(signo, true, cb)?;
+  Ok(SignalStream { rx })
+}
+
+pub async fn ctrl_c() -> std::io::Result<()> {
+  let mut stream = signal_stream(libc::SIGINT)?;
+  match stream.recv().await {
+    Some(_) => Ok(()),
+    None => Err(std::io::Error::new(
+      std::io::ErrorKind::Other,
+      "failed to receive SIGINT signal",
+    )),
+  }
+}

runtime/Cargo.toml

@@ -62,6 +62,7 @@ deno_path_util.workspace = true
 deno_permissions.workspace = true
 deno_process.workspace = true
 deno_resolver = { workspace = true, features = ["sync"] }
+deno_signals.workspace = true
 deno_telemetry.workspace = true
 deno_terminal.workspace = true
 deno_tls.workspace = true

runtime/worker.rs

@@ -71,10 +71,7 @@ pub(crate) static SIGUSR2_RX: LazyLock<tokio::sync::watch::Receiver<()>> =
     let (tx, rx) = tokio::sync::watch::channel(());
 
     tokio::spawn(async move {
-      let mut sigusr2 = tokio::signal::unix::signal(
+      let mut sigusr2 = deno_signals::signal_stream(libc::SIGUSR2).unwrap();
-        tokio::signal::unix::SignalKind::user_defined2(),
-      )
-      .unwrap();
 
       loop {
         sigusr2.recv().await;