From 56d3914675a48e22f1cdad66c56b1a399629069c Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Sat, 24 Jun 2017 18:11:39 -0700
Subject: [PATCH] process: Bugfix: ensure `status_async` closes child's stdio
 handles after spawning

---
 src/lib.rs     | 18 ++++++++++++++----
 tests/stdio.rs | 21 ++++++++++++++++++++-
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index bcb835d2e..2b91ca69b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -178,13 +178,13 @@ pub trait CommandExt {
     ///
     /// The `StatusAsync` future returned will resolve to the `ExitStatus`
     /// type in the standard library representing how the process exited. If
-    /// output handles are set to a pipe then they will be immediately closed
-    /// after the child is spawned.
+    /// any input/output handles are set to a pipe then they will be immediately
+    /// closed after the child is spawned.
     ///
     /// The `handle` specified must be a handle to a valid event loop, and all
     /// I/O this child does will be associated with the specified event loop.
     ///
-    /// If the `OutputAsync` future is dropped before the future resolves, then
+    /// If the `StatusAsync` future is dropped before the future resolves, then
     /// the child will be killed, if it was spawned.
     fn status_async(&mut self, handle: &Handle) -> StatusAsync;
 
@@ -234,8 +234,18 @@ impl CommandExt for process::Command {
     }
 
     fn status_async(&mut self, handle: &Handle) -> StatusAsync {
+        let mut inner = self.spawn_async(handle);
+        if let Ok(child) = inner.as_mut() {
+            // Ensure we close any stdio handles so we can't deadlock
+            // waiting on the child which may be waiting to read/write
+            // to a pipe we're holding.
+            child.stdin.take();
+            child.stdout.take();
+            child.stderr.take();
+        }
+
         StatusAsync {
-            inner: self.spawn_async(handle).into_future().flatten(),
+            inner: inner.into_future().flatten(),
         }
     }
 
diff --git a/tests/stdio.rs b/tests/stdio.rs
index bd0cdee29..dfef91991 100644
--- a/tests/stdio.rs
+++ b/tests/stdio.rs
@@ -8,11 +8,12 @@ extern crate env_logger;
 
 use std::io;
 use std::process::{Stdio, ExitStatus, Command};
+use std::time::Duration;
 
 use futures::future::{BoxFuture, Future};
 use futures::stream::{self, Stream};
 use tokio_io::io::{read_until, write_all, read_to_end};
-use tokio_core::reactor::Core;
+use tokio_core::reactor::{Core, Timeout};
 use tokio_process::{CommandExt, Child};
 
 mod support;
@@ -117,3 +118,21 @@ fn wait_with_output_captures() {
     assert_eq!(output.stdout, written);
     assert_eq!(output.stderr.len(), 0);
 }
+
+#[test]
+fn status_closes_any_pipes() {
+    let mut core = Core::new().unwrap();
+
+    // Cat will open a pipe between the parent and child.
+    // If `status_async` doesn't ensure the handles are closed,
+    // we would end up blocking forever (and time out).
+    let child = cat().status_async(&core.handle());
+    let timeout = Timeout::new(Duration::from_secs(1), &core.handle())
+        .expect("timeout registration failed")
+        .map(|()| panic!("time out exceeded! did we get stuck waiting on the child?"));
+
+    match core.run(child.select(timeout)) {
+        Ok((status, _)) => assert!(status.success()),
+        Err(_) => panic!("failed to run futures"),
+    }
+}