diff --git a/Cargo.lock b/Cargo.lock index ad7d21b..9694e40 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,6 +1850,7 @@ dependencies = [ "rustix", "serde", "serial_test", + "signal-hook", "stacker", "static_assertions", "tempfile", @@ -2178,6 +2179,16 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "signal-hook" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8621587d4798caf8eb44879d42e56b9a93ea5dcd315a6487c357130095b62801" +dependencies = [ + "libc", + "signal-hook-registry", +] + [[package]] name = "signal-hook-registry" version = "1.4.2" diff --git a/Cargo.toml b/Cargo.toml index aa7d44b..280f7ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -74,6 +74,7 @@ hex = { version = "0.4.3" } heck = { version = "0.5.0" } libc = { version = "0.2" } uds = { git = "https://github.com/rosenpass/uds" } +signal-hook = "0.3.17" #Dev dependencies serial_test = "3.2.0" @@ -89,4 +90,4 @@ procspawn = { version = "1.0.1", features = ["test-support"] } #Broker dependencies (might need cleanup or changes) wireguard-uapi = { version = "3.0.0", features = ["xplatform"] } command-fds = "0.2.3" -rustix = { version = "0.38.41", features = ["net", "fs"] } +rustix = { version = "0.38.41", features = ["net", "fs", "process"] } diff --git a/rosenpass/Cargo.toml b/rosenpass/Cargo.toml index 247c2e0..517d964 100644 --- a/rosenpass/Cargo.toml +++ b/rosenpass/Cargo.toml @@ -62,6 +62,7 @@ heck = { workspace = true, optional = true } command-fds = { workspace = true, optional = true } rustix = { workspace = true, optional = true } uds = { workspace = true, optional = true, features = ["mio_1xx"] } +signal-hook = { workspace = true, optional = true } [build-dependencies] anyhow = { workspace = true } @@ -87,5 +88,6 @@ experiment_api = [ "rosenpass-util/experiment_file_descriptor_passing", "rosenpass-wireguard-broker/experiment_api", ] +internal_signal_handling_for_coverage_reports = ["signal-hook"] internal_testing = [] internal_bin_gen_ipc_msg_types = ["hex", "heck"] diff --git a/rosenpass/src/main.rs b/rosenpass/src/main.rs index 42369c5..36f6eaf 100644 --- a/rosenpass/src/main.rs +++ b/rosenpass/src/main.rs @@ -5,6 +5,7 @@ use clap::Parser; use clap_mangen::roff::{roman, Roff}; use log::error; use rosenpass::cli::CliArgs; +use rosenpass_util::functional::run; use std::process::exit; /// Printing custom man sections when generating the man page @@ -77,13 +78,40 @@ pub fn main() { // error!("error dummy"); } - let broker_interface = args.get_broker_interface(); - match args.run(broker_interface, None) { - Ok(_) => {} - Err(e) => { - error!("{e:?}"); - exit(1); + let res = run(|| { + #[cfg(feature = "internal_signal_handling_for_coverage_reports")] + let term_signal = terminate::TerminateRequested::new()?; + + let broker_interface = args.get_broker_interface(); + let err = match args.run(broker_interface, None) { + Ok(()) => return Ok(()), + Err(err) => err, + }; + + // This is very very hacky and just used for coverage measurement + #[cfg(feature = "internal_signal_handling_for_coverage_reports")] + { + let terminated_by_signal = err + .downcast_ref::() + .filter(|e| e.kind() == std::io::ErrorKind::Interrupted) + .filter(|_| term_signal.value()) + .is_some(); + if terminated_by_signal { + log::warn!( + "\ + Terminated by signal; this signal handler is correct during coverage testing \ + but should be otherwise disabled" + ); + return Ok(()); + } } + + Err(err) + }); + + if let Err(e) = res { + error!("{e:?}"); + exit(1); } } @@ -110,3 +138,47 @@ Peischl, Stephan Ajuvo, and Lisa Schmidt."; /// Custom main page section: Bugs. static BUGS_MAN: &str = r" The bugs are tracked at https://github.com/rosenpass/rosenpass/issues."; + +/// These signal handlers are used exclusively used during coverage testing +/// to ensure that the llvm-cov can produce reports during integration tests +/// with multiple processes where subprocesses are terminated via kill(2). +/// +/// llvm-cov does not support producing coverage reports when the process exits +/// through a signal, so this is necessary. +/// +/// The functionality of exiting gracefully upon reception of a terminating signal +/// is desired for the production variant of Rosenpass, but we should make sure +/// to use a higher quality implementation; in particular, we should use signalfd(2). +/// +#[cfg(feature = "internal_signal_handling_for_coverage_reports")] +mod terminate { + use signal_hook::flag::register as sig_register; + use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }; + + /// Automatically register a signal handler for common termination signals; + /// whether one of these signals was issued can be polled using [Self::value]. + /// + /// The signal handler is not removed when this struct goes out of scope. + pub struct TerminateRequested { + value: Arc, + } + + impl TerminateRequested { + /// Register signal handlers watching for common termination signals + pub fn new() -> anyhow::Result { + let value = Arc::new(AtomicBool::new(false)); + for sig in signal_hook::consts::TERM_SIGNALS.iter().copied() { + sig_register(sig, Arc::clone(&value))?; + } + Ok(Self { value }) + } + + /// Check whether a termination signal has been set + pub fn value(&self) -> bool { + self.value.load(Ordering::Relaxed) + } + } +} diff --git a/rosenpass/tests/api-integration-tests-api-setup.rs b/rosenpass/tests/api-integration-tests-api-setup.rs index 7b7624e..d38cad2 100644 --- a/rosenpass/tests/api-integration-tests-api-setup.rs +++ b/rosenpass/tests/api-integration-tests-api-setup.rs @@ -33,8 +33,10 @@ struct KillChild(std::process::Child); impl Drop for KillChild { fn drop(&mut self) { - self.0.kill().discard_result(); - self.0.wait().discard_result() + use rustix::process::{kill_process, Pid, Signal::Term}; + let pid = Pid::from_child(&self.0); + rustix::process::kill_process(pid, Term).discard_result(); + self.0.wait().discard_result(); } } @@ -153,7 +155,6 @@ fn api_integration_api_setup() -> anyhow::Result<()> { peer_b.config_file_path.to_str().context("")?, ]) .stdin(Stdio::null()) - .stderr(Stdio::null()) .stdout(Stdio::piped()) .spawn()?, );