Revert "mcp(win): full Windows Job Object cleanup wiring (patch draft… (#10013)

This commit is contained in:
Jack Amadeo
2026-06-25 09:59:12 -07:00
committed by GitHub
parent bb4a9a7920
commit 8ab0320a7b
7 changed files with 2 additions and 140 deletions
-2
View File
@@ -19,8 +19,6 @@ mod memory;
pub mod peekaboo;
pub mod subprocess;
pub mod tutorial;
#[cfg(windows)]
pub mod windows_job;
pub use autovisualiser::AutoVisualiserRouter;
pub use computercontroller::ComputerControllerServer;
+1 -1
View File
@@ -64,7 +64,7 @@ fn resolve_login_shell_path() -> Option<String> {
.stdout(Stdio::piped())
.stderr(Stdio::null());
// Spawn in a new session so that interactive shells job-control setup
// Spawn in a new session so that interactive shell job-control setup
// cannot steal the terminal foreground from the parent goose process.
cmd.wrap(ProcessSession);
-22
View File
@@ -1,22 +0,0 @@
// Stub Windows Job Object helpers for MCP path (no-ops on Windows).
// This file provides the minimal surface required by crates/goose-mcp/src/subprocess.rs
// when built on Windows. The real Windows Job Object integration lives in the goose crate.
#[cfg(windows)]
pub type HANDLE = *mut std::ffi::c_void;
#[cfg(windows)]
pub fn ensure_job_object() -> Option<HANDLE> {
None
}
#[cfg(windows)]
pub fn attach_pid_to_job(_pid: u32) {}
#[cfg(windows)]
pub fn init_windows_cleanup() {}
#[cfg(windows)]
pub fn windows_cleanup_enabled() -> bool {
false
}
+1 -1
View File
@@ -220,7 +220,7 @@ subtle = { version = "2.5", default-features = false, features = ["std"] }
gethostname = "1.1.0"
[target.'cfg(target_os = "windows")'.dependencies]
winapi = { version = "0.3.9", default-features = false, features = ["wincred", "std", "jobapi2", "winbase", "winnt", "processthreadsapi", "handleapi", "minwindef"] }
winapi = { workspace = true }
keyring = { workspace = true, features = ["windows-native"], optional = true }
# Platform-specific GPU acceleration for Whisper and local inference
@@ -400,15 +400,6 @@ async fn child_process_client(
let (transport, mut stderr) = TokioChildProcess::builder(command)
.stderr(Stdio::piped())
.spawn()?;
// Attach the child to a Windows Job Object to ensure proper cleanup on Goose exit
#[cfg(windows)]
{
if let Some(pid) = transport.id() {
// Initialize Job Object and attach the child process to it
crate::windows_job::init_windows_cleanup();
crate::windows_job::attach_pid_to_job(pid);
}
}
let mut stderr = stderr.take().ok_or_else(|| {
ExtensionError::SetupError("failed to attach child process stderr".to_owned())
})?;
-3
View File
@@ -46,11 +46,8 @@ pub mod slash_commands;
pub mod source_roots;
pub mod sources;
pub mod subprocess;
pub mod token_counter;
pub mod tool_inspection;
pub mod tool_monitor;
pub mod tracing;
pub mod utils;
#[cfg(windows)]
pub mod windows_job;
-102
View File
@@ -1,102 +0,0 @@
// Windows Job Object cleanup for child processes (Goose Windows support).
//
// Attaches spawned MCP subprocesses to a Job Object configured with
// JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. When the Goose process exits (and the
// job handle is closed by the OS), Windows terminates every process in the
// job, preventing orphaned child processes. This is the Windows analog of the
// Linux PR_SET_PDEATHSIG behavior in subprocess.rs.
#![allow(dead_code)]
#[cfg(windows)]
mod windows_impl {
use std::mem::{size_of, zeroed};
use std::ptr::null_mut;
use std::sync::atomic::{AtomicUsize, Ordering};
use winapi::shared::minwindef::FALSE;
use winapi::um::handleapi::CloseHandle;
use winapi::um::jobapi2::{AssignProcessToJobObject, CreateJobObjectW};
use winapi::um::processthreadsapi::OpenProcess;
use winapi::um::winbase::SetInformationJobObject;
use winapi::um::winnt::{
JobObjectExtendedLimitInformation, HANDLE, JOBOBJECT_EXTENDED_LIMIT_INFORMATION,
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, PROCESS_SET_QUOTA, PROCESS_TERMINATE,
};
// HANDLE (*mut c_void) is not Send/Sync, so we store the handle as a usize
// and cast back to HANDLE at use sites. 0 means "not yet created".
static JOB_HANDLE: AtomicUsize = AtomicUsize::new(0);
pub fn ensure_job_object() -> Option<HANDLE> {
let existing = JOB_HANDLE.load(Ordering::Acquire);
if existing != 0 {
return Some(existing as HANDLE);
}
unsafe {
let job = CreateJobObjectW(null_mut(), null_mut());
if job.is_null() {
return None;
}
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = zeroed();
info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
let set_res = SetInformationJobObject(
job,
JobObjectExtendedLimitInformation,
&mut info as *mut _ as *mut _,
size_of::<JOBOBJECT_EXTENDED_LIMIT_INFORMATION>() as u32,
);
if set_res == FALSE {
// If we fail to configure the job object to terminate on close, do not publish
// this handle. Cleaning up here avoids mutating global state with a partially
// configured Job Object.
CloseHandle(job);
return None;
}
// Publish the handle, but if another thread won the race, close ours.
match JOB_HANDLE.compare_exchange(0, job as usize, Ordering::AcqRel, Ordering::Acquire)
{
Ok(_) => Some(job),
Err(winner) => {
CloseHandle(job);
Some(winner as HANDLE)
}
}
}
}
pub fn attach_pid_to_job(pid: u32) {
let job = match ensure_job_object() {
Some(job) => job,
None => return,
};
unsafe {
// AssignProcessToJobObject requires PROCESS_SET_QUOTA in addition to
// PROCESS_TERMINATE; without it the assignment fails and the child is
// never tied to the job, leaving it orphaned on exit.
let proc = OpenProcess(PROCESS_TERMINATE | PROCESS_SET_QUOTA, FALSE, pid);
if !proc.is_null() {
if AssignProcessToJobObject(job, proc) == FALSE {
tracing::warn!(pid, "failed to assign child process to Windows job object");
}
CloseHandle(proc);
}
}
}
pub fn init_windows_cleanup() {
let _ = ensure_job_object();
}
pub fn windows_cleanup_enabled() -> bool {
JOB_HANDLE.load(Ordering::Acquire) != 0
}
}
#[cfg(windows)]
pub use windows_impl::{
attach_pid_to_job, ensure_job_object, init_windows_cleanup, windows_cleanup_enabled,
};