From fd34b680b59ed99b9e1c1283bdbfef8d779efaf5 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Thu, 21 Aug 2025 17:54:53 +0200 Subject: [PATCH] signals: Guard most of the crate for Unix The signals crate does not build on non-Unix-like platforms, such as Windows. It uses constants for signals that are not exported from `libc` on Windows, and transitive dependencies such as `sem_safe` fail to compile. Resolve this, by guarding most of the crate, certainly all real features, by `cfg(unix)`. To require less uses of the `cfg` macro, move all affected parts into a new module, and re-export it. The crate now builds on Windows (although it does not do anything interesting on that platform) and its Unix interface remains unchanged. Changes are integrated into `radicle-node`, but only affect Windows. --- crates/radicle-node/src/main.rs | 16 +++- crates/radicle-node/src/runtime.rs | 5 +- crates/radicle-signals/Cargo.toml | 2 +- crates/radicle-signals/src/lib.rs | 121 +--------------------------- crates/radicle-signals/src/unix.rs | 122 +++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 121 deletions(-) create mode 100644 crates/radicle-signals/src/unix.rs diff --git a/crates/radicle-node/src/main.rs b/crates/radicle-node/src/main.rs index f94dfb56..c1135f34 100644 --- a/crates/radicle-node/src/main.rs +++ b/crates/radicle-node/src/main.rs @@ -8,6 +8,7 @@ use radicle::node::device::Device; use radicle::profile; use radicle_node::crypto::ssh::keystore::{Keystore, MemorySigner}; use radicle_node::{Runtime, VERSION}; +#[cfg(unix)] use radicle_signals as signals; pub const HELP_MSG: &str = r#" @@ -139,8 +140,19 @@ fn execute() -> anyhow::Result<()> { log::warn!(target: "node", "Unable to set process open file limit: {e}"); } - let (notify, signals) = chan::bounded(1); - signals::install(notify)?; + #[cfg(unix)] + let signals = { + let (notify, signals) = chan::bounded(1); + signals::install(notify)?; + signals + }; + + #[cfg(windows)] + let signals = { + let (_, signals) = chan::bounded(1); + log::warn!(target: "node", "Signal handlers not installed."); + signals + }; if options.force { log::debug!(target: "node", "Removing existing control socket.."); diff --git a/crates/radicle-node/src/runtime.rs b/crates/radicle-node/src/runtime.rs index b0de2905..d50ceb0f 100644 --- a/crates/radicle-node/src/runtime.rs +++ b/crates/radicle-node/src/runtime.rs @@ -23,7 +23,6 @@ use radicle::node::address::Store as _; use radicle::node::notifications; use radicle::node::policy::config as policy; use radicle::node::Event; -use radicle::node::Handle as _; use radicle::node::UserAgent; use radicle::profile::Home; use radicle::{cob, git, storage, Storage}; @@ -280,7 +279,11 @@ impl Runtime { let handle = self.handle.clone(); || control::listen(listener, handle) }); + + #[cfg(unix)] let _signals = thread::spawn(&self.id, "signals", move || loop { + use radicle::node::Handle as _; + match self.signals.recv() { Ok(Signal::Terminate | Signal::Interrupt) => { log::info!(target: "node", "Termination signal received; shutting down.."); diff --git a/crates/radicle-signals/Cargo.toml b/crates/radicle-signals/Cargo.toml index f46e507f..821b3311 100644 --- a/crates/radicle-signals/Cargo.toml +++ b/crates/radicle-signals/Cargo.toml @@ -8,7 +8,7 @@ edition.workspace = true version = "0.11.0" rust-version.workspace = true -[dependencies] +[target.'cfg(unix)'.dependencies] crossbeam-channel = { workspace = true } libc = { workspace = true } signals_receipts = { version = "0.2.0", features = ["channel_notify_facility"] } diff --git a/crates/radicle-signals/src/lib.rs b/crates/radicle-signals/src/lib.rs index f0e474c0..824c2d9f 100644 --- a/crates/radicle-signals/src/lib.rs +++ b/crates/radicle-signals/src/lib.rs @@ -1,12 +1,8 @@ -use std::io; +#[cfg(unix)] +mod unix; -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; +#[cfg(unix)] +pub use unix::*; /// Operating system signal. #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -20,112 +16,3 @@ pub enum Signal { /// `SIGWINCH`. WindowChanged, } - -impl TryFrom for Signal { - type Error = SignalNumber; - - fn try_from(value: SignalNumber) -> Result { - match value { - libc::SIGTERM => Ok(Self::Terminate), - libc::SIGINT => Ok(Self::Interrupt), - libc::SIGWINCH => Ok(Self::WindowChanged), - libc::SIGHUP => Ok(Self::Hangup), - _ => Err(value), - } - } -} - -// The signals of interest to handle. -signals_receipts::channel_notify_facility! { - SIGINT - SIGTERM - SIGHUP - SIGWINCH -} - -/// Install global signal handlers, with notifications sent to the given -/// `notify` channel. -pub fn install(notify: chan::Sender) -> io::Result<()> { - /// The sender type must implement the facility's trait. - #[derive(Debug)] - struct ChanSender(chan::Sender); - - /// 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) - } - } - } - - 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<()> { - 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!() - } - _ => io::Error::other(e), // The error type is non-exhaustive. - }) -} - -/// Do [`uninstall()`], terminate the internal "signals-receipt" -/// thread, and wait for that thread to finish. -/// -/// 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. -/// -/// 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!() - } - _ => io::Error::other(e), // The error type is non-exhaustive. - }) -} diff --git a/crates/radicle-signals/src/unix.rs b/crates/radicle-signals/src/unix.rs new file mode 100644 index 00000000..a7e37db1 --- /dev/null +++ b/crates/radicle-signals/src/unix.rs @@ -0,0 +1,122 @@ +use std::io; + +use crossbeam_channel as chan; + +use signals_receipts::channel_notify_facility::{ + self, FinishError, InstallError, SendError, SignalsChannel as _, UninstallError, +}; + +use signals_receipts::SignalNumber; + +use crate::*; + +use crate::unix::channel_notify_facility_premade::SignalsChannel; + +impl TryFrom for Signal { + type Error = SignalNumber; + + fn try_from(value: SignalNumber) -> Result { + match value { + libc::SIGTERM => Ok(Self::Terminate), + libc::SIGINT => Ok(Self::Interrupt), + libc::SIGWINCH => Ok(Self::WindowChanged), + libc::SIGHUP => Ok(Self::Hangup), + _ => Err(value), + } + } +} + +// The signals of interest to handle. +signals_receipts::channel_notify_facility! { + SIGINT + SIGTERM + SIGHUP + SIGWINCH +} + +/// Install global signal handlers, with notifications sent to the given +/// `notify` channel. +pub fn install(notify: chan::Sender) -> io::Result<()> { + /// The sender type must implement the facility's trait. + #[derive(Debug)] + struct ChanSender(chan::Sender); + + /// 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) + } + } + } + + 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<()> { + 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!() + } + _ => io::Error::other(e), // The error type is non-exhaustive. + }) +} + +/// Do [`uninstall()`], terminate the internal "signals-receipt" +/// thread, and wait for that thread to finish. +/// +/// 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. +/// +/// 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!() + } + _ => io::Error::other(e), // The error type is non-exhaustive. + }) +}