From 7c254eca446e56bbc41cbc309c2588f2d241f46a Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Mon, 7 Sep 2020 23:03:25 -0700
Subject: [PATCH] process: make `Child::kill` async (#2823)

* This changes the `Child::kill` to be an async method which awaits the
  child after sending a kill signal. This avoids leaving zombie
  processes on Unix platforms if the caller forgets to await the child
  after the kill completes
* A `start_kill` method was also added on `Child` which only sends the
  kill signal to the child process. This allows for kill signals to be
  sent even outside of async contexts.
---
 tokio/src/process/mod.rs          | 36 ++++++++++++++++++++-----------
 tokio/tests/process_issue_2174.rs |  3 +--
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/tokio/src/process/mod.rs b/tokio/src/process/mod.rs
index 057755c9813..9c19c72ea97 100644
--- a/tokio/src/process/mod.rs
+++ b/tokio/src/process/mod.rs
@@ -774,6 +774,23 @@ impl Child {
         }
     }
 
+    /// Attempts to force the child to exit, but does not wait for the request
+    /// to take effect.
+    ///
+    /// On Unix platforms, this is the equivalent to sending a SIGKILL. Note
+    /// that on Unix platforms it is possible for a zombie process to remain
+    /// after a kill is sent; to avoid this, the caller should ensure that either
+    /// `child.wait().await` or `child.try_wait()` is invoked successfully.
+    pub fn start_kill(&mut self) -> io::Result<()> {
+        match &mut self.child {
+            FusedChild::Child(child) => child.kill(),
+            FusedChild::Done(_) => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "invalid argument: can't kill an exited process",
+            )),
+        }
+    }
+
     /// Forces the child to exit.
     ///
     /// This is equivalent to sending a SIGKILL on unix platforms.
@@ -795,21 +812,14 @@ impl Child {
     ///     tokio::spawn(async move { send.send(()) });
     ///     tokio::select! {
     ///         _ = child.wait() => {}
-    ///         _ = recv => {
-    ///             child.kill().expect("kill failed");
-    ///             // NB: await the child here to avoid a zombie process on Unix platforms
-    ///             child.wait().await.unwrap();
-    ///         }
+    ///         _ = recv => child.kill().await.expect("kill failed"),
     ///     }
     /// }
-    pub fn kill(&mut self) -> io::Result<()> {
-        match &mut self.child {
-            FusedChild::Child(child) => child.kill(),
-            FusedChild::Done(_) => Err(io::Error::new(
-                io::ErrorKind::InvalidInput,
-                "invalid argument: can't kill an exited process",
-            )),
-        }
+    /// ```
+    pub async fn kill(&mut self) -> io::Result<()> {
+        self.start_kill()?;
+        self.wait().await?;
+        Ok(())
     }
 
     /// Waits for the child to exit completely, returning the status that it
diff --git a/tokio/tests/process_issue_2174.rs b/tokio/tests/process_issue_2174.rs
index d12825f6ca7..4493d54ab1a 100644
--- a/tokio/tests/process_issue_2174.rs
+++ b/tokio/tests/process_issue_2174.rs
@@ -39,8 +39,7 @@ async fn issue_2174() {
     time::delay_for(Duration::from_secs(1)).await;
 
     // Kill the child process.
-    child.kill().unwrap();
-    let _ = child.wait().await;
+    child.kill().await.unwrap();
 
     assert_err!(handle.await);
 }