From 403bee37528bbdf0d6b553a6ffa5978fe3a77275 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sun, 1 Feb 2026 13:55:59 +0000 Subject: [PATCH] Move Ctrl+C handler to karva_runner with config flag Add `create_ctrlc_handler` field to `ParallelTestConfig` to control whether the shutdown signal handler is installed. This moves the handler setup from the karva crate into karva_runner, using `OnceLock` for idempotent initialization. The change eliminates the panic when calling `run_parallel_tests` multiple times and removes the messy `Option<&Receiver<()>>` parameter. Fixes #393 Co-Authored-By: Claude Opus 4.5 --- Cargo.lock | 3 +- crates/karva/Cargo.toml | 2 -- crates/karva/src/lib.rs | 10 ++---- crates/karva_benchmark/src/walltime.rs | 3 +- crates/karva_runner/Cargo.toml | 1 + crates/karva_runner/src/lib.rs | 1 + crates/karva_runner/src/orchestration.rs | 15 ++++++++- crates/karva_runner/src/shutdown.rs | 40 ++++++++++++++++++++++++ 8 files changed, 61 insertions(+), 14 deletions(-) create mode 100644 crates/karva_runner/src/shutdown.rs diff --git a/Cargo.lock b/Cargo.lock index f85b5ae9..df09cee8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1171,8 +1171,6 @@ dependencies = [ "camino", "clap", "colored 3.1.1", - "crossbeam-channel", - "ctrlc", "directories", "dunce", "fs4", @@ -1414,6 +1412,7 @@ dependencies = [ "anyhow", "camino", "crossbeam-channel", + "ctrlc", "ignore", "karva_cache", "karva_cli", diff --git a/crates/karva/Cargo.toml b/crates/karva/Cargo.toml index 8e379752..4cd6bb12 100644 --- a/crates/karva/Cargo.toml +++ b/crates/karva/Cargo.toml @@ -30,8 +30,6 @@ argfile = { workspace = true } camino = { workspace = true } clap = { workspace = true, features = ["wrap_help", "string", "env"] } colored = { workspace = true } -crossbeam-channel = { workspace = true } -ctrlc = { workspace = true } tracing = { workspace = true, features = ["release_max_level_debug"] } wild = { workspace = true } diff --git a/crates/karva/src/lib.rs b/crates/karva/src/lib.rs index cb2cc095..aa62fe9f 100644 --- a/crates/karva/src/lib.rs +++ b/crates/karva/src/lib.rs @@ -120,18 +120,12 @@ pub(crate) fn test(args: TestCommand) -> Result { let config = karva_runner::ParallelTestConfig { num_workers, no_cache, + create_ctrlc_handler: true, }; - let (shutdown_tx, shutdown_rx) = crossbeam_channel::unbounded(); - - ctrlc::set_handler(move || { - let _ = shutdown_tx.send(()); - }) - .expect("Error setting Ctrl+C handler"); - let start_time = Instant::now(); - let result = karva_runner::run_parallel_tests(&db, &config, &sub_command, Some(&shutdown_rx))?; + let result = karva_runner::run_parallel_tests(&db, &config, &sub_command)?; print_test_output( printer, diff --git a/crates/karva_benchmark/src/walltime.rs b/crates/karva_benchmark/src/walltime.rs index b3a3a70a..354d195e 100644 --- a/crates/karva_benchmark/src/walltime.rs +++ b/crates/karva_benchmark/src/walltime.rs @@ -62,6 +62,7 @@ fn test_project(project: &ProjectDatabase) { let config = karva_runner::ParallelTestConfig { num_workers, no_cache: false, + create_ctrlc_handler: false, }; let args = SubTestCommand { @@ -71,7 +72,7 @@ fn test_project(project: &ProjectDatabase) { ..SubTestCommand::default() }; - karva_runner::run_parallel_tests(project, &config, &args, None).unwrap(); + karva_runner::run_parallel_tests(project, &config, &args).unwrap(); } pub fn bench_project(bencher: Bencher, benchmark: &ProjectBenchmark) { diff --git a/crates/karva_runner/Cargo.toml b/crates/karva_runner/Cargo.toml index 973cd592..6f8c282e 100644 --- a/crates/karva_runner/Cargo.toml +++ b/crates/karva_runner/Cargo.toml @@ -21,6 +21,7 @@ karva_system = { workspace = true } anyhow = { workspace = true } camino = { workspace = true } crossbeam-channel = { workspace = true } +ctrlc = { workspace = true } ignore = { workspace = true } tracing = { workspace = true } which = { workspace = true } diff --git a/crates/karva_runner/src/lib.rs b/crates/karva_runner/src/lib.rs index 57ac9496..d0e53f68 100644 --- a/crates/karva_runner/src/lib.rs +++ b/crates/karva_runner/src/lib.rs @@ -1,5 +1,6 @@ mod collection; mod orchestration; mod partition; +mod shutdown; pub use orchestration::{ParallelTestConfig, run_parallel_tests}; diff --git a/crates/karva_runner/src/orchestration.rs b/crates/karva_runner/src/orchestration.rs index 06150cc6..10a23311 100644 --- a/crates/karva_runner/src/orchestration.rs +++ b/crates/karva_runner/src/orchestration.rs @@ -4,6 +4,8 @@ use std::time::{Duration, Instant}; use anyhow::{Context, Result}; use camino::Utf8PathBuf; use crossbeam_channel::{Receiver, TryRecvError}; + +use crate::shutdown::shutdown_receiver; use karva_cache::{ AggregatedResults, CACHE_DIR, CacheReader, RunHash, reader::read_recent_durations, }; @@ -118,6 +120,12 @@ impl WorkerManager { pub struct ParallelTestConfig { pub num_workers: usize, pub no_cache: bool, + /// Whether to create a Ctrl+C handler for graceful shutdown. + /// + /// When `true`, a signal handler is installed (idempotently) to handle + /// Ctrl+C and gracefully stop workers. Set to `false` in contexts where + /// the handler should not be installed (e.g., benchmarks). + pub create_ctrlc_handler: bool, } /// Spawn worker processes for each partition @@ -179,7 +187,6 @@ pub fn run_parallel_tests( db: &ProjectDatabase, config: &ParallelTestConfig, args: &SubTestCommand, - shutdown_rx: Option<&Receiver<()>>, ) -> Result { let mut test_paths = Vec::new(); @@ -238,6 +245,12 @@ pub fn run_parallel_tests( let mut worker_manager = spawn_workers(db, &partitions, &cache_dir, &run_hash, args)?; + let shutdown_rx = if config.create_ctrlc_handler { + Some(shutdown_receiver()) + } else { + None + }; + let num_workers = worker_manager.wait_all(shutdown_rx); for worker in &mut worker_manager.workers { diff --git a/crates/karva_runner/src/shutdown.rs b/crates/karva_runner/src/shutdown.rs new file mode 100644 index 00000000..c912aa42 --- /dev/null +++ b/crates/karva_runner/src/shutdown.rs @@ -0,0 +1,40 @@ +use std::sync::OnceLock; + +use crossbeam_channel::Receiver; + +static SHUTDOWN_CHANNEL: OnceLock<(crossbeam_channel::Sender<()>, Receiver<()>)> = OnceLock::new(); + +/// Returns a reference to the global shutdown receiver. +/// +/// The first call initializes the channel and sets up the Ctrl+C handler. +/// Subsequent calls return the same receiver. This is safe to call multiple +/// times (idempotent). +pub fn shutdown_receiver() -> &'static Receiver<()> { + let (_, rx) = SHUTDOWN_CHANNEL.get_or_init(|| { + let (tx, rx) = crossbeam_channel::unbounded(); + + let handler_tx = tx.clone(); + if let Err(err) = ctrlc::set_handler(move || { + let _ = handler_tx.send(()); + }) { + tracing::warn!("Failed to set Ctrl+C handler: {err}"); + } + + (tx, rx) + }); + rx +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_shutdown_receiver_is_idempotent() { + let rx1 = shutdown_receiver(); + let rx2 = shutdown_receiver(); + + // Both calls should return the same receiver + assert!(std::ptr::eq(rx1, rx2)); + } +}