mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Support managed app-server lifecycle on Windows (#42381)
## Why The PID-managed app-server lifecycle previously supported Unix only, leaving Windows unable to start and stop the managed daemon. ## What changed - Launch the Windows daemon as a detached process and fail clearly when the host Job Object prevents detachment. - Add Windows file locking and process creation-time checks so stale or reused PIDs cannot cause an unrelated process to be terminated. - Request graceful shutdown through a shutdown file, then terminate the verified process after the grace period. - Prepare private daemon state directories consistently and pin the executable path across installer junction updates. ## Testing Add Windows tests for process-query permissions, reused and inaccessible PIDs, and stale-record cleanup. GitOrigin-RevId: 5ea579a7c3dc8bb6122ec3a02513836ccaacc41c
This commit is contained in:
1 parent
0588fc941c
commit
715294448f
9 files changed
+464
-37
No files matched your search
Generated
+1
@@ -2168,6 +2168,7 @@ dependencies = [
|
|||||||
"tempfile",
|
"tempfile",
|
||||||
"tokio",
|
"tokio",
|
||||||
"tokio-tungstenite",
|
"tokio-tungstenite",
|
||||||
|
"windows-sys 0.52.0",
|
||||||
]
|
]
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
|
|||||||
@@ -38,3 +38,6 @@ tokio-tungstenite = { workspace = true }
|
|||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
pretty_assertions = { workspace = true }
|
pretty_assertions = { workspace = true }
|
||||||
tempfile = { workspace = true }
|
tempfile = { workspace = true }
|
||||||
|
|
||||||
|
[target.'cfg(windows)'.dependencies]
|
||||||
|
windows-sys = { version = "0.52", features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_IO", "Win32_System_JobObjects", "Win32_Security", "Win32_Security_Authorization", "Win32_System_Threading"] }
|
||||||
@@ -1,4 +1,6 @@
|
|||||||
mod pid;
|
mod pid;
|
||||||
|
#[cfg(windows)]
|
||||||
|
pub(crate) mod windows;
|
||||||
|
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
//! PID reservations serialize launches and protect stale-record cleanup.
|
//! PID reservations serialize detached launches; creation times protect stale-record cleanup.
|
||||||
|
|
||||||
use std::io::SeekFrom;
|
use std::io::SeekFrom;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
@@ -8,7 +8,7 @@ use std::time::Duration;
|
|||||||
use anyhow::Context;
|
use anyhow::Context;
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use anyhow::bail;
|
use anyhow::bail;
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
use codex_app_server_transport::REMOTE_CONTROL_DISABLED_ENV_VAR;
|
use codex_app_server_transport::REMOTE_CONTROL_DISABLED_ENV_VAR;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde::Serialize;
|
use serde::Serialize;
|
||||||
@@ -26,7 +26,7 @@ const START_TIMEOUT: Duration = Duration::from_secs(10);
|
|||||||
const STDERR_LOG_TAIL_BYTES: u64 = 4096;
|
const STDERR_LOG_TAIL_BYTES: u64 = 4096;
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
#[cfg_attr(not(unix), allow(dead_code))]
|
#[cfg_attr(not(any(unix, windows)), allow(dead_code))]
|
||||||
pub(crate) struct PidBackend {
|
pub(crate) struct PidBackend {
|
||||||
codex_bin: PathBuf,
|
codex_bin: PathBuf,
|
||||||
pid_file: PathBuf,
|
pid_file: PathBuf,
|
||||||
@@ -68,7 +68,7 @@ enum PidFileState {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy)]
|
#[derive(Debug, Clone, Copy)]
|
||||||
#[cfg_attr(not(unix), allow(dead_code))]
|
#[cfg_attr(not(any(unix, windows)), allow(dead_code))]
|
||||||
enum PidCommandKind {
|
enum PidCommandKind {
|
||||||
AppServer { remote_control_enabled: bool },
|
AppServer { remote_control_enabled: bool },
|
||||||
UpdateLoop,
|
UpdateLoop,
|
||||||
@@ -115,7 +115,12 @@ impl PidBackend {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(any(unix, windows))]
|
||||||
|
pub(crate) async fn start(&self) -> Result<Option<u32>> {
|
||||||
|
self.start_inner().await
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(not(any(unix, windows)))]
|
||||||
pub(crate) async fn start(&self) -> Result<Option<u32>> {
|
pub(crate) async fn start(&self) -> Result<Option<u32>> {
|
||||||
bail!("pid-managed app-server startup is unsupported on this platform")
|
bail!("pid-managed app-server startup is unsupported on this platform")
|
||||||
}
|
}
|
||||||
@@ -133,7 +138,21 @@ impl PidBackend {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let pid = record.pid;
|
let pid = record.pid;
|
||||||
|
#[cfg(unix)]
|
||||||
self.terminate_process(pid)?;
|
self.terminate_process(pid)?;
|
||||||
|
#[cfg(windows)]
|
||||||
|
let process = {
|
||||||
|
let Some(process) = super::windows::Process::open(pid)? else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
if process.start_time()? != record.process_start_time {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
fs::write(self.pid_file.with_extension("shutdown"), pid.to_string())
|
||||||
|
.await
|
||||||
|
.context("failed to request daemon shutdown")?;
|
||||||
|
process
|
||||||
|
};
|
||||||
let started_at = tokio::time::Instant::now();
|
let started_at = tokio::time::Instant::now();
|
||||||
let deadline = tokio::time::Instant::now() + STOP_TIMEOUT;
|
let deadline = tokio::time::Instant::now() + STOP_TIMEOUT;
|
||||||
let mut forced = false;
|
let mut forced = false;
|
||||||
@@ -155,7 +174,10 @@ impl PidBackend {
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if !forced && started_at.elapsed() >= STOP_GRACE_PERIOD {
|
if !forced && started_at.elapsed() >= STOP_GRACE_PERIOD {
|
||||||
|
#[cfg(unix)]
|
||||||
self.force_terminate_process(pid)?;
|
self.force_terminate_process(pid)?;
|
||||||
|
#[cfg(windows)]
|
||||||
|
process.terminate()?;
|
||||||
forced = true;
|
forced = true;
|
||||||
}
|
}
|
||||||
sleep(STOP_POLL_INTERVAL).await;
|
sleep(STOP_POLL_INTERVAL).await;
|
||||||
@@ -275,7 +297,7 @@ impl PidBackend {
|
|||||||
Ok(reservation_lock)
|
Ok(reservation_lock)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
async fn open_stderr_log(&self) -> Result<fs::File> {
|
async fn open_stderr_log(&self) -> Result<fs::File> {
|
||||||
let stderr_log_file = stderr_log_file_for_pid_file(&self.pid_file);
|
let stderr_log_file = stderr_log_file_for_pid_file(&self.pid_file);
|
||||||
fs::OpenOptions::new()
|
fs::OpenOptions::new()
|
||||||
@@ -292,7 +314,7 @@ impl PidBackend {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
fn command_args(&self) -> Vec<&'static str> {
|
fn command_args(&self) -> Vec<&'static str> {
|
||||||
match self.command_kind {
|
match self.command_kind {
|
||||||
PidCommandKind::AppServer {
|
PidCommandKind::AppServer {
|
||||||
@@ -305,7 +327,7 @@ impl PidBackend {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
fn command_env(&self) -> Option<(&'static str, &'static str)> {
|
fn command_env(&self) -> Option<(&'static str, &'static str)> {
|
||||||
match self.command_kind {
|
match self.command_kind {
|
||||||
PidCommandKind::AppServer {
|
PidCommandKind::AppServer {
|
||||||
@@ -325,6 +347,7 @@ impl PidBackend {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(unix)]
|
||||||
fn force_terminate_process(&self, pid: u32) -> Result<()> {
|
fn force_terminate_process(&self, pid: u32) -> Result<()> {
|
||||||
match self.command_kind {
|
match self.command_kind {
|
||||||
PidCommandKind::AppServer { .. } => force_terminate_process(pid),
|
PidCommandKind::AppServer { .. } => force_terminate_process(pid),
|
||||||
@@ -441,17 +464,17 @@ fn force_terminate_process_group(pid: u32) -> Result<()> {
|
|||||||
Err(err).with_context(|| format!("failed to force terminate pid-managed updater group {pid}"))
|
Err(err).with_context(|| format!("failed to force terminate pid-managed updater group {pid}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn terminate_process(_pid: u32) -> Result<()> {
|
fn terminate_process(_pid: u32) -> Result<()> {
|
||||||
bail!("pid-managed app-server shutdown is unsupported on this platform")
|
bail!("pid-managed app-server shutdown is unsupported on this platform")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn force_terminate_process(_pid: u32) -> Result<()> {
|
fn force_terminate_process(_pid: u32) -> Result<()> {
|
||||||
bail!("pid-managed app-server shutdown is unsupported on this platform")
|
bail!("pid-managed app-server shutdown is unsupported on this platform")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn force_terminate_process_group(_pid: u32) -> Result<()> {
|
fn force_terminate_process_group(_pid: u32) -> Result<()> {
|
||||||
bail!("pid-managed updater shutdown is unsupported on this platform")
|
bail!("pid-managed updater shutdown is unsupported on this platform")
|
||||||
}
|
}
|
||||||
@@ -469,13 +492,13 @@ async fn process_matches_record(record: &PidRecord) -> Result<bool> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
async fn process_matches_record(_record: &PidRecord) -> Result<bool> {
|
async fn process_matches_record(_record: &PidRecord) -> Result<bool> {
|
||||||
Ok(false)
|
Ok(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
#[cfg_attr(not(unix), allow(dead_code))]
|
#[cfg_attr(not(any(unix, windows)), allow(dead_code))]
|
||||||
enum EmptyPidReservation {
|
enum EmptyPidReservation {
|
||||||
Active,
|
Active,
|
||||||
Stale,
|
Stale,
|
||||||
@@ -498,12 +521,12 @@ fn try_lock_file(file: &fs::File) -> Result<bool> {
|
|||||||
Err(err).context("failed to lock pid reservation")
|
Err(err).context("failed to lock pid reservation")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn try_lock_file(_file: &fs::File) -> Result<bool> {
|
fn try_lock_file(_file: &fs::File) -> Result<bool> {
|
||||||
bail!("pid-managed app-server startup is unsupported on this platform")
|
bail!("pid-managed app-server startup is unsupported on this platform")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
async fn reservation_lock_is_active(path: &Path) -> Result<bool> {
|
async fn reservation_lock_is_active(path: &Path) -> Result<bool> {
|
||||||
let file = match fs::OpenOptions::new()
|
let file = match fs::OpenOptions::new()
|
||||||
.write(true)
|
.write(true)
|
||||||
@@ -524,12 +547,12 @@ async fn reservation_lock_is_active(path: &Path) -> Result<bool> {
|
|||||||
Ok(!try_lock_file(&file)?)
|
Ok(!try_lock_file(&file)?)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
async fn reservation_lock_is_active(_path: &Path) -> Result<bool> {
|
async fn reservation_lock_is_active(_path: &Path) -> Result<bool> {
|
||||||
Ok(false)
|
Ok(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(any(unix, windows))]
|
||||||
async fn inspect_empty_pid_reservation(
|
async fn inspect_empty_pid_reservation(
|
||||||
pid_path: &Path,
|
pid_path: &Path,
|
||||||
lock_path: &Path,
|
lock_path: &Path,
|
||||||
@@ -572,7 +595,7 @@ async fn inspect_empty_pid_reservation(
|
|||||||
Ok(EmptyPidReservation::Record(record))
|
Ok(EmptyPidReservation::Record(record))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
async fn inspect_empty_pid_reservation(
|
async fn inspect_empty_pid_reservation(
|
||||||
_pid_path: &Path,
|
_pid_path: &Path,
|
||||||
_lock_path: &Path,
|
_lock_path: &Path,
|
||||||
@@ -600,10 +623,53 @@ async fn read_process_start_time(pid: u32) -> Result<String> {
|
|||||||
Ok(start_time.to_string())
|
Ok(start_time.to_string())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(all(test, unix))]
|
#[cfg(all(test, any(unix, windows)))]
|
||||||
#[path = "pid_tests.rs"]
|
#[path = "pid_tests.rs"]
|
||||||
mod tests;
|
mod tests;
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(windows)]
|
||||||
|
use super::windows::try_lock_file;
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
async fn read_process_start_time(pid: u32) -> Result<String> {
|
||||||
|
super::windows::Process::open(pid)?
|
||||||
|
.context("daemon process exited during startup")?
|
||||||
|
.start_time()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
async fn process_matches_record(record: &PidRecord) -> Result<bool> {
|
||||||
|
let process = match super::windows::Process::open(record.pid) {
|
||||||
|
Ok(process) => process,
|
||||||
|
// A managed daemon is queryable by its launching user. A stale PID may
|
||||||
|
// have been reused by a protected process; never try to terminate it.
|
||||||
|
Err(err)
|
||||||
|
if err.downcast_ref::<std::io::Error>().is_some_and(|err| {
|
||||||
|
err.raw_os_error()
|
||||||
|
== Some(windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED as i32)
|
||||||
|
}) =>
|
||||||
|
{
|
||||||
|
return Ok(false);
|
||||||
|
}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
};
|
||||||
|
let Some(process) = process else {
|
||||||
|
return Ok(false);
|
||||||
|
};
|
||||||
|
Ok(process.is_running()? && process.start_time()? == record.process_start_time)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
fn force_terminate_process(pid: u32) -> Result<()> {
|
||||||
|
if let Some(process) = super::windows::Process::open(pid)? {
|
||||||
|
process.terminate()?;
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
use force_terminate_process as terminate_process;
|
||||||
|
|
||||||
|
#[cfg(any(unix, windows))]
|
||||||
#[path = "pid_start.rs"]
|
#[path = "pid_start.rs"]
|
||||||
mod start;
|
mod start;
|
||||||
@@ -1,4 +1,5 @@
|
|||||||
//! Detached Unix process launch and PID publication under the reservation lock.
|
//! Detached process launch and PID publication. Hold the reservation lock until
|
||||||
|
//! the record is published.
|
||||||
|
|
||||||
use super::PidBackend;
|
use super::PidBackend;
|
||||||
use super::PidFileState;
|
use super::PidFileState;
|
||||||
@@ -11,21 +12,24 @@ use tokio::fs;
|
|||||||
use tokio::process::Command;
|
use tokio::process::Command;
|
||||||
|
|
||||||
impl PidBackend {
|
impl PidBackend {
|
||||||
pub(crate) async fn start(&self) -> Result<Option<u32>> {
|
pub(super) async fn start_inner(&self) -> Result<Option<u32>> {
|
||||||
if let Some(parent) = self.pid_file.parent() {
|
if let Some(parent) = self.pid_file.parent() {
|
||||||
fs::create_dir_all(parent)
|
codex_uds::prepare_private_socket_directory(parent)
|
||||||
.await
|
.await
|
||||||
.with_context(|| format!("failed to create pid directory {}", parent.display()))?;
|
.with_context(|| format!("failed to create pid directory {}", parent.display()))?;
|
||||||
}
|
}
|
||||||
let reservation_lock = self.acquire_reservation_lock().await?;
|
let reservation_lock = self.acquire_reservation_lock().await?;
|
||||||
let _pid_file = loop {
|
loop {
|
||||||
match fs::OpenOptions::new()
|
match fs::OpenOptions::new()
|
||||||
.create_new(true)
|
.create_new(true)
|
||||||
.write(true)
|
.write(true)
|
||||||
.open(&self.pid_file)
|
.open(&self.pid_file)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(pid_file) => break pid_file,
|
Ok(pid_file) => {
|
||||||
|
drop(pid_file);
|
||||||
|
break;
|
||||||
|
}
|
||||||
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
|
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
|
||||||
match self.read_pid_file_state_with_lock_held().await? {
|
match self.read_pid_file_state_with_lock_held().await? {
|
||||||
PidFileState::Missing => continue,
|
PidFileState::Missing => continue,
|
||||||
@@ -47,8 +51,15 @@ impl PidBackend {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
};
|
}
|
||||||
let mut command = Command::new(&self.codex_bin);
|
// Pin the Windows image path across installer junction retargeting.
|
||||||
|
#[cfg(windows)]
|
||||||
|
let codex_bin = fs::canonicalize(&self.codex_bin)
|
||||||
|
.await
|
||||||
|
.unwrap_or_else(|_| self.codex_bin.clone());
|
||||||
|
#[cfg(not(windows))]
|
||||||
|
let codex_bin = &self.codex_bin;
|
||||||
|
let mut command = Command::new(codex_bin);
|
||||||
let stderr_log = match self.open_stderr_log().await {
|
let stderr_log = match self.open_stderr_log().await {
|
||||||
Ok(stderr_log) => stderr_log,
|
Ok(stderr_log) => stderr_log,
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
@@ -77,13 +88,37 @@ impl PidBackend {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
{
|
||||||
|
use windows_sys::Win32::System::Threading::CREATE_BREAKAWAY_FROM_JOB;
|
||||||
|
use windows_sys::Win32::System::Threading::DETACHED_PROCESS;
|
||||||
|
// Never retry inside the parent's Job Object: that would report a
|
||||||
|
// successful launch that dies when the terminal/SSH session closes.
|
||||||
|
command.creation_flags(DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB);
|
||||||
|
let shutdown_file = self.pid_file.with_extension("shutdown");
|
||||||
|
match fs::remove_file(&shutdown_file).await {
|
||||||
|
Ok(()) => {}
|
||||||
|
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
|
||||||
|
Err(err) => return Err(err).context("failed to clear daemon shutdown request"),
|
||||||
|
}
|
||||||
|
command.env(
|
||||||
|
codex_app_server_transport::DAEMON_SHUTDOWN_FILE_ENV,
|
||||||
|
shutdown_file,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
let child = match command.spawn() {
|
let child = match command.spawn() {
|
||||||
Ok(child) => child,
|
Ok(child) => child,
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
let _ = fs::remove_file(&self.pid_file).await;
|
let _ = fs::remove_file(&self.pid_file).await;
|
||||||
return Err(err).with_context(|| {
|
return Err(err).with_context(|| {
|
||||||
|
let job_hint = if cfg!(windows) {
|
||||||
|
" (the Windows host job must allow breakaway)"
|
||||||
|
} else {
|
||||||
|
""
|
||||||
|
};
|
||||||
format!(
|
format!(
|
||||||
"failed to spawn detached app-server process using {}",
|
"failed to spawn detached app-server process using {}{job_hint}",
|
||||||
self.codex_bin.display()
|
self.codex_bin.display()
|
||||||
)
|
)
|
||||||
});
|
});
|
||||||
@@ -92,7 +127,15 @@ impl PidBackend {
|
|||||||
let pid = child
|
let pid = child
|
||||||
.id()
|
.id()
|
||||||
.context("spawned app-server process has no pid")?;
|
.context("spawned app-server process has no pid")?;
|
||||||
let record = match read_process_start_time(pid).await {
|
let record = match async {
|
||||||
|
#[cfg(windows)]
|
||||||
|
super::super::windows::Process::open(pid)?
|
||||||
|
.context("daemon exited during launch")?
|
||||||
|
.ensure_detached()?;
|
||||||
|
read_process_start_time(pid).await
|
||||||
|
}
|
||||||
|
.await
|
||||||
|
{
|
||||||
Ok(process_start_time) => PidRecord {
|
Ok(process_start_time) => PidRecord {
|
||||||
pid,
|
pid,
|
||||||
process_start_time,
|
process_start_time,
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#[cfg(unix)]
|
||||||
use std::process::Stdio;
|
use std::process::Stdio;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
@@ -11,6 +12,7 @@ use super::PidCommandKind;
|
|||||||
use super::PidFileState;
|
use super::PidFileState;
|
||||||
use super::PidLogTail;
|
use super::PidLogTail;
|
||||||
use super::PidRecord;
|
use super::PidRecord;
|
||||||
|
#[cfg(unix)]
|
||||||
use super::read_process_start_time;
|
use super::read_process_start_time;
|
||||||
use super::read_stderr_log_tail;
|
use super::read_stderr_log_tail;
|
||||||
use super::stderr_log_file_for_pid_file;
|
use super::stderr_log_file_for_pid_file;
|
||||||
@@ -99,7 +101,11 @@ async fn stop_waits_for_live_reservation_to_resolve() {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn start_retries_stale_empty_pid_file_under_its_own_lock() {
|
async fn start_retries_stale_empty_pid_file_under_its_own_lock() {
|
||||||
let temp_dir = TempDir::new().expect("temp dir");
|
let temp_dir = TempDir::new().expect("temp dir");
|
||||||
let pid_file = temp_dir.path().join("app-server.pid");
|
let state_dir = temp_dir.path().join("state");
|
||||||
|
codex_uds::prepare_private_socket_directory(&state_dir)
|
||||||
|
.await
|
||||||
|
.expect("private state directory");
|
||||||
|
let pid_file = state_dir.join("app-server.pid");
|
||||||
tokio::fs::write(&pid_file, "")
|
tokio::fs::write(&pid_file, "")
|
||||||
.await
|
.await
|
||||||
.expect("write pid file");
|
.expect("write pid file");
|
||||||
@@ -149,6 +155,7 @@ async fn stale_record_cleanup_preserves_replacement_record() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(unix)]
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn stop_reaps_untracked_app_server_child() {
|
async fn stop_reaps_untracked_app_server_child() {
|
||||||
let temp_dir = TempDir::new().expect("temp dir");
|
let temp_dir = TempDir::new().expect("temp dir");
|
||||||
@@ -255,3 +262,68 @@ async fn read_stderr_log_tail_returns_recent_complete_lines() {
|
|||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[tokio::test]
|
||||||
|
async fn stale_creation_time_never_stops_reused_pid() {
|
||||||
|
let temp = TempDir::new().expect("temp");
|
||||||
|
let backend = PidBackend::new(
|
||||||
|
temp.path().join("codex.exe"),
|
||||||
|
temp.path().join("server.pid"),
|
||||||
|
/*remote_control_enabled*/ false,
|
||||||
|
);
|
||||||
|
let record = PidRecord {
|
||||||
|
pid: std::process::id(),
|
||||||
|
process_start_time: "stale".into(),
|
||||||
|
};
|
||||||
|
tokio::fs::write(&backend.pid_file, serde_json::to_vec(&record).unwrap())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
backend.stop().await.expect("stale record cleanup");
|
||||||
|
assert!(!backend.pid_file.exists());
|
||||||
|
assert!(!backend.pid_file.with_extension("shutdown").exists());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[test]
|
||||||
|
fn inaccessible_reused_pid_is_stale_without_hiding_process_open_errors() {
|
||||||
|
use futures::FutureExt;
|
||||||
|
use windows_sys::Win32::Security::ImpersonateAnonymousToken;
|
||||||
|
use windows_sys::Win32::Security::RevertToSelf;
|
||||||
|
use windows_sys::Win32::System::Threading::GetCurrentThread;
|
||||||
|
|
||||||
|
// Isolate impersonation from Tokio workers and other tests. Query our own
|
||||||
|
// process anonymously instead of assuming a system PID is inaccessible.
|
||||||
|
std::thread::spawn(|| {
|
||||||
|
let record = PidRecord {
|
||||||
|
pid: std::process::id(),
|
||||||
|
process_start_time: "stale".into(),
|
||||||
|
};
|
||||||
|
assert!(
|
||||||
|
crate::backend::windows::Process::open(record.pid)
|
||||||
|
.unwrap()
|
||||||
|
.is_some()
|
||||||
|
);
|
||||||
|
assert_ne!(unsafe { ImpersonateAnonymousToken(GetCurrentThread()) }, 0);
|
||||||
|
let opened = crate::backend::windows::Process::open(record.pid);
|
||||||
|
// Windows identity checks are synchronous; never suspend while impersonating.
|
||||||
|
let matches = super::process_matches_record(&record).now_or_never();
|
||||||
|
let reverted = unsafe { RevertToSelf() };
|
||||||
|
assert_ne!(reverted, 0);
|
||||||
|
|
||||||
|
let error = match opened {
|
||||||
|
Err(error) => error,
|
||||||
|
Ok(_) => panic!("anonymous caller unexpectedly queried the test process"),
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
error
|
||||||
|
.downcast_ref::<std::io::Error>()
|
||||||
|
.unwrap()
|
||||||
|
.raw_os_error(),
|
||||||
|
Some(windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED as i32),
|
||||||
|
);
|
||||||
|
assert!(!matches.expect("identity check must not suspend").unwrap());
|
||||||
|
})
|
||||||
|
.join()
|
||||||
|
.expect("anonymous identity check");
|
||||||
|
}
|
||||||
@@ -0,0 +1,161 @@
|
|||||||
|
//! Windows process identity and file locks. Keep a process handle across shutdown
|
||||||
|
//! so PID reuse can never redirect forced termination to a different process.
|
||||||
|
|
||||||
|
use std::io;
|
||||||
|
use std::os::windows::io::AsRawHandle;
|
||||||
|
use std::os::windows::io::FromRawHandle;
|
||||||
|
use std::os::windows::io::OwnedHandle;
|
||||||
|
|
||||||
|
use anyhow::Context;
|
||||||
|
use anyhow::Result;
|
||||||
|
use windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER;
|
||||||
|
use windows_sys::Win32::Foundation::ERROR_LOCK_VIOLATION;
|
||||||
|
use windows_sys::Win32::Foundation::FILETIME;
|
||||||
|
use windows_sys::Win32::Foundation::WAIT_OBJECT_0;
|
||||||
|
use windows_sys::Win32::Foundation::WAIT_TIMEOUT;
|
||||||
|
use windows_sys::Win32::Storage::FileSystem::LOCKFILE_EXCLUSIVE_LOCK;
|
||||||
|
use windows_sys::Win32::Storage::FileSystem::LOCKFILE_FAIL_IMMEDIATELY;
|
||||||
|
use windows_sys::Win32::Storage::FileSystem::LockFileEx;
|
||||||
|
use windows_sys::Win32::System::JobObjects::IsProcessInJob;
|
||||||
|
use windows_sys::Win32::System::Threading::GetProcessId;
|
||||||
|
use windows_sys::Win32::System::Threading::GetProcessTimes;
|
||||||
|
use windows_sys::Win32::System::Threading::OpenProcess;
|
||||||
|
use windows_sys::Win32::System::Threading::PROCESS_ACCESS_RIGHTS;
|
||||||
|
use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION;
|
||||||
|
use windows_sys::Win32::System::Threading::PROCESS_SYNCHRONIZE;
|
||||||
|
use windows_sys::Win32::System::Threading::PROCESS_TERMINATE;
|
||||||
|
use windows_sys::Win32::System::Threading::TerminateProcess;
|
||||||
|
use windows_sys::Win32::System::Threading::WaitForSingleObject;
|
||||||
|
|
||||||
|
pub(super) struct Process(OwnedHandle);
|
||||||
|
|
||||||
|
impl Process {
|
||||||
|
pub(super) fn open(pid: u32) -> Result<Option<Self>> {
|
||||||
|
Self::open_with_access(pid, PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn open_with_access(pid: u32, access: PROCESS_ACCESS_RIGHTS) -> Result<Option<Self>> {
|
||||||
|
let handle = unsafe {
|
||||||
|
OpenProcess(access, /*binherithandle*/ 0, pid)
|
||||||
|
};
|
||||||
|
if handle == 0 {
|
||||||
|
let err = io::Error::last_os_error();
|
||||||
|
return if err.raw_os_error() == Some(ERROR_INVALID_PARAMETER as i32) {
|
||||||
|
Ok(None)
|
||||||
|
} else {
|
||||||
|
Err(err).context("failed to open daemon process")
|
||||||
|
};
|
||||||
|
}
|
||||||
|
Ok(Some(Self(unsafe {
|
||||||
|
OwnedHandle::from_raw_handle(handle as _)
|
||||||
|
})))
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(super) fn start_time(&self) -> Result<String> {
|
||||||
|
let mut created: FILETIME = unsafe { std::mem::zeroed() };
|
||||||
|
let mut exited = created;
|
||||||
|
let mut kernel = created;
|
||||||
|
let mut user = created;
|
||||||
|
if unsafe {
|
||||||
|
GetProcessTimes(
|
||||||
|
self.0.as_raw_handle() as _,
|
||||||
|
&mut created,
|
||||||
|
&mut exited,
|
||||||
|
&mut kernel,
|
||||||
|
&mut user,
|
||||||
|
)
|
||||||
|
} == 0
|
||||||
|
{
|
||||||
|
return Err(io::Error::last_os_error()).context("failed to query daemon creation time");
|
||||||
|
}
|
||||||
|
Ok(
|
||||||
|
((u64::from(created.dwHighDateTime) << 32) | u64::from(created.dwLowDateTime))
|
||||||
|
.to_string(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(super) fn is_running(&self) -> Result<bool> {
|
||||||
|
match unsafe {
|
||||||
|
WaitForSingleObject(self.0.as_raw_handle() as _, /*dwmilliseconds*/ 0)
|
||||||
|
} {
|
||||||
|
WAIT_TIMEOUT => Ok(true),
|
||||||
|
WAIT_OBJECT_0 => Ok(false),
|
||||||
|
_ => Err(io::Error::last_os_error()).context("failed to wait for daemon 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(());
|
||||||
|
}
|
||||||
|
let pid = unsafe { GetProcessId(self.0.as_raw_handle() as _) };
|
||||||
|
let Some(target) = Self::open_with_access(
|
||||||
|
pid,
|
||||||
|
PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE | PROCESS_TERMINATE,
|
||||||
|
)?
|
||||||
|
else {
|
||||||
|
return Ok(());
|
||||||
|
};
|
||||||
|
// Keep the original identity handle alive and validate the handle that
|
||||||
|
// will actually be terminated, rather than trusting a second PID lookup.
|
||||||
|
if target.start_time()? != self.start_time()? || !target.is_running()? {
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
if unsafe {
|
||||||
|
TerminateProcess(target.0.as_raw_handle() as _, /*uexitcode*/ 1)
|
||||||
|
} == 0
|
||||||
|
{
|
||||||
|
return Err(io::Error::last_os_error()).context("failed to terminate daemon process");
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn try_lock_file(file: &tokio::fs::File) -> Result<bool> {
|
||||||
|
let mut overlapped = unsafe { std::mem::zeroed() };
|
||||||
|
if unsafe {
|
||||||
|
LockFileEx(
|
||||||
|
file.as_raw_handle() as _,
|
||||||
|
LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY,
|
||||||
|
/*dwreserved*/ 0,
|
||||||
|
/*nnumberofbytestolocklow*/ 1,
|
||||||
|
/*nnumberofbytestolockhigh*/ 0,
|
||||||
|
&mut overlapped,
|
||||||
|
)
|
||||||
|
} != 0
|
||||||
|
{
|
||||||
|
return Ok(true);
|
||||||
|
}
|
||||||
|
let err = io::Error::last_os_error();
|
||||||
|
if err.raw_os_error() == Some(ERROR_LOCK_VIOLATION as i32) {
|
||||||
|
return Ok(false);
|
||||||
|
}
|
||||||
|
Err(err).context("failed to lock daemon state")
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "windows_tests.rs"]
|
||||||
|
mod tests;
|
||||||
@@ -0,0 +1,47 @@
|
|||||||
|
use super::Process;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
use std::os::windows::io::AsRawHandle;
|
||||||
|
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
|
||||||
|
use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW;
|
||||||
|
use windows_sys::Win32::System::Threading::TerminateProcess;
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn identity_queries_do_not_require_termination_access() {
|
||||||
|
let mut child = tokio::process::Command::new("powershell.exe")
|
||||||
|
.args([
|
||||||
|
"-NoProfile",
|
||||||
|
"-NonInteractive",
|
||||||
|
"-Command",
|
||||||
|
"Start-Sleep 60",
|
||||||
|
])
|
||||||
|
.creation_flags(CREATE_NO_WINDOW)
|
||||||
|
.kill_on_drop(true)
|
||||||
|
.spawn()
|
||||||
|
.expect("child");
|
||||||
|
let process = Process::open(child.id().expect("pid"))
|
||||||
|
.expect("query handle")
|
||||||
|
.expect("live process");
|
||||||
|
assert!(!process.start_time().expect("creation time").is_empty());
|
||||||
|
assert!(process.is_running().expect("liveness"));
|
||||||
|
// Check the rights on the actual query handle, independent of privileges
|
||||||
|
// that could let the caller reopen the process with termination access.
|
||||||
|
assert_eq!(
|
||||||
|
unsafe {
|
||||||
|
TerminateProcess(process.0.as_raw_handle() as _, /*uexitcode*/ 1)
|
||||||
|
},
|
||||||
|
0
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
std::io::Error::last_os_error().raw_os_error(),
|
||||||
|
Some(ERROR_ACCESS_DENIED as i32)
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
process
|
||||||
|
.is_running()
|
||||||
|
.expect("query must not terminate child")
|
||||||
|
);
|
||||||
|
child
|
||||||
|
.kill()
|
||||||
|
.await
|
||||||
|
.expect("cleanup through original spawn handle");
|
||||||
|
}
|
||||||
@@ -1,4 +1,8 @@
|
|||||||
|
//! Managed app-server lifecycle, serialized across CLI invocations and the updater.
|
||||||
|
|
||||||
mod backend;
|
mod backend;
|
||||||
|
#[cfg(windows)]
|
||||||
|
use backend::windows::try_lock_file;
|
||||||
mod client;
|
mod client;
|
||||||
mod managed_install;
|
mod managed_install;
|
||||||
mod remote_control_client;
|
mod remote_control_client;
|
||||||
@@ -727,12 +731,18 @@ impl Daemon {
|
|||||||
|
|
||||||
async fn open_operation_lock_file(&self) -> Result<tokio::fs::File> {
|
async fn open_operation_lock_file(&self) -> Result<tokio::fs::File> {
|
||||||
if let Some(parent) = self.operation_lock_file.parent() {
|
if let Some(parent) = self.operation_lock_file.parent() {
|
||||||
tokio::fs::create_dir_all(parent).await.with_context(|| {
|
#[cfg(unix)]
|
||||||
format!(
|
if let Some(home) = parent.parent() {
|
||||||
"failed to create daemon state directory {}",
|
tokio::fs::create_dir_all(home).await?;
|
||||||
parent.display()
|
}
|
||||||
)
|
codex_uds::prepare_private_socket_directory(parent)
|
||||||
})?;
|
.await
|
||||||
|
.with_context(|| {
|
||||||
|
format!(
|
||||||
|
"failed to create daemon state directory {}",
|
||||||
|
parent.display()
|
||||||
|
)
|
||||||
|
})?;
|
||||||
}
|
}
|
||||||
tokio::fs::OpenOptions::new()
|
tokio::fs::OpenOptions::new()
|
||||||
.create(true)
|
.create(true)
|
||||||
@@ -842,7 +852,7 @@ fn try_lock_file(file: &tokio::fs::File) -> Result<bool> {
|
|||||||
Err(err).context("failed to lock daemon operation")
|
Err(err).context("failed to lock daemon operation")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn try_lock_file(_file: &tokio::fs::File) -> Result<bool> {
|
fn try_lock_file(_file: &tokio::fs::File) -> Result<bool> {
|
||||||
Ok(true)
|
Ok(true)
|
||||||
}
|
}
|
||||||
@@ -1006,6 +1016,28 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn stop_creates_missing_home_parent() {
|
||||||
|
let temp = TempDir::new().expect("temp dir");
|
||||||
|
let state = temp.path().join("missing-home").join("daemon-state");
|
||||||
|
let daemon = Daemon {
|
||||||
|
socket_path: state.join("server.sock"),
|
||||||
|
pid_file: state.join("server.pid"),
|
||||||
|
update_pid_file: state.join("updater.pid"),
|
||||||
|
operation_lock_file: state.join("daemon.lock"),
|
||||||
|
settings_file: state.join("settings.json"),
|
||||||
|
managed_codex_bin: state.join("missing-codex"),
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
daemon
|
||||||
|
.run(super::LifecycleCommand::Stop)
|
||||||
|
.await
|
||||||
|
.expect("stop on fresh home")
|
||||||
|
.status,
|
||||||
|
LifecycleStatus::NotRunning,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn not_ready_context_reports_daemon_app_server_before_stderr() {
|
async fn not_ready_context_reports_daemon_app_server_before_stderr() {
|
||||||
let temp_dir = TempDir::new().expect("temp dir");
|
let temp_dir = TempDir::new().expect("temp dir");
|
||||||
|
|||||||
Reference in new issue
Block a user