mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
fix: clean up MCP subprocesses after abrupt parent exit (#8242)
Signed-off-by: fre <anonwurcod@proton.me> Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
Generated
+3
-2
@@ -4369,6 +4369,7 @@ dependencies = [
|
||||
"jsonwebtoken",
|
||||
"keyring",
|
||||
"lazy_static",
|
||||
"libc",
|
||||
"llama-cpp-2",
|
||||
"lru",
|
||||
"minijinja",
|
||||
@@ -5746,9 +5747,9 @@ checksum = "7a79a3332a6609480d7d0c9eab957bca6b455b91bb84e66d19f5ff66294b85b8"
|
||||
|
||||
[[package]]
|
||||
name = "libc"
|
||||
version = "0.2.183"
|
||||
version = "0.2.184"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d"
|
||||
checksum = "48f5d2a454e16a5ea0f4ced81bd44e4cfc7bd3a507b61887c99fd3538b28e4af"
|
||||
|
||||
[[package]]
|
||||
name = "libdbus-sys"
|
||||
|
||||
@@ -188,6 +188,7 @@ pkcs1 = { version = "0.7", default-features = false, features = ["pkcs8"], optio
|
||||
pkcs8 = { version = "0.10", default-features = false, features = ["alloc"], optional = true }
|
||||
sec1 = { version = "0.7", default-features = false, features = ["der", "pkcs8"], optional = true }
|
||||
|
||||
|
||||
[target.'cfg(target_os = "windows")'.dependencies]
|
||||
winapi = { workspace = true }
|
||||
keyring = { version = "3.6.2", features = ["windows-native"] }
|
||||
@@ -201,6 +202,7 @@ keyring = { version = "3.6.2", features = ["apple-native"] }
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
keyring = { version = "3.6.2", features = ["sync-secret-service"] }
|
||||
libc = "0.2.184"
|
||||
|
||||
[dev-dependencies]
|
||||
serial_test = { workspace = true }
|
||||
|
||||
@@ -3,6 +3,25 @@ use tokio::process::Command;
|
||||
#[cfg(windows)]
|
||||
const CREATE_NO_WINDOW_FLAG: u32 = 0x08000000;
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn configure_parent_death_signal(command: &mut Command) {
|
||||
let parent_pid = unsafe { libc::getpid() };
|
||||
|
||||
unsafe {
|
||||
command.pre_exec(move || {
|
||||
if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) != 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
|
||||
if libc::getppid() != parent_pid {
|
||||
return Err(std::io::Error::from_raw_os_error(libc::ESRCH));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
pub trait SubprocessExt {
|
||||
fn set_no_window(&mut self) -> &mut Self;
|
||||
}
|
||||
@@ -34,5 +53,7 @@ pub fn configure_subprocess(command: &mut Command) {
|
||||
// SIGINT when the user presses Ctrl+C in the terminal.
|
||||
#[cfg(unix)]
|
||||
command.process_group(0);
|
||||
#[cfg(target_os = "linux")]
|
||||
configure_parent_death_signal(command);
|
||||
command.set_no_window();
|
||||
}
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
#![cfg(target_os = "linux")]
|
||||
|
||||
use goose::subprocess::configure_subprocess;
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::path::PathBuf;
|
||||
use std::process::{Command, Stdio};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
const HELPER_ENV: &str = "GOOSE_SUBPROCESS_PARENT_DEATH_HELPER";
|
||||
|
||||
#[ctor::ctor]
|
||||
fn maybe_run_helper() {
|
||||
if std::env::var_os(HELPER_ENV).is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.expect("runtime");
|
||||
|
||||
let pid = runtime.block_on(async {
|
||||
let mut command = tokio::process::Command::new("sleep");
|
||||
command.arg("30");
|
||||
command.stdin(Stdio::null());
|
||||
command.stdout(Stdio::null());
|
||||
command.stderr(Stdio::null());
|
||||
configure_subprocess(&mut command);
|
||||
|
||||
let child = command.spawn().expect("spawn child");
|
||||
let pid = child.id().expect("child pid");
|
||||
std::mem::forget(child);
|
||||
pid
|
||||
});
|
||||
|
||||
println!("{pid}");
|
||||
std::io::stdout().flush().expect("flush pid");
|
||||
|
||||
unsafe {
|
||||
libc::_exit(0);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_process_exits_when_parent_process_dies() {
|
||||
let current_exe = std::env::current_exe().expect("current test binary");
|
||||
let mut helper = Command::new(current_exe)
|
||||
.env(HELPER_ENV, "1")
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::inherit())
|
||||
.spawn()
|
||||
.expect("spawn helper");
|
||||
|
||||
let pid_line = {
|
||||
let stdout = helper.stdout.take().expect("helper stdout");
|
||||
let mut reader = BufReader::new(stdout);
|
||||
let mut line = String::new();
|
||||
reader.read_line(&mut line).expect("read child pid");
|
||||
line
|
||||
};
|
||||
|
||||
let child_pid = pid_line.trim().parse::<u32>().expect("parse child pid");
|
||||
|
||||
let status = helper.wait().expect("wait for helper");
|
||||
assert!(status.success(), "helper exited unsuccessfully: {status}");
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while process_exists(child_pid) {
|
||||
assert!(
|
||||
Instant::now() < deadline,
|
||||
"child process {child_pid} still exists after helper exit"
|
||||
);
|
||||
std::thread::sleep(Duration::from_millis(100));
|
||||
}
|
||||
}
|
||||
|
||||
fn process_exists(pid: u32) -> bool {
|
||||
PathBuf::from(format!("/proc/{pid}")).exists()
|
||||
}
|
||||
Reference in New Issue
Block a user