From a01a87a05a09bcb3c4f90c2db1da66548dd35405 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 7 Sep 2020 10:59:56 -0700 Subject: [PATCH 1/2] process: make `Child::kill` async * 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..e1fb9dd6022 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); } From 816ef91b616506645f0bcc60565788ad1a957412 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 7 Sep 2020 12:25:03 -0700 Subject: [PATCH 2/2] Update tokio/src/process/mod.rs Co-authored-by: Alice Ryhl --- tokio/src/process/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio/src/process/mod.rs b/tokio/src/process/mod.rs index e1fb9dd6022..9c19c72ea97 100644 --- a/tokio/src/process/mod.rs +++ b/tokio/src/process/mod.rs @@ -779,7 +779,7 @@ impl Child { /// /// 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 + /// 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 {