Skip to content

Commit

Permalink
guard process creation with a mutex (#25)
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 #14.
  • Loading branch information
nico authored Apr 2, 2022
1 parent 5aa3a46 commit 4eb59a6
Show file tree
Hide file tree
Showing 5 changed files with 33 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
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ mod task;
pub mod trace;
pub mod work;

#[cfg(unix)]
#[macro_use]
extern crate lazy_static;

#[cfg(not(windows))]
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
5 changes: 4 additions & 1 deletion tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ 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 4eb59a6

Please sign in to comment.