diff --git a/Cargo.lock b/Cargo.lock index 47034aef..00dac9b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,6 +213,12 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "base64" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" + [[package]] name = "base64ct" version = "1.6.0" @@ -655,9 +661,9 @@ checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] name = "errno" -version = "0.3.8" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" dependencies = [ "libc", "windows-sys 0.52.0", @@ -750,9 +756,9 @@ dependencies = [ [[package]] name = "getrandom" -version = "0.2.14" +version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94b22e06ecb0110981051723910cbf0b5f5e09a2062dd7663334ee79a9d1286c" +checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" dependencies = [ "cfg-if", "libc", @@ -2134,10 +2140,11 @@ dependencies = [ [[package]] name = "radicle-signals" -version = "0.10.0" +version = "0.11.0" dependencies = [ "crossbeam-channel", "libc", + "signals_receipts", ] [[package]] @@ -2385,6 +2392,18 @@ dependencies = [ "zeroize", ] +[[package]] +name = "sem_safe" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9110d660d39b5f65f10a978ce0dd5e0a08c50e1d5589cadfad9b9a5f221fdb14" +dependencies = [ + "base64 0.22.1", + "errno", + "getrandom", + "libc", +] + [[package]] name = "serde" version = "1.0.198" @@ -2456,6 +2475,18 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "signals_receipts" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74fa07c27e948f68f2f72241a189850526fbd3b428eee702884ba1883a39610e" +dependencies = [ + "cfg-if", + "errno", + "libc", + "sem_safe", +] + [[package]] name = "signature" version = "1.6.4" diff --git a/radicle-signals/Cargo.toml b/radicle-signals/Cargo.toml index fbe61e04..968ac0e3 100644 --- a/radicle-signals/Cargo.toml +++ b/radicle-signals/Cargo.toml @@ -5,8 +5,9 @@ homepage = "https://radicle.xyz" repository = "https://app.radicle.xyz/seeds/seed.radicle.xyz/rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5" license = "MIT OR Apache-2.0" edition = "2021" -version = "0.10.0" +version = "0.11.0" [dependencies] crossbeam-channel = { version = "0.5.6" } libc = { version = "0.2" } +signals_receipts = { version = "0.2.0", features = ["channel_notify_facility"] } diff --git a/radicle-signals/src/lib.rs b/radicle-signals/src/lib.rs index 5cabe19f..f0e474c0 100644 --- a/radicle-signals/src/lib.rs +++ b/radicle-signals/src/lib.rs @@ -1,7 +1,12 @@ use std::io; -use std::sync::Mutex; use crossbeam_channel as chan; +use signals_receipts::channel_notify_facility::{ + self, FinishError, InstallError, SendError, SignalsChannel as _, UninstallError, +}; +use signals_receipts::SignalNumber; + +use crate::channel_notify_facility_premade::SignalsChannel; /// Operating system signal. #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -16,10 +21,10 @@ pub enum Signal { WindowChanged, } -impl TryFrom for Signal { - type Error = i32; +impl TryFrom for Signal { + type Error = SignalNumber; - fn try_from(value: i32) -> Result { + fn try_from(value: SignalNumber) -> Result { match value { libc::SIGTERM => Ok(Self::Terminate), libc::SIGINT => Ok(Self::Interrupt), @@ -30,90 +35,97 @@ impl TryFrom for Signal { } } -/// Signal notifications are sent via this channel. -static NOTIFY: Mutex>> = Mutex::new(None); +// The signals of interest to handle. +signals_receipts::channel_notify_facility! { + SIGINT + SIGTERM + SIGHUP + SIGWINCH +} -/// A slice of signals to handle. -const SIGNALS: &[i32] = &[libc::SIGINT, libc::SIGTERM, libc::SIGHUP, libc::SIGWINCH]; - -/// Install global signal handlers. +/// Install global signal handlers, with notifications sent to the given +/// `notify` channel. pub fn install(notify: chan::Sender) -> io::Result<()> { - if let Ok(mut channel) = NOTIFY.try_lock() { - if channel.is_some() { - return Err(io::Error::new( - io::ErrorKind::AlreadyExists, - "signal handler is already installed", - )); - } - *channel = Some(notify); + /// The sender type must implement the facility's trait. + #[derive(Debug)] + struct ChanSender(chan::Sender); - unsafe { _install() }?; - } else { - return Err(io::Error::new( - io::ErrorKind::WouldBlock, - "unable to install signal handler", - )); + /// This also does our desired conversion from signal numbers to our + /// `Signal` representation. + impl channel_notify_facility::Sender for ChanSender { + fn send(&self, sig_num: SignalNumber) -> Result<(), SendError> { + if let Ok(sig) = sig_num.try_into() { + self.0.send(sig).or(Err(SendError::Disconnected)) + } else { + debug_assert!(false, "only called for recognized signal numbers"); + // Unrecognized signal numbers would be ignored, but + // this never occurs. + Err(SendError::Ignored) + } + } } - Ok(()) + + SignalsChannel::install_with_outside_channel(ChanSender(notify)).map_err(|e| match e { + InstallError::AlreadyInstalled { unused_notify: _ } => io::Error::new( + io::ErrorKind::AlreadyExists, + "signal handling is already installed", + ), + _ => io::Error::other(e), // The error type is non-exhaustive. + }) } /// Uninstall global signal handlers. +/// +/// The caller must ensure that all `Receiver`s for the other end of the +/// channel are dropped to disconnect the channel, to ensure that the +/// internal "signals-receipt" thread wakes to clean-up (in case it's +/// blocked on sending on the channel). Such dropping usually occurs +/// naturally when uninstalling, since the other end is no longer needed +/// then, and may be done after calling this. Not doing so might +/// deadlock the "signals-receipt" thread. pub fn uninstall() -> io::Result<()> { - if let Ok(mut channel) = NOTIFY.try_lock() { - if channel.is_none() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - "signal handler is already uninstalled", - )); + SignalsChannel::uninstall_with_outside_channel().map_err(|e| match e { + UninstallError::AlreadyUninstalled => io::Error::new( + io::ErrorKind::NotFound, + "signal handling is already uninstalled", + ), + #[allow(clippy::unreachable)] + UninstallError::WrongMethod => { + // SAFETY: Impossible, because `SignalsChannel` is private + // and so `SignalsChannel::install()` is never done. + unreachable!() } - *channel = None; - - unsafe { _uninstall() }?; - } else { - return Err(io::Error::new( - io::ErrorKind::WouldBlock, - "unable to uninstall signal handler", - )); - } - Ok(()) + _ => io::Error::other(e), // The error type is non-exhaustive. + }) } -/// Install global signal handlers. +/// Do [`uninstall()`], terminate the internal "signals-receipt" +/// thread, and wait for that thread to finish. /// -/// # Safety +/// This is provided in case it's ever needed to completely clean-up the +/// facility to be like it hadn't been installed before. It's +/// unnecessary to use this, just to uninstall the handling. Usually, +/// only using `uninstall` is desirable so that the "signals-receipt" +/// thread is kept alive for faster reuse when re-installing the +/// handling. /// -/// Calls `libc` functions safely. -unsafe fn _install() -> io::Result<()> { - for signal in SIGNALS { - if libc::signal(*signal, handler as libc::sighandler_t) == libc::SIG_ERR { - return Err(io::Error::last_os_error()); +/// The caller must ensure that all `Receiver`s for the other end of the +/// channel have **already** been dropped to disconnect the channel, +/// before calling this, to ensure that the "signals-receipt" thread +/// wakes (in case it's blocked on sending on the channel) to see that +/// it must finish. If this is not done, this might deadlock. +pub fn finish() -> io::Result<()> { + SignalsChannel::finish_with_outside_channel().map_err(|e| match e { + FinishError::AlreadyFinished => io::Error::new( + io::ErrorKind::NotFound, + "signal-handling facility is already finished", + ), + #[allow(clippy::unreachable)] + FinishError::WrongMethod => { + // SAFETY: Impossible, because `SignalsChannel` is private + // and so `SignalsChannel::install()` is never done. + unreachable!() } - } - Ok(()) -} - -/// Uninstall global signal handlers. -/// -/// # Safety -/// -/// Calls `libc` functions safely. -unsafe fn _uninstall() -> io::Result<()> { - for signal in SIGNALS { - if libc::signal(*signal, libc::SIG_DFL) == libc::SIG_ERR { - return Err(io::Error::last_os_error()); - } - } - Ok(()) -} - -/// Called by `libc` when a signal is received. -extern "C" fn handler(sig: libc::c_int, _info: *mut libc::siginfo_t, _data: *mut libc::c_void) { - let Ok(sig) = sig.try_into() else { - return; - }; - if let Ok(guard) = NOTIFY.try_lock() { - if let Some(c) = &*guard { - c.try_send(sig).ok(); - } - } + _ => io::Error::other(e), // The error type is non-exhaustive. + }) }