signal: optimize unix signal storage to skip zero (#7819)

tokio/src/signal/registry.rs

@@ -50,12 +50,6 @@ impl Storage for Vec<EventInfo> {
     }
 }
 
-/// 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,
+    OsExtraData: 'static + Send + Sync + Default,
-    OsStorage: 'static + Send + Sync + Init,
+    OsStorage: 'static + Send + Sync + Default,
 {
     Globals {
-        extra: OsExtraData::init(),
+        extra: OsExtraData::default(),
-        registry: Registry::new(OsStorage::init()),
+        registry: Registry::new(OsStorage::default()),
     }
 }
 
 pub(crate) fn globals() -> &'static Globals
 where
-    OsExtraData: 'static + Send + Sync + Init,
+    OsExtraData: 'static + Send + Sync + Default,
-    OsStorage: 'static + Send + Sync + Init,
+    OsStorage: 'static + Send + Sync + Default,
 {
     static GLOBALS: OnceLock<Globals> = OnceLock::new();
 

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 {
+impl OsStorage {
-    fn init() -> Self {
+    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)
+        let inner = std::iter::repeat_with(SignalInfo::default)
-            .take(libc::SIGRTMAX() as usize + 1)
+            .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 {
+impl Default for OsExtraData {
-    fn init() -> Self {
+    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(
+        let inputs = signal_hook_registry::FORBIDDEN;
-            SignalKind::from_raw(signal_hook_registry::FORBIDDEN[0]),
+
-            &Handle::default(),
+        for &input in inputs {
-        )
+            assert_eq!(
-        .unwrap_err();
+                signal_enable(SignalKind::from_raw(input), &Handle::default())
+                    .unwrap_err()
+                    .kind(),
+                ErrorKind::Other,
+            );
+        }
     }
 
     #[test]

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();