From d5c9c233cc6cc4a3f00d97becfd3370802a2f7f5 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 6 Feb 2026 20:34:06 +0000 Subject: [PATCH 1/2] dd: remove signal-hook dependency, use raw sigaction handler --- Cargo.lock | 15 ++------ Cargo.toml | 1 - src/uu/dd/Cargo.toml | 3 +- src/uu/dd/src/dd.rs | 30 ++++++---------- src/uu/dd/src/progress.rs | 50 ++++++-------------------- src/uucore/src/lib/features/signals.rs | 18 +++++++++- 6 files changed, 42 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 489778c5a07..15fe975972f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -755,7 +755,7 @@ dependencies = [ "mio", "parking_lot", "rustix", - "signal-hook 0.3.18", + "signal-hook", "signal-hook-mio", "winapi", ] @@ -2765,16 +2765,6 @@ dependencies = [ "signal-hook-registry", ] -[[package]] -name = "signal-hook" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b57709da74f9ff9f4a27dce9526eec25ca8407c45a7887243b031a58935fb8e" -dependencies = [ - "libc", - "signal-hook-registry", -] - [[package]] name = "signal-hook-mio" version = "0.2.5" @@ -2783,7 +2773,7 @@ checksum = "b75a19a7a740b25bc7944bdee6172368f988763b744e3d4dfe753f6b4ece40cc" dependencies = [ "libc", "mio", - "signal-hook 0.3.18", + "signal-hook", ] [[package]] @@ -3410,7 +3400,6 @@ dependencies = [ "gcd", "libc", "nix", - "signal-hook 0.4.3", "tempfile", "thiserror 2.0.18", "uucore", diff --git a/Cargo.toml b/Cargo.toml index 5b2b1c83c58..936a27d684c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -377,7 +377,6 @@ self_cell = "1.0.4" # FIXME we use the exact version because the new 0.5.3 requires an MSRV of 1.88 selinux = "=0.5.2" string-interner = "0.19.0" -signal-hook = "0.4.1" tempfile = "3.15.0" terminal_size = "0.4.0" textwrap = { version = "0.16.1", features = ["terminal_size"] } diff --git a/src/uu/dd/Cargo.toml b/src/uu/dd/Cargo.toml index 5d5819c302d..9da9143a82a 100644 --- a/src/uu/dd/Cargo.toml +++ b/src/uu/dd/Cargo.toml @@ -32,8 +32,7 @@ thiserror = { workspace = true } fluent = { workspace = true } [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] -nix = { workspace = true, features = ["fs"] } -signal-hook = { workspace = true } +nix = { workspace = true, features = ["fs", "signal"] } [[bin]] name = "dd" diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 48f7ef1b98e..9f83b475d01 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -23,6 +23,8 @@ use nix::fcntl::OFlag; use parseargs::Parser; use progress::ProgUpdateType; use progress::{ProgUpdate, ReadStat, StatusLevel, WriteStat, gen_prog_updater}; +#[cfg(target_os = "linux")] +use progress::{check_and_reset_sigusr1, install_sigusr1_handler}; use uucore::io::OwnedFileDescriptorOrHandle; use uucore::translate; @@ -122,18 +124,9 @@ impl Alarm { Self { interval, trigger } } - /// Returns a closure that allows to manually trigger the alarm - /// - /// This is useful for cases where more than one alarm even source exists - /// In case of `dd` there is the SIGUSR1/SIGINFO case where we want to - /// trigger an manual progress report. - pub fn manual_trigger_fn(&self) -> Box { - let weak_trigger = Arc::downgrade(&self.trigger); - Box::new(move || { - if let Some(trigger) = weak_trigger.upgrade() { - trigger.store(ALARM_TRIGGER_SIGNAL, Relaxed); - } - }) + /// Manually trigger the alarm as a signal event + pub fn manual_trigger(&self) { + self.trigger.store(ALARM_TRIGGER_SIGNAL, Relaxed); } /// Use this function to poll for any pending alarm event @@ -1183,14 +1176,9 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { // This avoids the need to query the OS monotonic clock for every block. let alarm = Alarm::with_interval(Duration::from_secs(1)); - // The signal handler spawns an own thread that waits for signals. - // When the signal is received, it calls a handler function. - // We inject a handler function that manually triggers the alarm. #[cfg(target_os = "linux")] - let signal_handler = progress::SignalHandler::install_signal_handler(alarm.manual_trigger_fn()); - #[cfg(target_os = "linux")] - if let Err(e) = &signal_handler { - if Some(StatusLevel::None) != i.settings.status { + if let Err(e) = install_sigusr1_handler() { + if i.settings.status != Some(StatusLevel::None) { eprintln!("{}\n\t{e}", translate!("dd-warning-signal-handler")); } } @@ -1270,6 +1258,10 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { // error. rstat += rstat_update; wstat += wstat_update; + #[cfg(target_os = "linux")] + if check_and_reset_sigusr1() { + alarm.manual_trigger(); + } match alarm.get_trigger() { ALARM_TRIGGER_NONE => {} t @ (ALARM_TRIGGER_TIMER | ALARM_TRIGGER_SIGNAL) => { diff --git a/src/uu/dd/src/progress.rs b/src/uu/dd/src/progress.rs index c7aef33875b..cc5527f5573 100644 --- a/src/uu/dd/src/progress.rs +++ b/src/uu/dd/src/progress.rs @@ -10,13 +10,10 @@ //! [`gen_prog_updater`] function can be used to implement a progress //! updater that runs in its own thread. use std::io::Write; -use std::sync::mpsc; #[cfg(target_os = "linux")] -use std::thread::JoinHandle; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc; use std::time::Duration; - -#[cfg(target_os = "linux")] -use signal_hook::iterator::Handle; use uucore::{ error::{UResult, set_exit_code}, format::num_format::{FloatVariant, Formatter}, @@ -448,47 +445,22 @@ pub(crate) fn gen_prog_updater( } } -/// signal handler listens for SIGUSR1 signal and runs provided closure. #[cfg(target_os = "linux")] -pub(crate) struct SignalHandler { - handle: Handle, - thread: Option>, -} +static SIGUSR1_RECEIVED: AtomicBool = AtomicBool::new(false); #[cfg(target_os = "linux")] -impl SignalHandler { - pub(crate) fn install_signal_handler( - f: Box, - ) -> Result { - use signal_hook::consts::signal::SIGUSR1; - use signal_hook::iterator::Signals; - - let mut signals = Signals::new([SIGUSR1])?; - let handle = signals.handle(); - let thread = std::thread::spawn(move || { - for signal in &mut signals { - match signal { - SIGUSR1 => (*f)(), - _ => unreachable!(), - } - } - }); +pub(crate) fn check_and_reset_sigusr1() -> bool { + SIGUSR1_RECEIVED.swap(false, Ordering::Relaxed) +} - Ok(Self { - handle, - thread: Some(thread), - }) - } +#[cfg(target_os = "linux")] +extern "C" fn sigusr1_handler(_: std::os::raw::c_int) { + SIGUSR1_RECEIVED.store(true, Ordering::Relaxed); } #[cfg(target_os = "linux")] -impl Drop for SignalHandler { - fn drop(&mut self) { - self.handle.close(); - if let Some(thread) = std::mem::take(&mut self.thread) { - thread.join().unwrap(); - } - } +pub(crate) fn install_sigusr1_handler() -> Result<(), nix::errno::Errno> { + uucore::signals::install_signal_handler(nix::sys::signal::Signal::SIGUSR1, sigusr1_handler) } /// Return a closure that can be used in its own thread to print progress info. diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 6d0956b39ca..319b3604886 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -14,7 +14,8 @@ use nix::errno::Errno; #[cfg(unix)] use nix::sys::signal::{ - SigHandler::SigDfl, SigHandler::SigIgn, Signal::SIGINT, Signal::SIGPIPE, signal, + SaFlags, SigAction, SigHandler, SigHandler::SigDfl, SigHandler::SigIgn, SigSet, Signal, + Signal::SIGINT, Signal::SIGPIPE, sigaction, signal, }; /// The default signal value. @@ -435,6 +436,21 @@ pub fn ignore_interrupts() -> Result<(), Errno> { unsafe { signal(SIGINT, SigIgn) }.map(|_| ()) } +/// Installs a signal handler. The handler must be async-signal-safe. +#[cfg(unix)] +pub fn install_signal_handler( + sig: Signal, + handler: extern "C" fn(std::os::raw::c_int), +) -> Result<(), Errno> { + let action = SigAction::new( + SigHandler::Handler(handler), + SaFlags::SA_RESTART, + SigSet::empty(), + ); + unsafe { sigaction(sig, &action) }?; + Ok(()) +} + // Detect closed stdin/stdout before Rust reopens them as /dev/null (see issue #2873) #[cfg(unix)] use std::sync::atomic::{AtomicBool, Ordering}; From d422445c73dc06073a0dc1c69f1540a679f2e1cb Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 6 Feb 2026 21:04:44 +0000 Subject: [PATCH 2/2] fix doc comment referencing removed manual_trigger_fn --- src/uu/dd/src/dd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 9f83b475d01..52c8bc4b294 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -92,7 +92,7 @@ struct Settings { /// /// After being constructed with [`Alarm::with_interval`], [`Alarm::get_trigger`] /// will return [`ALARM_TRIGGER_TIMER`] once per the given [`Duration`]. -/// Alarm can be manually triggered with closure returned by [`Alarm::manual_trigger_fn`]. +/// Alarm can be manually triggered with [`Alarm::manual_trigger`]. /// [`Alarm::get_trigger`] will return [`ALARM_TRIGGER_SIGNAL`] in this case. /// /// Can be cloned, but the trigger status is shared across all instances so only