Detach non-tty child spawns

This commit is contained in:
Alex Kotliarskyi
2026-01-16 16:35:39 -08:00
parent c26fe64539
commit 0226fa6df9
7 changed files with 86 additions and 14 deletions

View File

@@ -549,6 +549,7 @@ async fn exec(
))
})?;
let arg0_ref = arg0.as_deref();
let detach_from_tty = matches!(sandbox, SandboxType::None);
let child = spawn_child_async(
PathBuf::from(program),
args.into(),
@@ -557,6 +558,7 @@ async fn exec(
sandbox_policy,
StdioPolicy::RedirectForShellTool,
env,
detach_from_tty,
)
.await?;
consume_truncated_output(child, expiration, stdout_stream).await

View File

@@ -35,6 +35,7 @@ where
sandbox_policy,
stdio_policy,
env,
false,
)
.await
}

View File

@@ -39,6 +39,7 @@ pub async fn spawn_command_under_seatbelt(
sandbox_policy,
stdio_policy,
env,
false,
)
.await
}

View File

@@ -35,6 +35,7 @@ pub enum StdioPolicy {
/// For now, we take `SandboxPolicy` as a parameter to spawn_child() because
/// we need to determine whether to set the
/// `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` environment variable.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn spawn_child_async(
program: PathBuf,
args: Vec<String>,
@@ -43,6 +44,7 @@ pub(crate) async fn spawn_child_async(
sandbox_policy: &SandboxPolicy,
stdio_policy: StdioPolicy,
env: HashMap<String, String>,
detach_from_tty: bool,
) -> std::io::Result<Child> {
trace!(
"spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {sandbox_policy:?} {stdio_policy:?} {env:?}"
@@ -66,12 +68,16 @@ pub(crate) async fn spawn_child_async(
#[cfg(unix)]
unsafe {
let set_process_group = matches!(stdio_policy, StdioPolicy::RedirectForShellTool);
let isolate_process_group = matches!(stdio_policy, StdioPolicy::RedirectForShellTool);
#[cfg(target_os = "linux")]
let parent_pid = libc::getpid();
cmd.pre_exec(move || {
if set_process_group {
codex_utils_pty::process_group::set_process_group()?;
if isolate_process_group {
if detach_from_tty {
codex_utils_pty::process_group::detach_from_tty()?;
} else {
codex_utils_pty::process_group::set_process_group()?;
}
}
// This relies on prctl(2), so it only works on Linux.

View File

@@ -10,6 +10,7 @@ use tokio::time::Duration;
use tokio::time::Instant;
use tokio_util::sync::CancellationToken;
use crate::exec::SandboxType;
use crate::exec_env::create_env;
use crate::protocol::ExecCommandSource;
use crate::sandboxing::ExecEnv;
@@ -470,14 +471,26 @@ impl UnifiedExecProcessManager {
)
.await
} else {
codex_utils_pty::pipe::spawn_process_no_stdin(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
let detach_from_tty = matches!(env.sandbox, SandboxType::None);
if detach_from_tty {
codex_utils_pty::pipe::spawn_process_no_stdin_detached(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
} else {
codex_utils_pty::pipe::spawn_process_no_stdin(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
}
};
let spawned =
spawn_result.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;

View File

@@ -103,6 +103,7 @@ async fn spawn_process_with_stdin_mode(
env: &HashMap<String, String>,
arg0: &Option<String>,
stdin_mode: PipeStdinMode,
detach_from_tty: bool,
) -> Result<SpawnedProcess> {
if program.is_empty() {
anyhow::bail!("missing program for pipe spawn");
@@ -118,7 +119,11 @@ async fn spawn_process_with_stdin_mode(
#[cfg(unix)]
unsafe {
command.pre_exec(move || {
crate::process_group::set_process_group()?;
if detach_from_tty {
crate::process_group::detach_from_tty()?;
} else {
crate::process_group::set_process_group()?;
}
#[cfg(target_os = "linux")]
crate::process_group::set_parent_death_signal(parent_pid)?;
Ok(())
@@ -253,7 +258,7 @@ pub async fn spawn_process(
env: &HashMap<String, String>,
arg0: &Option<String>,
) -> Result<SpawnedProcess> {
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped).await
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped, false).await
}
/// Spawn a process using regular pipes, but close stdin immediately.
@@ -264,5 +269,27 @@ pub async fn spawn_process_no_stdin(
env: &HashMap<String, String>,
arg0: &Option<String>,
) -> Result<SpawnedProcess> {
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null).await
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null, false).await
}
/// Spawn a process using regular pipes, but detach it from the controlling TTY.
pub async fn spawn_process_detached(
program: &str,
args: &[String],
cwd: &Path,
env: &HashMap<String, String>,
arg0: &Option<String>,
) -> Result<SpawnedProcess> {
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped, true).await
}
/// Spawn a process using regular pipes, close stdin immediately, and detach it from the TTY.
pub async fn spawn_process_no_stdin_detached(
program: &str,
args: &[String],
cwd: &Path,
env: &HashMap<String, String>,
arg0: &Option<String>,
) -> Result<SpawnedProcess> {
spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null, true).await
}

View File

@@ -4,6 +4,8 @@
//! command can be cleaned up reliably:
//! - `set_process_group` is called in `pre_exec` so the child starts its own
//! process group.
//! - `detach_from_tty` starts a new session so non-interactive children do not
//! inherit the controlling TTY.
//! - `kill_process_group_by_pid` targets the whole group (children/grandchildren)
//! - `kill_process_group` targets a known process group ID directly
//! instead of a single PID.
@@ -42,6 +44,26 @@ pub fn set_parent_death_signal(_parent_pid: i32) -> io::Result<()> {
Ok(())
}
#[cfg(unix)]
/// Detach from the controlling TTY by starting a new session.
pub fn detach_from_tty() -> io::Result<()> {
let result = unsafe { libc::setsid() };
if result == -1 {
let err = io::Error::last_os_error();
if err.raw_os_error() == Some(libc::EPERM) {
return set_process_group();
}
return Err(err);
}
Ok(())
}
#[cfg(not(unix))]
/// No-op on non-Unix platforms.
pub fn detach_from_tty() -> io::Result<()> {
Ok(())
}
#[cfg(unix)]
/// Put the calling process into its own process group.
///