mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Run managed daemon processes at background priority on macOS (#54401)
## What changed - Launch managed processes through `taskpolicy -b` on macOS when a five-second probe succeeds, preserving the managed process PID. Fall back to direct launch if the probe fails or times out. - Increase the macOS startup timeout from 10 to 30 seconds to accommodate background CPU and I/O scheduling, and serialize daemon startup test fixtures on macOS. - Use the shared background command helper and retain `CREATE_NO_WINDOW` alongside Windows detachment flags. ## Testing Add macOS tests for PID preservation, background priority when available, and direct-launch fallback after a probe timeout. GitOrigin-RevId: ab3328e2e8f62218ac9602e4172563c00056875b
This commit is contained in:
committed by
copyberry
parent
aabf13307a
commit
97ebf2d94d
Generated
+1
@@ -2227,6 +2227,7 @@ dependencies = [
|
||||
"codex-uds",
|
||||
"codex-utils-cargo-bin",
|
||||
"codex-utils-home-dir",
|
||||
"codex-utils-process",
|
||||
"futures",
|
||||
"libc",
|
||||
"pretty_assertions",
|
||||
|
||||
@@ -20,6 +20,7 @@ codex-features = { workspace = true }
|
||||
codex-http-client = { workspace = true }
|
||||
codex-install-context = { workspace = true }
|
||||
codex-utils-home-dir = { workspace = true }
|
||||
codex-utils-process = { workspace = true }
|
||||
codex-uds = { workspace = true }
|
||||
futures = { workspace = true }
|
||||
libc = { workspace = true }
|
||||
|
||||
@@ -95,7 +95,6 @@ impl PidBackend {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let mut command = Command::new(&codex_bin);
|
||||
let stderr_log = match self.open_stderr_log().await {
|
||||
Ok(stderr_log) => stderr_log,
|
||||
Err(err) => {
|
||||
@@ -105,17 +104,10 @@ impl PidBackend {
|
||||
return Err(err);
|
||||
}
|
||||
};
|
||||
command
|
||||
// Handoff suppression belongs to the foreground CLI, not its long-lived children.
|
||||
.env_remove(crate::telemetry::HANDOFF_ENV)
|
||||
.args(self.command_args().iter().map(std::borrow::Cow::as_ref))
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::from(stderr_log.into_std().await));
|
||||
// Older or pinned managed binaries may predate this optional startup flag.
|
||||
let managed_app_server =
|
||||
matches!(self.command_kind, super::PidCommandKind::AppServer { .. });
|
||||
if managed_app_server
|
||||
let managed_daemon_supported = managed_app_server
|
||||
&& matches!(
|
||||
tokio::time::timeout(
|
||||
std::time::Duration::from_secs(5),
|
||||
@@ -128,10 +120,8 @@ impl PidBackend {
|
||||
.status(),
|
||||
).await,
|
||||
Ok(Ok(status)) if status.success()
|
||||
)
|
||||
{
|
||||
command.arg("--managed-daemon");
|
||||
} else if managed_app_server {
|
||||
);
|
||||
if managed_app_server && !managed_daemon_supported {
|
||||
let codex_home = self
|
||||
.pid_file
|
||||
.parent()
|
||||
@@ -146,6 +136,31 @@ impl PidBackend {
|
||||
}
|
||||
}
|
||||
}
|
||||
let command = crate::background_command::managed_process(&codex_bin).await;
|
||||
let mut command = match command {
|
||||
Ok(command) => Command::from(command),
|
||||
Err(err) => {
|
||||
if replacement.is_none() {
|
||||
let _ = fs::remove_file(&self.pid_file).await;
|
||||
}
|
||||
return Err(err).with_context(|| {
|
||||
format!(
|
||||
"failed to spawn detached app-server process using {}",
|
||||
self.codex_bin.display()
|
||||
)
|
||||
});
|
||||
}
|
||||
};
|
||||
command
|
||||
// Handoff suppression belongs to the foreground CLI, not its long-lived children.
|
||||
.env_remove(crate::telemetry::HANDOFF_ENV)
|
||||
.args(self.command_args().iter().map(std::borrow::Cow::as_ref))
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::from(stderr_log.into_std().await));
|
||||
if managed_daemon_supported {
|
||||
command.arg("--managed-daemon");
|
||||
}
|
||||
if let Some((key, value)) = self.command_env() {
|
||||
command.env(key, value);
|
||||
}
|
||||
@@ -172,6 +187,7 @@ impl PidBackend {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use windows_sys::Win32::System::Threading::CREATE_BREAKAWAY_FROM_JOB;
|
||||
use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW;
|
||||
use windows_sys::Win32::System::Threading::DETACHED_PROCESS;
|
||||
// A Windows process pins its working directory for its lifetime.
|
||||
// Keep both managed children out of the launching project's directory.
|
||||
@@ -179,7 +195,7 @@ impl PidBackend {
|
||||
command.current_dir(&workdir);
|
||||
// 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);
|
||||
command.creation_flags(CREATE_NO_WINDOW | DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB);
|
||||
if matches!(self.command_kind, PidCommandKind::UpdateLoop { .. }) {
|
||||
match fs::remove_file(self.pid_file.with_extension("ready")).await {
|
||||
Ok(()) => {}
|
||||
|
||||
@@ -1,11 +1,67 @@
|
||||
//! Recovers background startup after the launching directory is removed.
|
||||
//! Preserve process-scoped paths; retain usable Unix cwd defaults for clients.
|
||||
//! Prepares long-lived daemon commands for background execution and recovers
|
||||
//! startup after the launching directory is removed.
|
||||
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
#[cfg(target_os = "macos")]
|
||||
use std::process::Stdio;
|
||||
#[cfg(target_os = "macos")]
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Result;
|
||||
|
||||
pub(crate) async fn managed_process(program: &Path) -> std::io::Result<Command> {
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
let mut probe = tokio::process::Command::new("/usr/sbin/taskpolicy");
|
||||
probe.args(["-b", "/usr/bin/true"]);
|
||||
return managed_process_with_probe(program, probe, Duration::from_secs(5)).await;
|
||||
}
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
Ok(codex_utils_process::background_command(program))
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
async fn managed_process_with_probe(
|
||||
program: &Path,
|
||||
mut probe: tokio::process::Command,
|
||||
timeout: Duration,
|
||||
) -> std::io::Result<Command> {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let metadata = program.metadata()?;
|
||||
if !metadata.is_file() || metadata.permissions().mode() & 0o111 == 0 {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::PermissionDenied,
|
||||
"managed Codex path is not executable",
|
||||
));
|
||||
}
|
||||
|
||||
// Preserve daemon availability when taskpolicy is denied by a restricted
|
||||
// environment such as a test sandbox, or when the probe cannot finish promptly.
|
||||
let available = matches!(
|
||||
tokio::time::timeout(
|
||||
timeout,
|
||||
probe
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.kill_on_drop(true)
|
||||
.status(),
|
||||
)
|
||||
.await,
|
||||
Ok(Ok(status)) if status.success()
|
||||
);
|
||||
if available {
|
||||
// taskpolicy uses POSIX_SPAWN_SETEXEC, so the managed process keeps this PID.
|
||||
let mut command = codex_utils_process::background_command("/usr/sbin/taskpolicy");
|
||||
command.arg("-b").arg(program);
|
||||
return Ok(command);
|
||||
}
|
||||
|
||||
Ok(codex_utils_process::background_command(program))
|
||||
}
|
||||
|
||||
pub(crate) fn set_working_directory(command: &mut Command, state_dir: &Path) -> Result<()> {
|
||||
#[cfg(unix)]
|
||||
let cwd_missing = match std::env::current_dir() {
|
||||
@@ -90,3 +146,7 @@ pub(crate) fn set_working_directory(command: &mut Command, state_dir: &Path) ->
|
||||
command.current_dir(state_dir);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(all(test, target_os = "macos"))]
|
||||
#[path = "background_command_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
//! Behavioral coverage for managed macOS process launch policy.
|
||||
|
||||
use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_process_preserves_pid_and_uses_background_priority_when_available() {
|
||||
let command = super::managed_process(std::path::Path::new("/usr/bin/ruby"))
|
||||
.await
|
||||
.expect("managed command");
|
||||
let taskpolicy_available =
|
||||
command.get_program() == std::ffi::OsStr::new("/usr/sbin/taskpolicy");
|
||||
let mut command = tokio::process::Command::from(command);
|
||||
let mut child = command
|
||||
.args([
|
||||
"-e",
|
||||
"puts \"#{Process.pid} #{Process.getpriority(4, 0)}\"; STDOUT.flush; STDIN.read(1)",
|
||||
])
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.spawn()
|
||||
.expect("start managed process");
|
||||
let spawned_pid = child.id().expect("managed process pid");
|
||||
|
||||
child
|
||||
.stdin
|
||||
.take()
|
||||
.expect("managed process stdin")
|
||||
.write_all(b"x")
|
||||
.await
|
||||
.expect("release managed process");
|
||||
let output = child.wait_with_output().await.expect("managed process");
|
||||
|
||||
assert!(output.status.success());
|
||||
let output = String::from_utf8(output.stdout).expect("utf-8 managed process output");
|
||||
let (pid, priority) = output.trim().split_once(' ').expect("pid and priority");
|
||||
assert_eq!(pid.parse::<u32>().expect("numeric pid"), spawned_pid);
|
||||
if taskpolicy_available {
|
||||
assert_eq!(priority, "1");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn timed_out_taskpolicy_probe_falls_back_to_direct_launch() {
|
||||
let mut probe = tokio::process::Command::new("/bin/sleep");
|
||||
probe.arg("60");
|
||||
let started = Instant::now();
|
||||
let command = super::managed_process_with_probe(
|
||||
std::path::Path::new("/usr/bin/ruby"),
|
||||
probe,
|
||||
Duration::from_millis(20),
|
||||
)
|
||||
.await
|
||||
.expect("managed command");
|
||||
|
||||
assert!(started.elapsed() < Duration::from_secs(1));
|
||||
assert_eq!(command.get_program(), std::ffi::OsStr::new("/usr/bin/ruby"));
|
||||
|
||||
let output = tokio::process::Command::from(command)
|
||||
.args(["-e", "puts 'ready'"])
|
||||
.output()
|
||||
.await
|
||||
.expect("direct launch");
|
||||
assert!(output.status.success());
|
||||
assert_eq!(output.stdout, b"ready\n");
|
||||
}
|
||||
@@ -48,6 +48,11 @@ use settings::MAX_SHUTDOWN_GRACE_SECONDS;
|
||||
use tokio::time::sleep;
|
||||
|
||||
const START_POLL_INTERVAL: Duration = Duration::from_millis(50);
|
||||
// The managed process starts with background CPU and I/O policy on macOS, so
|
||||
// foreground load can legitimately delay readiness.
|
||||
#[cfg(target_os = "macos")]
|
||||
const START_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
const START_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
// Leave room for the longest graceful stop, forced-exit check, and restart.
|
||||
const OPERATION_LOCK_TIMEOUT: Duration =
|
||||
|
||||
@@ -8,6 +8,9 @@ use std::fs;
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
static DAEMON_STARTUP_SERIAL: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(1);
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn auto_daemon_start_attaches_to_shared_server() -> Result<()> {
|
||||
@@ -152,6 +155,11 @@ async fn bedrock_onboarding_leaves_a_running_daemon_untouched() -> Result<()> {
|
||||
}
|
||||
|
||||
async fn daemon_startup(command: &str) -> Result<()> {
|
||||
// Background QoS is intentionally allowed to reduce throughput. Avoid
|
||||
// making many independent daemon fixtures compete during this test binary.
|
||||
#[cfg(target_os = "macos")]
|
||||
let _serial = DAEMON_STARTUP_SERIAL.acquire().await?;
|
||||
|
||||
#[cfg(windows)]
|
||||
let elevated = codex_app_server_daemon::is_elevated()?;
|
||||
#[cfg(not(windows))]
|
||||
|
||||
Reference in New Issue
Block a user