api2/node/termproxy: fix zombies on worker abort

tokios kill_on_drop sometimes leaves zombies around, especially
when there is not another tokio::process::Command spawned after

so instead of relying on the 'kill_on_drop' feature, we explicitly
kill the child on a worker abort. to be able to do this
we have to use 'tokio::select' instead of 'futures::select' since
the latter requires the future to be fused, which consumes the
child handle, leaving us no possibility to kill it after fusing.
(tokio::select does not need the futures to be fused, so we
can reuse the child future after the select again)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
This commit is contained in:
Dominik Csapak 2020-07-29 13:50:27 +02:00 committed by Wolfgang Bumiller
parent adfdc36936
commit d2b0c78e23

View File

@ -2,10 +2,7 @@ use std::net::TcpListener;
use std::os::unix::io::AsRawFd;
use anyhow::{bail, format_err, Error};
use futures::{
future::{FutureExt, TryFutureExt},
select,
};
use futures::future::{FutureExt, TryFutureExt};
use hyper::body::Body;
use hyper::http::request::Parts;
use hyper::upgrade::Upgraded;
@ -172,7 +169,6 @@ async fn termproxy(
let mut cmd = tokio::process::Command::new("/usr/bin/termproxy");
cmd.args(&arguments)
.kill_on_drop(true)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped());
@ -199,8 +195,9 @@ async fn termproxy(
Ok::<(), Error>(())
};
select!{
res = child.fuse() => {
let mut needs_kill = false;
let res = tokio::select!{
res = &mut child => {
let exit_code = res?;
if !exit_code.success() {
match exit_code.code() {
@ -210,10 +207,29 @@ async fn termproxy(
}
Ok(())
},
res = stdout_fut.fuse() => res,
res = stderr_fut.fuse() => res,
res = worker.abort_future().fuse() => res.map_err(Error::from),
res = stdout_fut => res,
res = stderr_fut => res,
res = worker.abort_future() => {
needs_kill = true;
res.map_err(Error::from)
}
};
if needs_kill {
if res.is_ok() {
child.kill()?;
child.await?;
return Ok(());
}
if let Err(err) = child.kill() {
worker.warn(format!("error killing termproxy: {}", err));
} else if let Err(err) = child.await {
worker.warn(format!("error awaiting termproxy: {}", err));
}
}
res
},
)?;