Merge pull request #10 from alexmaco/kod

Fix Child::output to avoid killing the subprocess early when kill_on_drop is given

src/lib.rs

@@ -77,10 +77,11 @@ pub mod windows;
 /// An event delivered every time the SIGCHLD signal occurs.
 static SIGCHLD: Event = Event::new();
 
-/// A guard that pushes abandoned child processes into the zombie list.
+/// A guard that can kill child processes, or push them into the zombie list.
 struct ChildGuard {
     inner: Option<std::process::Child>,
     reap_on_drop: bool,
+    kill_on_drop: bool,
 }
 
 impl ChildGuard {
@@ -119,9 +120,6 @@ pub struct Child {
 
     /// The inner child process handle.
     child: Arc<Mutex<ChildGuard>>,
-
-    /// Whether to kill the process on drop.
-    kill_on_drop: bool,
 }
 
 impl Child {
@@ -243,6 +241,9 @@ impl Child {
         // When the last reference to the child process is dropped, push it into the zombie list.
         impl Drop for ChildGuard {
             fn drop(&mut self) {
+                if self.kill_on_drop {
+                    self.get_mut().kill().ok();
+                }
                 if self.reap_on_drop {
                     let mut zombies = ZOMBIES.lock().unwrap();
                     if let Ok(None) = self.get_mut().try_wait() {
@@ -259,8 +260,8 @@ impl Child {
             child: Arc::new(Mutex::new(ChildGuard {
                 inner: Some(child),
                 reap_on_drop: cmd.reap_on_drop,
+                kill_on_drop: cmd.kill_on_drop,
             })),
-            kill_on_drop: cmd.kill_on_drop,
         })
     }
 
@@ -422,14 +423,6 @@ impl Child {
     }
 }
 
-impl Drop for Child {
-    fn drop(&mut self) {
-        if self.kill_on_drop {
-            self.kill().ok();
-        }
-    }
-}
-
 impl fmt::Debug for Child {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Child")

tests/std.rs

@@ -366,3 +366,18 @@ fn test_capture_env_at_spawn() {
         );
     })
 }
+
+#[test]
+#[cfg(unix)]
+fn child_status_preserved_with_kill_on_drop() {
+    future::block_on(async {
+        let p = Command::new("true").kill_on_drop(true).spawn().unwrap();
+
+        // Calling output, since it takes ownership of the child
+        // Child::status would work, but without special care,
+        // dropping p inside of output would kill the subprocess early,
+        // and report the wrong exit status
+        let res = p.output().await;
+        assert!(res.unwrap().status.success());
+    })
+}