From 712e8615a2dd75ae23a4251fcf9092ece00d2abf Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 1 Apr 2022 22:05:54 -0400 Subject: [PATCH] guard process creation with a mutex Fixes an fd leak on all unix systems that don't have pipe2. As of today, that's all unix systems that are not dragonfly bsd, freebsd, linux, netbsd, openbsd, or redox -- in particular, macOS. Fixes #14. --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/lib.rs | 3 +++ src/task.rs | 19 +++++++++++++++++-- tests/basic.rs | 2 +- 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a5ba22..d4e410e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -90,6 +90,12 @@ dependencies = [ "winapi-build", ] +[[package]] +name = "lazy_static" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" + [[package]] name = "libc" version = "0.2.112" @@ -104,6 +110,7 @@ dependencies = [ "getopts", "jemallocator", "kernel32-sys", + "lazy_static", "libc", "tempfile", "winapi 0.3.9", diff --git a/Cargo.toml b/Cargo.toml index 54c798a..952e149 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" getopts = "0.2" anyhow = "1.0" libc = "0.2" +lazy_static = "1.4.0" [target.'cfg(windows)'.dependencies] kernel32-sys = "0.2.2" diff --git a/src/lib.rs b/src/lib.rs index 561ab23..9e7a0bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,9 @@ mod task; pub mod trace; pub mod work; +#[macro_use] +extern crate lazy_static; + #[cfg(not(target_env = "msvc"))] use jemallocator::Jemalloc; diff --git a/src/task.rs b/src/task.rs index 68c6075..14cacd3 100644 --- a/src/task.rs +++ b/src/task.rs @@ -18,6 +18,9 @@ use std::io::Write; #[cfg(unix)] use std::os::unix::process::ExitStatusExt; +#[cfg(unix)] +use std::sync::Mutex; + #[cfg(windows)] extern crate winapi; @@ -83,12 +86,24 @@ fn run_task( Ok(result) } +#[cfg(unix)] +lazy_static! { + static ref TASK_MUTEX: Mutex = Mutex::new(0); +} + #[cfg(unix)] fn run_command(cmdline: &str) -> anyhow::Result { - let mut cmd = std::process::Command::new("/bin/sh") + // Command::spawn() can leak FSs when run concurrently, see #14. + let just_one = TASK_MUTEX.lock().unwrap(); + let p = std::process::Command::new("/bin/sh") .arg("-c") .arg(cmdline) - .output()?; + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn()?; + drop(just_one); + + let mut cmd = p.wait_with_output()?; let mut output = Vec::new(); output.append(&mut cmd.stdout); output.append(&mut cmd.stderr); diff --git a/tests/basic.rs b/tests/basic.rs index ca66c52..022aacc 100644 --- a/tests/basic.rs +++ b/tests/basic.rs @@ -26,7 +26,7 @@ fn print_output(out: &std::process::Output) { fn assert_output_contains(out: &std::process::Output, text: &str) { let out = std::str::from_utf8(&out.stdout).unwrap(); if !out.contains(text) { - panic!("assertion failed; expected output to contain {:?}", text); + panic!("assertion failed; expected output to contain {:?} but got {}", text, out); } }