Allow Windows daemon launches with residual job membership (#48157)

## Why

A Windows breakaway launch can leave an outer Job Object attached after escaping an inner job. Job membership alone does not establish whether the daemon will be terminated, so rejecting all remaining membership can block valid launches.

## What changed

Remove the job-membership checks from the launch preflight and the spawned daemon. Keep the suspended breakaway probe to verify that launch is permitted before stopping an existing daemon, and terminate and reap the probe afterward.

## Testing

Add a Windows regression test with nested jobs that verifies the probe and daemon leave the inner job while retaining the outer job, and that daemon startup succeeds.

GitOrigin-RevId: 4c6aafd158300fa428ec398d72dc5d3411c0d8b8
This commit is contained in:
Eric Traut
2026-09-25 16:52:50 -07:00
committed by Ian MacLeod
parent d9275a63ed
commit c13e9478d7
3 changed files with 156 additions and 48 deletions
@@ -258,10 +258,6 @@ impl PidBackend {
.id()
.context("spawned app-server process has no pid")?;
let record = match async {
#[cfg(windows)]
super::super::windows::Process::open(pid)?
.context("daemon exited during launch")?
.ensure_detached()?;
let process_start_time = read_process_start_time(pid).await?;
#[cfg(any(target_os = "linux", target_os = "macos"))]
let process_identity = super::identity::read_process_details(pid)
@@ -28,7 +28,6 @@ use windows_sys::Win32::Storage::FileSystem::LOCKFILE_FAIL_IMMEDIATELY;
use windows_sys::Win32::Storage::FileSystem::LockFileEx;
use windows_sys::Win32::System::JobObjects::AssignProcessToJobObject;
use windows_sys::Win32::System::JobObjects::CreateJobObjectW;
use windows_sys::Win32::System::JobObjects::IsProcessInJob;
use windows_sys::Win32::System::JobObjects::JOB_OBJECT_LIMIT_BREAKAWAY_OK;
use windows_sys::Win32::System::JobObjects::JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
use windows_sys::Win32::System::JobObjects::JOBOBJECT_EXTENDED_LIMIT_INFORMATION;
@@ -77,8 +76,9 @@ pub(crate) fn ensure_not_elevated() -> Result<()> {
Ok(())
}
// Probe the actual child association: escaping an inner job can leave an outer
// job attached. Suspend the image so no application code runs before cleanup.
// Check that breakaway launch is permitted before stopping an existing daemon.
// An outer system job may remain attached; membership alone does not establish
// whether it will terminate the daemon. Suspend the probe before cleanup.
pub(crate) fn ensure_detached_launch(executable: &Path) -> Result<()> {
let mut child = Command::new(executable)
.creation_flags(CREATE_SUSPENDED | DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB)
@@ -87,30 +87,13 @@ pub(crate) fn ensure_detached_launch(executable: &Path) -> Result<()> {
.stderr(Stdio::null())
.spawn()
.context("cannot launch detached daemon; existing daemon was not stopped")?;
let mut in_job = 0;
let result = if unsafe {
IsProcessInJob(
child.as_raw_handle() as _,
/*jobhandle*/ 0,
&mut in_job,
)
} == 0
{
Err(io::Error::last_os_error()).context("failed to verify daemon launch capability")
} else if in_job != 0 {
Err(anyhow::anyhow!(
"host Job Object prevents daemon detachment; start from a host that allows breakaway"
))
} else {
Ok(())
};
child
.kill()
.context("failed to terminate suspended launch probe")?;
child
.wait()
.context("failed to reap suspended launch probe")?;
result
Ok(())
}
pub(super) struct Process(OwnedHandle);
@@ -170,29 +153,6 @@ impl Process {
}
}
pub(super) fn ensure_detached(&self) -> Result<()> {
let mut in_job = 0;
if unsafe {
IsProcessInJob(
self.0.as_raw_handle() as _,
/*jobhandle*/ 0,
&mut in_job,
)
} == 0
{
let error = io::Error::last_os_error();
self.terminate()?;
return Err(error).context("failed to verify daemon detachment");
}
if in_job != 0 {
self.terminate()?;
anyhow::bail!(
"host Job Object prevents daemon detachment; start from a host that allows breakaway"
);
}
Ok(())
}
pub(super) fn terminate(&self) -> Result<()> {
if !self.is_running()? {
return Ok(());
@@ -1,7 +1,11 @@
use super::Process;
use pretty_assertions::assert_eq;
use std::os::windows::io::AsRawHandle;
use std::os::windows::io::FromRawHandle;
use std::os::windows::io::OwnedHandle;
use std::os::windows::process::CommandExt;
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
use windows_sys::Win32::System::JobObjects::IsProcessInJob;
use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW;
use windows_sys::Win32::System::Threading::TerminateProcess;
@@ -45,6 +49,154 @@ fn detached_launch_preflight_rejects_restrictive_job() {
assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed"));
}
#[test]
fn detached_launch_preflight_allows_residual_job() {
const CHILD: &str = "CODEX_TEST_RESIDUAL_LAUNCH_JOB";
let executable = std::env::current_exe().expect("test executable");
if std::env::var_os(CHILD).is_some() {
let mut jobs = Vec::new();
for inner in [false, true] {
let job = unsafe { super::CreateJobObjectW(std::ptr::null(), std::ptr::null()) };
assert_ne!(job, 0);
let job = unsafe { OwnedHandle::from_raw_handle(job as _) };
if inner {
let mut limits: super::JOBOBJECT_EXTENDED_LIMIT_INFORMATION =
unsafe { std::mem::zeroed() };
limits.BasicLimitInformation.LimitFlags = super::JOB_OBJECT_LIMIT_BREAKAWAY_OK;
assert_ne!(
unsafe {
super::SetInformationJobObject(
job.as_raw_handle() as _,
super::JobObjectExtendedLimitInformation,
(&limits as *const super::JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(),
std::mem::size_of_val(&limits) as u32,
)
},
0
);
}
assert_ne!(
unsafe {
super::AssignProcessToJobObject(
job.as_raw_handle() as _,
super::GetCurrentProcess(),
)
},
0
);
jobs.push(job);
}
let mut probe = std::process::Command::new(&executable)
.creation_flags(
super::CREATE_SUSPENDED
| super::DETACHED_PROCESS
| super::CREATE_BREAKAWAY_FROM_JOB,
)
.spawn()
.expect("breakaway probe");
let mut in_outer = 0;
let mut in_inner = 0;
let outer_checked = unsafe {
IsProcessInJob(
probe.as_raw_handle() as _,
jobs[0].as_raw_handle() as _,
&mut in_outer,
)
};
let inner_checked = unsafe {
IsProcessInJob(
probe.as_raw_handle() as _,
jobs[1].as_raw_handle() as _,
&mut in_inner,
)
};
probe.kill().expect("terminate probe");
probe.wait().expect("reap probe");
assert_ne!(outer_checked, 0);
assert_ne!(inner_checked, 0);
assert_ne!(in_outer, 0, "probe must retain its outer job");
assert_eq!(in_inner, 0, "probe must leave its inner job");
super::ensure_detached_launch(&executable).expect("residual job must be accepted");
if let Err(err) = super::ensure_not_elevated() {
assert!(err.to_string().contains("non-elevated terminal"));
return;
}
let temp = tempfile::tempdir().expect("temp dir");
let script = temp.path().join("codex.cmd");
std::fs::write(
&script,
"@echo off\r\nif \"%3\"==\"--help\" exit /b 1\r\necho ready > \"%~dp0ready\"\r\nfor /l %%i in (1,1,10000000) do @rem\r\n",
)
.expect("write daemon fixture");
let backend = crate::backend::pid::PidBackend::new(
script,
temp.path().join("state").join("daemon.pid"),
/*remote_control_enabled*/ false,
);
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("runtime");
let pid = runtime
.block_on(backend.start())
.expect("daemon launch")
.expect("new daemon pid");
let process = Process::open(pid)
.expect("open daemon")
.expect("live daemon");
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while !temp.path().join("ready").exists() && std::time::Instant::now() < deadline {
std::thread::sleep(std::time::Duration::from_millis(50));
}
let ready = temp.path().join("ready").exists();
let outer_checked = unsafe {
IsProcessInJob(
process.0.as_raw_handle() as _,
jobs[0].as_raw_handle() as _,
&mut in_outer,
)
};
let inner_checked = unsafe {
IsProcessInJob(
process.0.as_raw_handle() as _,
jobs[1].as_raw_handle() as _,
&mut in_inner,
)
};
process.terminate().expect("terminate daemon fixture");
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while process.is_running().expect("daemon liveness") && std::time::Instant::now() < deadline
{
std::thread::sleep(std::time::Duration::from_millis(50));
}
assert!(!process.is_running().expect("daemon liveness"));
assert!(ready, "daemon fixture did not start");
assert_ne!(outer_checked, 0);
assert_ne!(inner_checked, 0);
assert_ne!(in_outer, 0, "daemon must retain its outer job");
assert_eq!(in_inner, 0, "daemon must leave its inner job");
return;
}
let output = std::process::Command::new(executable)
.args([
"--exact",
"backend::windows::tests::detached_launch_preflight_allows_residual_job",
"--nocapture",
])
.env(CHILD, "1")
.output()
.expect("isolated job test");
assert!(
output.status.success(),
"{}\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed"));
}
#[tokio::test]
async fn identity_queries_do_not_require_termination_access() {
let mut child = tokio::process::Command::new("powershell.exe")