diff --git a/Cargo.lock b/Cargo.lock index 77f6d3e..5921d5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -106,12 +106,6 @@ version = "3.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" -[[package]] -name = "bytes" -version = "1.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" - [[package]] name = "camino" version = "1.2.2" @@ -586,17 +580,6 @@ dependencies = [ "libc", ] -[[package]] -name = "mio" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50b7e5b27aa02a74bac8c3f23f448f8d87ff11f92d3aac1a6ed369ee08cc56c1" -dependencies = [ - "libc", - "wasi", - "windows-sys", -] - [[package]] name = "once_cell" version = "1.21.4" @@ -615,12 +598,6 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" -[[package]] -name = "pin-project-lite" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" - [[package]] name = "portable-atomic" version = "1.13.1" @@ -948,20 +925,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "tokio" -version = "1.51.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f66bf9585cda4b724d3e78ab34b73fb2bbaba9011b9bfdf69dc836382ea13b8c" -dependencies = [ - "bytes", - "libc", - "mio", - "pin-project-lite", - "signal-hook-registry", - "windows-sys", -] - [[package]] name = "twox-hash" version = "2.1.2" @@ -1043,12 +1006,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "wasi" -version = "0.11.1+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" - [[package]] name = "wasip2" version = "1.0.2+wasi-0.2.9" @@ -1389,7 +1346,6 @@ dependencies = [ "tempfile", "thread_local", "time-humanize", - "tokio", "twox-hash", ] diff --git a/Cargo.toml b/Cargo.toml index a211aa5..74d1f3a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,6 @@ cli = [ "tempfile", "thread_local", "time-humanize", - "tokio", "twox-hash", ] coverage = [] @@ -64,11 +63,6 @@ target-triple = { version = "1.0.0", optional = true } tempfile = { version = "3.27.0", optional = true } thread_local = { version = "1.1.9", optional = true } time-humanize = { version = "0.1.3", optional = true } -tokio = { version = "1.50.0", features = [ - "process", - "rt", - "time", -], optional = true } twox-hash = { version = "2.1.2", optional = true } [lints.clippy] diff --git a/src/bin/cargo-ziggy/main.rs b/src/bin/cargo-ziggy/main.rs index fe70b8a..790e82d 100644 --- a/src/bin/cargo-ziggy/main.rs +++ b/src/bin/cargo-ziggy/main.rs @@ -419,7 +419,6 @@ pub struct Common { terminate: Arc, sigs_done: Option<()>, pub cargo_path: PathBuf, - runtime: OnceLock, metadata: OnceLock>, } @@ -431,7 +430,6 @@ impl Common { cargo_path: std::env::var("CARGO") .unwrap_or_else(|_| String::from("cargo")) .into(), - runtime: OnceLock::new(), metadata: OnceLock::new(), } } @@ -471,15 +469,6 @@ impl Common { cmd } - fn async_runtime(&self) -> &tokio::runtime::Runtime { - self.runtime.get_or_init(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("Failed building tokio runtime") - }) - } - /// Cached `cargo metadata` fn metadata(&self) -> Option<&cargo_metadata::Metadata> { self.metadata diff --git a/src/bin/cargo-ziggy/run.rs b/src/bin/cargo-ziggy/run.rs index f8288a0..7761753 100644 --- a/src/bin/cargo-ziggy/run.rs +++ b/src/bin/cargo-ziggy/run.rs @@ -6,6 +6,8 @@ use std::{ env, fs, os::unix::process::ExitStatusExt, path::{Path, PathBuf}, + process, thread, + time::{Duration, Instant}, }; impl Run { @@ -100,10 +102,8 @@ impl Run { }; let runner = Runner::new( - common.async_runtime(), runner_path.as_std_path(), - self.timeout - .map(|s| tokio::time::Duration::from_secs(u64::from(s))), + self.timeout.map(|s| Duration::from_secs(u64::from(s))), ); for file in input_files { @@ -154,47 +154,48 @@ fn collect_dirs_recursively( } struct Runner<'a> { - rt: &'a tokio::runtime::Runtime, path: &'a Path, - timeout: Option, + timeout: Option, } impl<'a> Runner<'a> { - fn new( - rt: &'a tokio::runtime::Runtime, - path: &'a Path, - timeout: Option, - ) -> Self { - Self { rt, path, timeout } + fn new(path: &'a Path, timeout: Option) -> Self { + Self { path, timeout } } fn run(&self, seed: &Path) -> Status { - self.rt.block_on(async { - let mut child = match tokio::process::Command::new(self.path) - .arg(seed) - .env("RUST_BACKTRACE", "full") - .spawn() - .context("⚠️ couldn't spawn the runner process") - { - Ok(child) => child, - Err(e) => return e.into(), - }; - let res = if let Some(duration) = self.timeout { - if let Ok(res) = tokio::time::timeout(duration, child.wait()).await { - res - } else { - let _ = child.start_kill(); - return Status::Timeout; + let mut child = match process::Command::new(self.path) + .arg(seed) + .env("RUST_BACKTRACE", "full") + .spawn() + .context("⚠️ couldn't spawn the runner process") + { + Ok(child) => child, + Err(e) => return e.into(), + }; + let res = match self.timeout { + Some(duration) => { + let start = Instant::now(); + loop { + match child.try_wait() { + Ok(Some(status)) => break Ok(status), + Ok(None) if start.elapsed() >= duration => { + let _ = child.kill(); + let _ = child.wait(); + return Status::Timeout; + } + Ok(None) => thread::sleep(Duration::from_millis(10)), + Err(e) => break Err(e), + } } - } else { - child.wait().await } - .context("⚠️ couldn't wait for the runner process"); - match res { - Ok(status) => Status::Ok(status), - Err(e) => e.into(), - } - }) + None => child.wait(), + } + .context("⚠️ couldn't wait for the runner process"); + match res { + Ok(status) => Status::Ok(status), + Err(e) => e.into(), + } } } @@ -209,3 +210,132 @@ impl From for Status { Self::Err(err) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::{io::Write, os::unix::fs::PermissionsExt}; + + /// Write a `/bin/sh` script with `body` into `dir`, mark it executable, and + /// return its path. Used as a stand-in for the compiled runner binary. + fn executable_script(dir: &Path, name: &str, body: &str) -> PathBuf { + let path = dir.join(name); + let mut file = fs::File::create(&path).unwrap(); + writeln!(file, "#!/bin/sh\n{body}").unwrap(); + drop(file); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap(); + path + } + + fn dummy_seed(dir: &Path) -> PathBuf { + let seed = dir.join("seed"); + fs::write(&seed, b"input").unwrap(); + seed + } + + fn label(status: &Status) -> &'static str { + match status { + Status::Ok(_) => "Ok", + Status::Timeout => "Timeout", + Status::Err(_) => "Err", + } + } + + /// A run that finishes well within the timeout reports its real exit status + /// instead of timing out. + #[test] + fn completes_within_timeout_reports_success() { + let dir = tempfile::tempdir().unwrap(); + let runner_bin = executable_script(dir.path(), "fast-runner", "exit 0"); + let seed = dummy_seed(dir.path()); + + let runner = Runner::new(&runner_bin, Some(Duration::from_secs(30))); + match runner.run(&seed) { + Status::Ok(status) => assert!(status.success()), + other => panic!( + "expected Status::Ok(success), got Status::{}", + label(&other) + ), + } + } + + /// A non-zero exit within the timeout is surfaced as a failing `Status::Ok`, + /// not misreported as a timeout. + #[test] + fn nonzero_exit_is_reported_not_timeout() { + let dir = tempfile::tempdir().unwrap(); + let runner_bin = executable_script(dir.path(), "failing-runner", "exit 3"); + let seed = dummy_seed(dir.path()); + + let runner = Runner::new(&runner_bin, Some(Duration::from_secs(30))); + match runner.run(&seed) { + Status::Ok(status) => { + assert!(!status.success()); + assert_eq!(status.code(), Some(3)); + } + other => panic!( + "expected Status::Ok(failure), got Status::{}", + label(&other) + ), + } + } + + /// A run that outlasts the timeout is killed and reported as a timeout, and + /// the call returns promptly rather than blocking for the full run. + #[test] + fn exceeding_timeout_is_killed_and_reported() { + let dir = tempfile::tempdir().unwrap(); + // `exec` so the shell is replaced by `sleep`, ensuring the process we + // spawn is the one we kill on timeout. + let runner_bin = executable_script(dir.path(), "slow-runner", "exec sleep 30"); + let seed = dummy_seed(dir.path()); + + let runner = Runner::new(&runner_bin, Some(Duration::from_millis(100))); + let start = Instant::now(); + let status = runner.run(&seed); + let elapsed = start.elapsed(); + + assert!( + matches!(status, Status::Timeout), + "expected Status::Timeout, got Status::{}", + label(&status), + ); + // If the child were waited on rather than killed, this would take ~30s. + assert!( + elapsed < Duration::from_secs(10), + "timeout should return promptly after killing the child, took {elapsed:?}", + ); + } + + /// Without a timeout the runner simply waits for the process to finish. + #[test] + fn no_timeout_waits_for_completion() { + let dir = tempfile::tempdir().unwrap(); + let runner_bin = executable_script(dir.path(), "no-timeout-runner", "exit 0"); + let seed = dummy_seed(dir.path()); + + let runner = Runner::new(&runner_bin, None); + match runner.run(&seed) { + Status::Ok(status) => assert!(status.success()), + other => panic!( + "expected Status::Ok(success), got Status::{}", + label(&other) + ), + } + } + + /// A runner binary that cannot be spawned yields an error rather than a + /// timeout or a phantom success. + #[test] + fn spawn_failure_is_reported_as_error() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("does-not-exist"); + let seed = dummy_seed(dir.path()); + + let runner = Runner::new(&missing, Some(Duration::from_secs(30))); + match runner.run(&seed) { + Status::Err(_) => {} + other => panic!("expected Status::Err, got Status::{}", label(&other)), + } + } +}