From 46674789abdcd72189bb48a82c6c2121bc3922a0 Mon Sep 17 00:00:00 2001 From: Tim Vilgot Mikael Fredenberg <26655508+vilgotf@users.noreply.github.com> Date: Fri, 2 Jan 2026 15:24:55 +0100 Subject: [PATCH] signal: optimize unix signal storage to skip zero (#7819) --- tokio/src/signal/registry.rs | 18 ++++------ tokio/src/signal/unix.rs | 58 +++++++++++++++++++++++---------- tokio/src/signal/windows/sys.rs | 24 ++------------ 3 files changed, 49 insertions(+), 51 deletions(-) diff --git a/tokio/src/signal/registry.rs b/tokio/src/signal/registry.rs index 7ebdc028d..2a3eea398 100644 --- a/tokio/src/signal/registry.rs +++ b/tokio/src/signal/registry.rs @@ -50,12 +50,6 @@ impl Storage for Vec { } } -/// An interface for initializing a type. Useful for situations where we cannot -/// inject a configured instance in the constructor of another type. -pub(crate) trait Init { - fn init() -> Self; -} - /// Manages and distributes event notifications to any registered listeners. /// /// Generic over the underlying storage to allow for domain specific @@ -150,19 +144,19 @@ impl Globals { fn globals_init() -> Globals where - OsExtraData: 'static + Send + Sync + Init, - OsStorage: 'static + Send + Sync + Init, + OsExtraData: 'static + Send + Sync + Default, + OsStorage: 'static + Send + Sync + Default, { Globals { - extra: OsExtraData::init(), - registry: Registry::new(OsStorage::init()), + extra: OsExtraData::default(), + registry: Registry::new(OsStorage::default()), } } pub(crate) fn globals() -> &'static Globals where - OsExtraData: 'static + Send + Sync + Init, - OsStorage: 'static + Send + Sync + Init, + OsExtraData: 'static + Send + Sync + Default, + OsStorage: 'static + Send + Sync + Default, { static GLOBALS: OnceLock = OnceLock::new(); diff --git a/tokio/src/signal/unix.rs b/tokio/src/signal/unix.rs index 56bec97ff..ea75e8215 100644 --- a/tokio/src/signal/unix.rs +++ b/tokio/src/signal/unix.rs @@ -8,7 +8,7 @@ use crate::runtime::scheduler; use crate::runtime::signal::Handle; -use crate::signal::registry::{globals, EventId, EventInfo, Globals, Init, Storage}; +use crate::signal::registry::{globals, EventId, EventInfo, Globals, Storage}; use crate::signal::RxFuture; use crate::sync::watch; @@ -19,24 +19,32 @@ use std::sync::Once; use std::task::{Context, Poll}; #[cfg(not(any(target_os = "linux", target_os = "illumos")))] -pub(crate) type OsStorage = [SignalInfo; 34]; +pub(crate) struct OsStorage([SignalInfo; 33]); #[cfg(any(target_os = "linux", target_os = "illumos"))] -pub(crate) type OsStorage = Box<[SignalInfo]>; +pub(crate) struct OsStorage(Box<[SignalInfo]>); -impl Init for OsStorage { - fn init() -> Self { +impl OsStorage { + fn get(&self, id: EventId) -> Option<&SignalInfo> { + self.0.get(id - 1) + } +} + +impl Default for OsStorage { + fn default() -> Self { // There are reliable signals ranging from 1 to 33 available on every Unix platform. #[cfg(not(any(target_os = "linux", target_os = "illumos")))] - return std::array::from_fn(|_| SignalInfo::default()); + let inner = std::array::from_fn(|_| SignalInfo::default()); // On Linux and illumos, there are additional real-time signals // available. (This is also likely true on Solaris, but this should be // verified before being enabled.) #[cfg(any(target_os = "linux", target_os = "illumos"))] - return std::iter::repeat_with(SignalInfo::default) - .take(libc::SIGRTMAX() as usize + 1) + let inner = std::iter::repeat_with(SignalInfo::default) + .take(libc::SIGRTMAX() as usize) .collect(); + + Self(inner) } } @@ -49,7 +57,7 @@ impl Storage for OsStorage { where F: FnMut(&'a EventInfo), { - self.iter().map(|si| &si.event_info).for_each(f); + self.0.iter().map(|si| &si.event_info).for_each(f); } } @@ -59,8 +67,8 @@ pub(crate) struct OsExtraData { pub(crate) receiver: UnixStream, } -impl Init for OsExtraData { - fn init() -> Self { +impl Default for OsExtraData { + fn default() -> Self { let (receiver, sender) = UnixStream::pair().expect("failed to create UnixStream"); Self { sender, receiver } @@ -271,7 +279,7 @@ fn action(globals: &'static Globals, signal: libc::c_int) { /// returning any error along the way if that fails. fn signal_enable(signal: SignalKind, handle: &Handle) -> io::Result<()> { let signal = signal.0; - if signal < 0 || signal_hook_registry::FORBIDDEN.contains(&signal) { + if signal <= 0 || signal_hook_registry::FORBIDDEN.contains(&signal) { return Err(Error::new( ErrorKind::Other, format!("Refusing to register signal {signal}"), @@ -523,16 +531,30 @@ mod tests { #[test] fn signal_enable_error_on_invalid_input() { - signal_enable(SignalKind::from_raw(-1), &Handle::default()).unwrap_err(); + let inputs = [-1, 0]; + + for input in inputs { + assert_eq!( + signal_enable(SignalKind::from_raw(input), &Handle::default()) + .unwrap_err() + .kind(), + ErrorKind::Other, + ); + } } #[test] fn signal_enable_error_on_forbidden_input() { - signal_enable( - SignalKind::from_raw(signal_hook_registry::FORBIDDEN[0]), - &Handle::default(), - ) - .unwrap_err(); + let inputs = signal_hook_registry::FORBIDDEN; + + for &input in inputs { + assert_eq!( + signal_enable(SignalKind::from_raw(input), &Handle::default()) + .unwrap_err() + .kind(), + ErrorKind::Other, + ); + } } #[test] diff --git a/tokio/src/signal/windows/sys.rs b/tokio/src/signal/windows/sys.rs index 70329e2b6..518560ecf 100644 --- a/tokio/src/signal/windows/sys.rs +++ b/tokio/src/signal/windows/sys.rs @@ -1,7 +1,7 @@ use std::io; use std::sync::Once; -use crate::signal::registry::{globals, EventId, EventInfo, Init, Storage}; +use crate::signal::registry::{globals, EventId, EventInfo, Storage}; use crate::signal::RxFuture; use windows_sys::core::BOOL; @@ -48,7 +48,7 @@ fn event_requires_infinite_sleep_in_handler(signum: u32) -> bool { } } -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct OsStorage { ctrl_break: EventInfo, ctrl_close: EventInfo, @@ -57,18 +57,6 @@ pub(crate) struct OsStorage { ctrl_shutdown: EventInfo, } -impl Init for OsStorage { - fn init() -> Self { - Self { - ctrl_break: Default::default(), - ctrl_close: Default::default(), - ctrl_c: Default::default(), - ctrl_logoff: Default::default(), - ctrl_shutdown: Default::default(), - } - } -} - impl Storage for OsStorage { fn event_info(&self, id: EventId) -> Option<&EventInfo> { match u32::try_from(id) { @@ -93,15 +81,9 @@ impl Storage for OsStorage { } } -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct OsExtraData {} -impl Init for OsExtraData { - fn init() -> Self { - Self {} - } -} - fn global_init() -> io::Result<()> { static INIT: Once = Once::new();