From b9c6eb309c6cf63c9423db8c7d668db24bea79e5 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Sat, 24 Jun 2017 17:59:41 -0700
Subject: [PATCH] process: Add `status_async2` as a closer analog to
 `spawn_async`

---
 src/lib.rs     | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/stdio.rs | 19 +++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/src/lib.rs b/src/lib.rs
index 2b91ca69b..90c25a555 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -186,8 +186,27 @@ pub trait CommandExt {
     ///
     /// If the `StatusAsync` future is dropped before the future resolves, then
     /// the child will be killed, if it was spawned.
+    #[deprecated(note = "use the more flexible `spawn_async2` method instead")]
+    #[allow(deprecated)]
     fn status_async(&mut self, handle: &Handle) -> StatusAsync;
 
+    /// Executes a command as a child process, waiting for it to finish and
+    /// collecting its exit status.
+    ///
+    /// By default, stdin, stdout and stderr are inherited from the parent.
+    ///
+    /// The `StatusAsync` future returned will resolve to the `ExitStatus`
+    /// type in the standard library representing how the process exited. If
+    /// 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 `StatusAsync` future is dropped before the future resolves, then
+    /// the child will be killed, if it was spawned.
+    fn status_async2(&mut self, handle: &Handle) -> io::Result<StatusAsync2>;
+
     /// Executes the command as a child process, waiting for it to finish and
     /// collecting all of its output.
     ///
@@ -233,6 +252,7 @@ impl CommandExt for process::Command {
         Ok(child)
     }
 
+    #[allow(deprecated)]
     fn status_async(&mut self, handle: &Handle) -> StatusAsync {
         let mut inner = self.spawn_async(handle);
         if let Ok(child) = inner.as_mut() {
@@ -249,6 +269,21 @@ impl CommandExt for process::Command {
         }
     }
 
+    fn status_async2(&mut self, handle: &Handle) -> io::Result<StatusAsync2> {
+        self.spawn_async(handle).map(|mut child| {
+            // 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();
+
+            StatusAsync2 {
+                inner: child,
+            }
+        })
+    }
+
     fn output_async(&mut self, handle: &Handle) -> OutputAsync {
         self.stdout(Stdio::piped());
         self.stderr(Stdio::piped());
@@ -411,11 +446,14 @@ impl Future for WaitWithOutput {
 /// exit status. This future will resolves to the `ExitStatus` type in the
 /// standard library.
 #[must_use = "futures do nothing unless polled"]
+#[deprecated(note = "use the more flexible `SpawnAsync2` adapter instead")]
+#[allow(deprecated)]
 #[derive(Debug)]
 pub struct StatusAsync {
     inner: Flatten<FutureResult<Child, io::Error>>,
 }
 
+#[allow(deprecated)]
 impl Future for StatusAsync {
     type Item = ExitStatus;
     type Error = io::Error;
@@ -425,6 +463,26 @@ impl Future for StatusAsync {
     }
 }
 
+/// Future returned by the `CommandExt::status_async2` method.
+///
+/// This future is used to conveniently spawn a child and simply wait for its
+/// exit status. This future will resolves to the `ExitStatus` type in the
+/// standard library.
+#[must_use = "futures do nothing unless polled"]
+#[derive(Debug)]
+pub struct StatusAsync2 {
+    inner: Child,
+}
+
+impl Future for StatusAsync2 {
+    type Item = ExitStatus;
+    type Error = io::Error;
+
+    fn poll(&mut self) -> Poll<ExitStatus, io::Error> {
+        self.inner.poll()
+    }
+}
+
 /// Future returned by the `CommandExt::output_async` method.
 ///
 /// This future is mostly equivalent to spawning a process and then calling
diff --git a/tests/stdio.rs b/tests/stdio.rs
index dfef91991..79177131c 100644
--- a/tests/stdio.rs
+++ b/tests/stdio.rs
@@ -119,6 +119,7 @@ fn wait_with_output_captures() {
     assert_eq!(output.stderr.len(), 0);
 }
 
+#[allow(deprecated)]
 #[test]
 fn status_closes_any_pipes() {
     let mut core = Core::new().unwrap();
@@ -136,3 +137,21 @@ fn status_closes_any_pipes() {
         Err(_) => panic!("failed to run futures"),
     }
 }
+
+#[test]
+fn status2_closes_any_pipes() {
+    let mut core = Core::new().unwrap();
+
+    // Cat will open a pipe between the parent and child.
+    // If `status_async2` doesn't ensure the handles are closed,
+    // we would end up blocking forever (and time out).
+    let child = cat().status_async2(&core.handle()).unwrap();
+    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"),
+    }
+}