From 821114f650435c89b9180f76c29e58a29a33c3e2 Mon Sep 17 00:00:00 2001 From: Harnoor Lal Date: Sun, 8 Mar 2026 19:42:45 -0700 Subject: [PATCH 1/2] fix(cli): replace process::exit with ExitCode returns process::exit bypasses Drop, which skips joining the cache-write background thread and can corrupt .chainsaw.cache. Replace all three process::exit(1) calls in main.rs with ExitCode-based control flow: - main() returns ExitCode instead of calling process::exit - run() returns Result for typed exit codes - --chain/--cut "no results" paths return Ok(ExitCode::FAILURE) Adds a structural regression test that scans production code for any process::exit calls, preventing reintroduction. Fixes #148 --- src/main.rs | 71 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/src/main.rs b/src/main.rs index d504fac..05e4280 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; use std::io::IsTerminal; use std::path::{Path, PathBuf}; +use std::process::ExitCode; use std::sync::Arc; use std::time::Instant; @@ -197,7 +198,7 @@ fn resolve_color(no_color: bool) -> bool { /// cold builds and 21% faster on cached builds, with 2x less total kernel time. const MAX_WALKER_THREADS: usize = 8; -fn main() { +fn main() -> ExitCode { let cpus = std::thread::available_parallelism() .map(std::num::NonZero::get) .unwrap_or(1); @@ -211,16 +212,19 @@ fn main() { let no_color = cli.no_color; let sc = report::StderrColor::new(no_color); - if let Err(e) = run(cli.command, no_color, sc) { - eprintln!("{} {e}", sc.error("error:")); - if let Some(hint) = e.hint() { - eprintln!("hint: {hint}"); + match run(cli.command, no_color, sc) { + Ok(code) => code, + Err(e) => { + eprintln!("{} {e}", sc.error("error:")); + if let Some(hint) = e.hint() { + eprintln!("hint: {hint}"); + } + ExitCode::FAILURE } - std::process::exit(1); } } -fn run(command: Commands, no_color: bool, sc: report::StderrColor) -> Result<(), Error> { +fn run(command: Commands, no_color: bool, sc: report::StderrColor) -> Result { let color = resolve_color(no_color); match command { Commands::Trace(args) => run_trace(args, color, sc), @@ -231,11 +235,11 @@ fn run(command: Commands, no_color: bool, sc: report::StderrColor) -> Result<(), entry, limit, quiet, - } => run_diff(a, b, entry, limit, quiet, color, sc), + } => run_diff(a, b, entry, limit, quiet, color, sc).map(|()| ExitCode::SUCCESS), - Commands::Packages(ref args) => run_packages(args, color, sc), + Commands::Packages(ref args) => run_packages(args, color, sc).map(|()| ExitCode::SUCCESS), - Commands::Repl { ref entry } => repl::run(entry, no_color, sc), + Commands::Repl { ref entry } => repl::run(entry, no_color, sc).map(|()| ExitCode::SUCCESS), Commands::Completions { shell } => { clap_complete::generate( @@ -244,7 +248,7 @@ fn run(command: Commands, no_color: bool, sc: report::StderrColor) -> Result<(), "chainsaw", &mut std::io::stdout(), ); - Ok(()) + Ok(ExitCode::SUCCESS) } } } @@ -253,7 +257,7 @@ fn run(command: Commands, no_color: bool, sc: report::StderrColor) -> Result<(), // trace subcommand // --------------------------------------------------------------------------- -fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<(), Error> { +fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result { let start = Instant::now(); // Validate mutually exclusive flags before loading graph @@ -311,9 +315,9 @@ fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<() print!("{}", report.to_terminal(color)); } if report.chains.is_empty() { - std::process::exit(1); + return Ok(ExitCode::FAILURE); } - return Ok(()); + return Ok(ExitCode::SUCCESS); } // --cut @@ -329,9 +333,9 @@ fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<() print!("{}", report.to_terminal(color)); } if report.chain_count == 0 { - std::process::exit(1); + return Ok(ExitCode::FAILURE); } - return Ok(()); + return Ok(ExitCode::SUCCESS); } // --diff-from @@ -344,12 +348,12 @@ fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<() } else { print!("{}", report.to_terminal(color)); } - return Ok(()); + return Ok(ExitCode::SUCCESS); } // --diff if let Some(ref diff_path) = args.diff { - return handle_trace_diff( + handle_trace_diff( &session, diff_path, &result, @@ -359,7 +363,8 @@ fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<() args.limit, color, sc, - ); + )?; + return Ok(ExitCode::SUCCESS); } // Normal trace output @@ -392,7 +397,7 @@ fn run_trace(args: TraceArgs, color: bool, sc: report::StderrColor) -> Result<() ); } - Ok(()) + Ok(ExitCode::SUCCESS) } /// Handle `trace --diff ` by comparing two entry points. @@ -728,4 +733,30 @@ mod tests { "expected flag name in error: {msg}" ); } + + /// Guard against process::exit in production code. process::exit bypasses + /// Drop, which skips joining the cache-write background thread and can + /// corrupt the on-disk cache. All exit codes flow through ExitCode instead. + #[test] + fn no_process_exit_in_production_code() { + let source = include_str!("main.rs"); + let production = source.split("#[cfg(test)]").next().unwrap_or(source); + let violations: Vec<_> = production + .lines() + .enumerate() + .filter(|(_, line)| { + let t = line.trim(); + !t.starts_with("//") && t.contains("process::exit") + }) + .collect(); + assert!( + violations.is_empty(), + "process::exit in production code (use ExitCode instead):\n{}", + violations + .iter() + .map(|(i, line)| format!(" line {}: {}", i + 1, line.trim())) + .collect::>() + .join("\n") + ); + } } From 31dc1450220fd30f347d8f38f3d466a416b1d1cd Mon Sep 17 00:00:00 2001 From: Harnoor Lal Date: Sun, 8 Mar 2026 20:07:38 -0700 Subject: [PATCH 2/2] docs(standards): remove process::exit exception from code standard The parenthetical "(except at the CLI boundary in main.rs)" contradicted the structural test no_process_exit_in_production_code in main.rs. process::exit bypasses Drop and can corrupt the cache, so there should be no exception. Align the standard with the test. --- docs/standards/code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/standards/code.md b/docs/standards/code.md index d06f1b1..894cfbe 100644 --- a/docs/standards/code.md +++ b/docs/standards/code.md @@ -4,7 +4,7 @@ - Edition 2024 - `#![warn(clippy::pedantic)]` with targeted `#[allow(...)]` where justified -- Idiomatic Rust: prefer stdlib patterns (iterators, windows, if-let chains, Result propagation with `?`) over manual indexing, `process::exit` (except at the CLI boundary in `main.rs`), or hand-rolled logic +- Idiomatic Rust: prefer stdlib patterns (iterators, windows, if-let chains, Result propagation with `?`) over manual indexing, `process::exit`, or hand-rolled logic - No bash scripts in the project: use `cargo xtask` for Rust logic, `Justfile` for external tool orchestration ## API Hygiene