Skip to content

Commit

Permalink
process: kill child on error while spawning
Browse files Browse the repository at this point in the history
There's a chance that between spawning of the `std Child`, and wrapping
it into a tokio impl an error can occurs, which would leave the child
alive, with no way to access it from the user side and not being part
of the tokio runtime.

This commit fixes that issue by wrapping the `std Child` right after
spawning with a struct that implements Drop, where the child would
be killed if it's dropped.

Fixes: tokio-rs#6797
  • Loading branch information
Maki325 committed Aug 27, 2024
1 parent 479a56a commit b68da52
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 14 deletions.
92 changes: 83 additions & 9 deletions tokio/src/process/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl OrphanQueue<StdChild> for GlobalOrphanQueue {
pub(crate) enum Child {
SignalReaper(Reaper<StdChild, GlobalOrphanQueue, Signal>),
#[cfg(all(target_os = "linux", feature = "rt"))]
PidfdReaper(pidfd_reaper::PidfdReaper<StdChild, GlobalOrphanQueue>),
PidfdReaper(pidfd_reaper::PidfdReaper<ChildDropGuard, GlobalOrphanQueue>),
}

impl fmt::Debug for Child {
Expand All @@ -115,21 +115,95 @@ impl fmt::Debug for Child {
}
}

pub(crate) struct ChildDropGuard {
child: Option<StdChild>,
kill_on_drop: bool,
}

impl Drop for ChildDropGuard {
fn drop(&mut self) {
if let Some(child) = &mut self.child {
if self.kill_on_drop {
drop(child.kill());
}
}
}
}

impl ChildDropGuard {
pub(crate) fn new(child: StdChild) -> ChildDropGuard {
return ChildDropGuard {
child: Some(child),
kill_on_drop: true,
};
}

pub(crate) fn extract(mut self) -> StdChild {
self.child.take().unwrap()
}

pub(crate) fn dont_kill_on_drop(&mut self) {
self.kill_on_drop = false;
}

pub(crate) fn take_stdin(&mut self) -> Option<std::process::ChildStdin> {
self.child
.as_mut()
.expect("child has gone away")
.stdin
.take()
}
pub(crate) fn take_stdout(&mut self) -> Option<std::process::ChildStdout> {
self.child
.as_mut()
.expect("child has gone away")
.stdout
.take()
}
pub(crate) fn take_stderr(&mut self) -> Option<std::process::ChildStderr> {
self.child
.as_mut()
.expect("child has gone away")
.stderr
.take()
}

pub(crate) fn inner_mut(&mut self) -> &mut StdChild {
self.child.as_mut().expect("child has gone away")
}
}

impl Wait for ChildDropGuard {
fn id(&self) -> u32 {
self.child.as_ref().expect("child has gone away").id()
}
fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
self.child.as_mut().expect("child has gone away").try_wait()
}
}

impl OrphanQueue<ChildDropGuard> for GlobalOrphanQueue {
fn push_orphan(&self, orphan: ChildDropGuard) {
get_orphan_queue().push_orphan(orphan.extract());
}
}

pub(crate) fn spawn_child(cmd: &mut std::process::Command) -> io::Result<SpawnedChild> {
let mut child = cmd.spawn()?;
let stdin = child.stdin.take().map(stdio).transpose()?;
let stdout = child.stdout.take().map(stdio).transpose()?;
let stderr = child.stderr.take().map(stdio).transpose()?;
let mut child = ChildDropGuard::new(cmd.spawn()?);
let stdin = child.take_stdin().map(stdio).transpose()?;
let stdout = child.take_stdout().map(stdio).transpose()?;
let stderr = child.take_stderr().map(stdio).transpose()?;

#[cfg(all(target_os = "linux", feature = "rt"))]
match pidfd_reaper::PidfdReaper::new(child, GlobalOrphanQueue) {
Ok(pidfd_reaper) => {
Ok(mut pidfd_reaper) => {
pidfd_reaper.inner_mut().dont_kill_on_drop();
return Ok(SpawnedChild {
child: Child::PidfdReaper(pidfd_reaper),
stdin,
stdout,
stderr,
})
});
}
Err((Some(err), _child)) => return Err(err),
Err((None, child_returned)) => child = child_returned,
Expand All @@ -138,7 +212,7 @@ pub(crate) fn spawn_child(cmd: &mut std::process::Command) -> io::Result<Spawned
let signal = signal(SignalKind::child())?;

Ok(SpawnedChild {
child: Child::SignalReaper(Reaper::new(child, GlobalOrphanQueue, signal)),
child: Child::SignalReaper(Reaper::new(child.extract(), GlobalOrphanQueue, signal)),
stdin,
stdout,
stderr,
Expand All @@ -158,7 +232,7 @@ impl Child {
match self {
Self::SignalReaper(signal_reaper) => signal_reaper.inner_mut(),
#[cfg(all(target_os = "linux", feature = "rt"))]
Self::PidfdReaper(pidfd_reaper) => pidfd_reaper.inner_mut(),
Self::PidfdReaper(pidfd_reaper) => pidfd_reaper.inner_mut().inner_mut(),
}
}

Expand Down
54 changes: 49 additions & 5 deletions tokio/src/process/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,59 @@ struct Waiting {
unsafe impl Sync for Waiting {}
unsafe impl Send for Waiting {}

struct DroppableChild {
child: Option<StdChild>,
}

impl Drop for DroppableChild {
fn drop(&mut self) {
if let Some(child) = &mut self.child {
drop(child.kill());
}
}
}

impl DroppableChild {
pub(crate) fn new(child: StdChild) -> DroppableChild {
return DroppableChild { child: Some(child) };
}

pub(crate) fn take_stdin(&mut self) -> Option<std::process::ChildStdin> {
self.child
.as_mut()
.expect("child has gone away")
.stdin
.take()
}
pub(crate) fn take_stdout(&mut self) -> Option<std::process::ChildStdout> {
self.child
.as_mut()
.expect("child has gone away")
.stdout
.take()
}
pub(crate) fn take_stderr(&mut self) -> Option<std::process::ChildStderr> {
self.child
.as_mut()
.expect("child has gone away")
.stderr
.take()
}

pub(crate) fn take(mut self) -> StdChild {
self.child.take().expect("child has gone away")
}
}

pub(crate) fn spawn_child(cmd: &mut StdCommand) -> io::Result<SpawnedChild> {
let mut child = cmd.spawn()?;
let stdin = child.stdin.take().map(stdio).transpose()?;
let stdout = child.stdout.take().map(stdio).transpose()?;
let stderr = child.stderr.take().map(stdio).transpose()?;
let mut child = DroppableChild::new(cmd.spawn()?);
let stdin = child.take_stdin().map(stdio).transpose()?;
let stdout = child.take_stdout().map(stdio).transpose()?;
let stderr = child.take_stderr().map(stdio).transpose()?;

Ok(SpawnedChild {
child: Child {
child,
child: child.take(),
waiting: None,
},
stdin,
Expand Down

0 comments on commit b68da52

Please sign in to comment.