Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix process group behavior on Windows #51

Merged
merged 1 commit into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 16 additions & 78 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ zip = "2.1"

async-once-cell = "0.5"
async-signal = "0.2"
command-group = { version = "5.0", features = ["with-tokio"] }
futures = "0.3"
process-wrap = { version = "8.0", features = ["tokio1"] }
reqwest = { version = "0.12", default-features = false, features = [
"rustls-tls",
"http2",
Expand Down
35 changes: 17 additions & 18 deletions lib/system/runner.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
#![allow(clippy::wildcard_imports)]

use std::ffi::OsStr;
use std::io::Result as IoResult;

use async_signal::{Signal, Signals};
use command_group::AsyncCommandGroup;
use futures::StreamExt;
use process_wrap::tokio::*;
use tokio::{
process::Command,
task::{spawn, JoinHandle},
Expand Down Expand Up @@ -71,34 +69,35 @@ where
let signal_handle = spawn_signal_listener_task()?;
let signal_aborter = signal_handle.abort_handle();

// Important - we do not want to leave any zombie
// processes behind if this async function is cancelled.
// We'll use Tokio's kill on drop functionality, as well
// as wrappers for process / job groups for each platform.
let mut command = Command::new(command);
command.args(args);
let mut wrapper = TokioCommandWrap::from(command);
wrapper.wrap(KillOnDrop);
/*
Important - we do not want to leave any zombie
processes behind if this async function is cancelled.

#[cfg(windows)]
{
wrapper.wrap(JobObject);
}
Note that since we also want to spawn the child process as part
of the current process group, we have to use the builder API from
`command-group` to spawn the child process, or this won't work.

let mut child_handle = wrapper.spawn()?;
The newer `process-wrap` crate claims to also support this behavior
for inheriting process group but it doesn't seem to work as expected.
*/
let mut child_handle = Command::new(command)
.args(args)
.group()
.kill_on_drop(true)
.spawn()?;

let code = tokio::select! {
// If the spawned process exits cleanly, we'll return its exit code,
// which may or may not exist. Interpret a non-existent code as 1.
command_result = Box::into_pin(child_handle.wait()) => {
command_result = child_handle.wait() => {
let code = command_result.ok().and_then(|s| s.code()).unwrap_or(1);
signal_aborter.abort();
code
}
// If the command was manually interrupted by a signal, we will
// return a special exit code for the signal. More details above.
task_result = signal_handle => {
Box::into_pin(child_handle.kill()).await.ok();
child_handle.kill().await.ok();
task_result.unwrap_or(EXIT_CODE_GOT_SIGNAL)
}
};
Expand Down