From 7e8eaa89897c3df254b212ae7fb45617a48ea19f Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Mon, 17 Jun 2024 19:10:43 -0400 Subject: [PATCH 1/2] feat: Add Ctrl-C handler for spawned children --- Cargo.lock | 48 +++++++++++++++++++++++++++ src/bin/bluebuild.rs | 4 ++- utils/Cargo.toml | 2 ++ utils/src/ctrlc_handler.rs | 68 ++++++++++++++++++++++++++++++++++++++ utils/src/lib.rs | 1 + utils/src/logging.rs | 6 ++++ 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 utils/src/ctrlc_handler.rs diff --git a/Cargo.lock b/Cargo.lock index ae1364fe..a3e48c76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -287,12 +287,14 @@ dependencies = [ "chrono", "clap", "colored", + "ctrlc", "directories", "format_serde_error", "indicatif", "indicatif-log-bridge", "log", "log4rs", + "nix 0.29.0", "nu-ansi-term", "once_cell", "os_pipe", @@ -329,6 +331,18 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "chrono" version = "0.4.38" @@ -553,6 +567,16 @@ dependencies = [ "typenum", ] +[[package]] +name = "ctrlc" +version = "3.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "672465ae37dc1bc6380a6547a8883d5dd397b0f1faaad4f265726cc7042a5345" +dependencies = [ + "nix 0.28.0", + "windows-sys 0.52.0", +] + [[package]] name = "deranged" version = "0.3.11" @@ -1158,6 +1182,30 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab250442c86f1850815b5d268639dff018c0627022bc1940eb2d642ca1ce12f0" +[[package]] +name = "nix" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" +dependencies = [ + "bitflags 2.5.0", + "cfg-if", + "cfg_aliases 0.1.1", + "libc", +] + +[[package]] +name = "nix" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" +dependencies = [ + "bitflags 2.5.0", + "cfg-if", + "cfg_aliases 0.2.1", + "libc", +] + [[package]] name = "nom" version = "7.1.3" diff --git a/src/bin/bluebuild.rs b/src/bin/bluebuild.rs index d50ae5f3..e9f7844d 100644 --- a/src/bin/bluebuild.rs +++ b/src/bin/bluebuild.rs @@ -1,5 +1,5 @@ use blue_build::commands::{BlueBuildArgs, BlueBuildCommand, CommandArgs}; -use blue_build_utils::logging::Logger; +use blue_build_utils::{ctrlc_handler, logging::Logger}; use clap::Parser; use log::LevelFilter; @@ -12,6 +12,8 @@ fn main() { .log_out_dir(args.log_out.clone()) .init(); + ctrlc_handler::init(); + log::trace!("Parsed arguments: {args:#?}"); match args.command { diff --git a/utils/Cargo.toml b/utils/Cargo.toml index c953c6be..0e8d3f0e 100644 --- a/utils/Cargo.toml +++ b/utils/Cargo.toml @@ -12,9 +12,11 @@ license.workspace = true atty = "0.2" base64 = "0.22.1" blake2 = "0.10.6" +ctrlc = { version = "3.4.4", features = ["termination"] } directories = "5" rand = "0.8.5" log4rs = { version = "1.3.0", features = ["background_rotation"] } +nix = { version = "0.29.0", features = ["signal"] } nu-ansi-term = { version = "0.50.0", features = ["gnu_legacy"] } os_pipe = { version = "1", features = ["io_safety"] } process_control = { version = "4", features = ["crossbeam-channel"] } diff --git a/utils/src/ctrlc_handler.rs b/utils/src/ctrlc_handler.rs new file mode 100644 index 00000000..0d7ae985 --- /dev/null +++ b/utils/src/ctrlc_handler.rs @@ -0,0 +1,68 @@ +use std::{ + process, + sync::{Arc, Mutex}, +}; + +use log::error; +use nix::{ + sys::signal::{kill, Signal::SIGTERM}, + unistd::Pid, +}; +use once_cell::sync::Lazy; + +static PID_LIST: Lazy>>> = Lazy::new(|| Arc::new(Mutex::new(vec![]))); + +/// Initialize Ctrl-C handler. This should be done at the start +/// of a binary. +/// +/// # Panics +/// Will panic if initialized more than once. +pub fn init() { + let pid_list = PID_LIST.clone(); + ctrlc::set_handler(move || { + let pid_list = pid_list.lock().expect("Should lock mutex"); + pid_list.iter().for_each(|pid| { + if let Err(e) = kill(Pid::from_raw(*pid), SIGTERM) { + error!("Failed to kill process {pid}: Error {e}"); + } + }); + drop(pid_list); + process::exit(1); + }) + .expect("Should create ctrlc handler"); +} + +/// Add a pid to the list to kill when the program +/// recieves a kill signal. +/// +/// # Panics +/// Will panic if the mutex cannot be locked. +pub fn add_pid(pid: T) +where + T: TryInto, +{ + if let Ok(pid) = pid.try_into() { + let mut pid_list = PID_LIST.lock().expect("Should lock pid_list"); + + if !pid_list.contains(&pid) { + pid_list.push(pid); + } + } +} + +/// Remove a pid from the list of pids to kill. +/// +/// # Panics +/// Will panic if the mutex cannot be locked. +pub fn remove_pid(pid: T) +where + T: TryInto, +{ + if let Ok(pid) = pid.try_into() { + let mut pid_list = PID_LIST.lock().expect("Should lock pid_list"); + + if let Some(index) = pid_list.iter().position(|val| *val == pid) { + pid_list.swap_remove(index); + } + } +} diff --git a/utils/src/lib.rs b/utils/src/lib.rs index bf95e250..2b295d39 100644 --- a/utils/src/lib.rs +++ b/utils/src/lib.rs @@ -1,5 +1,6 @@ pub mod command_output; pub mod constants; +pub mod ctrlc_handler; pub mod logging; pub mod syntax_highlighting; diff --git a/utils/src/logging.rs b/utils/src/logging.rs index 9f3fc77c..a3298216 100644 --- a/utils/src/logging.rs +++ b/utils/src/logging.rs @@ -33,6 +33,8 @@ use once_cell::sync::Lazy; use rand::Rng; use typed_builder::TypedBuilder; +use crate::ctrlc_handler::{add_pid, remove_pid}; + static MULTI_PROGRESS: Lazy = Lazy::new(MultiProgress::new); static LOG_DIR: Lazy> = Lazy::new(|| Mutex::new(PathBuf::new())); @@ -205,6 +207,9 @@ impl CommandLogging for Command { let mut child = self.spawn()?; + let child_pid = child.id(); + add_pid(child_pid); + // We drop the `Command` to prevent blocking on writer // https://docs.rs/os_pipe/latest/os_pipe/#examples drop(self); @@ -243,6 +248,7 @@ impl CommandLogging for Command { }); let status = child.wait()?; + remove_pid(child_pid); progress.finish(); Logger::multi_progress().remove(&progress); From 46f91b29670a242d676aff148a120cc282690d79 Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Mon, 17 Jun 2024 19:26:42 -0400 Subject: [PATCH 2/2] Add clippy allow for shadow COMMIT_HASH --- src/commands/generate.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/commands/generate.rs b/src/commands/generate.rs index cea56bda..80a8a70a 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -119,13 +119,16 @@ impl GenerateCommand { .recipe(&recipe_de) .recipe_path(recipe_path.as_path()) .registry(self.get_registry()) - .exports_tag(if shadow::COMMIT_HASH.is_empty() { - // This is done for users who install via - // cargo. Cargo installs do not carry git - // information via shadow - format!("v{}", crate_version!()) - } else { - shadow::COMMIT_HASH.to_string() + .exports_tag({ + #[allow(clippy::const_is_empty)] + if shadow::COMMIT_HASH.is_empty() { + // This is done for users who install via + // cargo. Cargo installs do not carry git + // information via shadow + format!("v{}", crate_version!()) + } else { + shadow::COMMIT_HASH.to_string() + } }) .build();