Skip to content

Commit

Permalink
guard process creation with a mutex
Browse files Browse the repository at this point in the history
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 evmar#14.
  • Loading branch information
nico committed Apr 2, 2022
1 parent 871c660 commit 712e861
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 3 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 17 additions & 2 deletions src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -83,12 +86,24 @@ fn run_task(
Ok(result)
}

#[cfg(unix)]
lazy_static! {
static ref TASK_MUTEX: Mutex<i32> = Mutex::new(0);
}

#[cfg(unix)]
fn run_command(cmdline: &str) -> anyhow::Result<TaskResult> {
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);
Expand Down
2 changes: 1 addition & 1 deletion tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 712e861

Please sign in to comment.