From 4eb59a689fdcf79a380db7a087eecb9229469ecc Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Sat, 2 Apr 2022 13:33:13 -0400
Subject: [PATCH] guard process creation with a mutex (#25)

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     |  4 ++++
 src/task.rs    | 19 +++++++++++++++++--
 tests/basic.rs |  5 ++++-
 5 files changed, 33 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 22ef44c..5b6e4cf 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 0dab6d6..8eae02b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -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;
 
diff --git a/src/task.rs b/src/task.rs
index d2ce7c0..f42b377 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<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);
diff --git a/tests/basic.rs b/tests/basic.rs
index 07359d8..4dc1b91 100644
--- a/tests/basic.rs
+++ b/tests/basic.rs
@@ -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
+        );
     }
 }