Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/runner/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,8 @@ impl InProcessRunner {
let wait_process = process_id;
tokio::spawn(async move {
match exit.await {
// The pinned Codex PTY reports a signal death as exit code 1.
// The pinned Codex PTY reports a signal death as exit code 1, or as 128 plus the
// signal for a child that inherited descriptors, as a governed one does.
Ok(code) => wait_life
.lock()
.expect("lifecycle")
Expand Down
24 changes: 18 additions & 6 deletions crates/server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,25 @@ async fn serve() -> Result<()> {
Ok(())
}
};
drop(started);
started.stop().await;
result
}

struct StartedRunner {
runner: RuntimeBackend,
_process: Option<RuntimeProcess>,
process: Option<RuntimeProcess>,
}

impl StartedRunner {
/// Gateway shutdown ends a worker this gateway started, and with it the worker's processes,
/// before the gateway exits.
async fn stop(self) {
let Self { runner, process } = self;
drop(runner);
if let Some(mut process) = process {
process.wait_exit().await;
}
}
}

async fn start_runner(cli: &Cli, store: Arc<Store>) -> Result<StartedRunner> {
Expand All @@ -60,7 +72,7 @@ async fn start_runner(cli: &Cli, store: Arc<Store>) -> Result<StartedRunner> {
match cli.runner {
RunnerMode::InProcess => Ok(StartedRunner {
runner: RuntimeBackend::in_process(on_release),
_process: None,
process: None,
}),
RunnerMode::Uds => {
if let Some(bin) = &cli.runtime_bin {
Expand All @@ -77,15 +89,15 @@ async fn start_runner(cli: &Cli, store: Arc<Store>) -> Result<StartedRunner> {
.await?;
return Ok(StartedRunner {
runner: RuntimeBackend::Uds(runner),
_process: Some(process),
process: Some(process),
});
}
let (process, runner) =
RuntimeProcess::spawn(bin, cli.runner_dir.as_deref(), on_release, store)
.await?;
return Ok(StartedRunner {
runner: RuntimeBackend::Uds(runner),
_process: Some(process),
process: Some(process),
});
}
let socket = cli.runner_socket.as_ref().ok_or_else(|| {
Expand All @@ -96,7 +108,7 @@ async fn start_runner(cli: &Cli, store: Arc<Store>) -> Result<StartedRunner> {
let runner = RuntimeProcess::connect_existing(socket, on_release, store).await?;
Ok(StartedRunner {
runner: RuntimeBackend::Uds(runner),
_process: None,
process: None,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/server/src/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ impl CodeSpace {

#[tool(
name = "process_status",
description = "Observe a managed process lifecycle. state is running or exited. termination is present only after exit (exited, signaled, timeout, terminated, or unknown). exit_code is present only when termination is exited; signal only when it is signaled, a signal CodeSpace did not send. A tty process reports a signal death as exited with exit_code 1. EOF is not success. Unknown process_id is rejected."
description = "Observe a managed process lifecycle. state is running or exited. termination is present only after exit (exited, signaled, timeout, terminated, or unknown). exit_code is present only when termination is exited; signal only when it is signaled, a signal CodeSpace did not send. A tty process reports a signal death as exited with exit_code 1, or 128 plus the signal number when it was launched through DevGuard. EOF is not success. Unknown process_id is rejected."
)]
async fn process_status(
&self,
Expand Down
96 changes: 92 additions & 4 deletions crates/server/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub struct RuntimeProcess {
dir: PathBuf,
socket: PathBuf,
pid: Option<u32>,
/// The gateway's connection to the worker, which `wait_exit` closes.
runner: UdsRunner,
shutdown: Arc<Notify>,
wait: Option<JoinHandle<()>>,
}
Expand Down Expand Up @@ -74,10 +76,7 @@ impl RuntimeProcess {
let wait = tokio::spawn(async move {
tokio::select! {
_ = child.wait() => {}
_ = wait_shutdown.notified() => {
let _ = child.start_kill();
let _ = child.wait().await;
}
_ = wait_shutdown.notified() => stop_worker(&mut child).await,
}
wait_store.release_all_processes();
});
Expand All @@ -99,6 +98,7 @@ impl RuntimeProcess {
dir,
socket,
pid,
runner: runner.clone(),
shutdown,
wait: Some(wait),
},
Expand Down Expand Up @@ -141,7 +141,12 @@ impl RuntimeProcess {
&self.dir
}

/// Stop the worker and wait for it to exit. Its connection closes first, so the worker ends
/// its processes as it exits; one still running after `WORKER_EXIT_GRACE` is killed.
pub async fn wait_exit(&mut self) {
// Closing waits for a write in progress, which a worker that no longer reads would hold
// up, so it gets the same bound.
let _ = tokio::time::timeout(WORKER_EXIT_GRACE, self.runner.close()).await;
self.shutdown.notify_one();
if let Some(wait) = self.wait.take() {
let _ = wait.await;
Expand Down Expand Up @@ -178,6 +183,24 @@ pub(crate) fn worker_command(bin: &Path, socket: &Path) -> Command {
command
}

/// How long a worker whose connection has closed may take to end its processes and exit before
/// the gateway kills it. Its processes end as it exits, as an in-process runner's do when the
/// gateway exits; a worker killed outright would leave its pipe processes running, as nothing
/// gives them a parent-death signal.
const WORKER_EXIT_GRACE: Duration = Duration::from_secs(5);

/// The worker exits by itself once its connection has closed (or has been lost): it is given
/// `WORKER_EXIT_GRACE` for that, then killed.
async fn stop_worker(child: &mut tokio::process::Child) {
if tokio::time::timeout(WORKER_EXIT_GRACE, child.wait())
.await
.is_err()
{
let _ = child.start_kill();
let _ = child.wait().await;
}
}

/// How long the gateway waits for a started worker's socket. A worker binary never run before
/// can take macOS a few seconds to start, as it checks a new executable file on its first exec.
const WORKER_READY_BOUND: Duration = Duration::from_secs(10);
Expand Down Expand Up @@ -507,6 +530,71 @@ mod tests {
assert!(!dir.exists() || std::fs::read_dir(&dir).map(|d| d.count()).unwrap_or(0) == 0);
}

/// Gateway shutdown ends the worker's processes (docs/runner-isolation.md): its connection
/// closes, so the worker ends a pipe and a PTY process as it exits, well before it would be
/// killed. A killed worker would leave the pipe process running.
#[tokio::test]
async fn a_stopped_worker_ends_its_processes() {
use codespace_domain::{ProcessId, Profile, WorkspaceId};
use codespace_policy::Workspace;
use codespace_runner::{Runner, RunnerExecRequest};

let Some(bin) = runtime_bin() else {
return;
};
let dir = tempfile::tempdir().unwrap();
let ws = Workspace::new(
WorkspaceId("demo".into()),
dir.path().to_path_buf(),
Profile::WorkspaceWrite,
);
let store = Arc::new(Store::memory().unwrap());
let (mut proc, runner) = RuntimeProcess::spawn(&bin, None, Arc::new(|_| {}), store)
.await
.expect("spawn runtime");
// Each process counts into its file until it ends.
let mut counts = Vec::new();
for (name, tty) in [("pipe", false), ("pty", true)] {
let count = dir.path().join(name);
let script = format!(
"n=0; while :; do n=$((n+1)); echo $n > '{}'; /bin/sleep 0.1; done",
count.display()
);
let mut req = RunnerExecRequest::for_host(
vec!["/bin/sh".into(), "-c".into(), script],
ProcessId(format!("proc-stop-{name}")),
Profile::WorkspaceWrite,
);
req.tty = tty;
runner.exec(&ws, req).await.expect("exec");
counts.push(count);
}
let deadline = std::time::Instant::now() + Duration::from_secs(10);
while !counts.iter().all(|count| count.exists()) {
assert!(
std::time::Instant::now() < deadline,
"the processes never ran"
);
tokio::time::sleep(Duration::from_millis(20)).await;
}
let asked = std::time::Instant::now();
proc.wait_exit().await;
let stopped = asked.elapsed();
assert!(
stopped < WORKER_EXIT_GRACE,
"the worker did not exit by itself: {stopped:?}"
);
let read = || -> Vec<String> {
counts
.iter()
.map(|count| std::fs::read_to_string(count).unwrap())
.collect()
};
let at_exit = read();
tokio::time::sleep(Duration::from_millis(1500)).await;
assert_eq!(read(), at_exit, "a process outlived its worker");
}

/// The wait for a worker's socket is long enough for a first exec, but a worker that exits
/// first fails the start at once.
#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion docs/ko/runner-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

게이트웨이는 임시 디렉터리 또는 `CODESPACE_RUNNER_DIR` 아래에 권한 0700의 고유 디렉터리를 만듭니다. 내부 통신은 u32 길이 접두부, 버전 7 핸드셰이크, 요청 ID, 프로세스 종료 이벤트를 사용하는 CodeSpace JSON입니다. Codex App Server RPC가 아닙니다. 같은 연결에서의 요청 재처리는 재접속 복구를 뜻하지 않습니다.

관리 프로세스의 수명은 MCP 연결이 아니라 러너 인스턴스가 소유합니다. Streamable HTTP나 MCP 클라이언트 끊김은 프로세스를 유지합니다. 게이트웨이와 worker의 UDS 연결이 끊기거나 게이트웨이가 종료되면(stdio EOF 포함) 관리 중인 worker와 자식 프로세스가 종료되고 핸들이 사라집니다. 재시작은 `process_id`를 복구하지 않습니다.
관리 프로세스의 수명은 MCP 연결이 아니라 러너 인스턴스가 소유합니다. Streamable HTTP나 MCP 클라이언트 끊김은 프로세스를 유지합니다. 게이트웨이와 worker의 UDS 연결이 끊기거나 게이트웨이가 종료되면(stdio EOF 포함) 관리 중인 worker와 자식 프로세스가 종료되고 핸들이 사라집니다. 종료할 때 게이트웨이는 worker와의 연결을 닫으므로 worker는 종료하면서 자기 프로세스를 끝내고, 5초 뒤에도 실행 중인 worker는 강제로 종료됩니다. 재시작은 `process_id`를 복구하지 않습니다.

**자식 프로세스를 시작하는 방식.** 파이프 명령, `tty: true` 명령, 패치 도우미, Linux 샌드박스 도우미, worker는 fork와 exec로 시작합니다. 모두 exec 전에 자식 프로세스 안에서 실행할 단계가 있기 때문입니다. 파이프, 패치 도우미, 샌드박스 도우미, worker 자식은 표준 디스크립터만 남기고([#79](https://github.com/novelKR/CodeSpace/issues/79)), PTY 자식은 새 세션의 리더가 됩니다. macOS에서는 같은 프로세스의 다른 스레드가 시스템 알림 라이브러리(libnotify)를 초기화하는 도중에 fork한 자식이 명령을 실행하기도 전에 `fork` 안에서 신호를 받아 종료됩니다. `process_status`는 이렇게 종료된 파이프 명령을 `signaled`로, `tty: true` 명령을 종료 코드 1로 끝난 것으로 보고합니다([Agent Loop 연동](agent-integration.md)). 그래서 `codespace-mcp`와 `codespace-codex-runtime`은 스레드를 시작하기 전에 이 초기화를 마치고, 위의 spawn은 모두 fork하기 전에 초기화를 마칩니다. 이 조치는 이 원인 하나를 없앨 뿐이며, 다중 스레드 프로세스에서 fork하는 것을 일반적으로 안전하게 만들지는 않습니다.

Expand Down
2 changes: 1 addition & 1 deletion docs/runner-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ There are three separate questions: which process runs the work, whether command

The gateway creates a unique 0700 directory beneath its temporary directory or `CODESPACE_RUNNER_DIR`. The internal protocol is CodeSpace JSON with a u32 length prefix, handshake version 7, request IDs, and process-exit events. It is not Codex App Server RPC. Same-connection replay is not reconnect recovery.

Managed process lifetime is owned by the runner instance, not the MCP connection. Streamable HTTP or MCP client disconnect keeps processes running. Gateway/worker UDS disconnect or gateway shutdown (including stdio EOF) ends the owned worker and its children; process handles are lost. Restart does not restore `process_id`.
Managed process lifetime is owned by the runner instance, not the MCP connection. Streamable HTTP or MCP client disconnect keeps processes running. Gateway/worker UDS disconnect or gateway shutdown (including stdio EOF) ends the owned worker and its children; process handles are lost. At shutdown the gateway closes the worker's connection, so the worker ends its processes as it exits; a worker still running 5 seconds later is killed. Restart does not restore `process_id`.

**How children start.** Pipe commands, `tty: true` commands, the patch helper, the Linux sandbox helper and the worker are started with fork and exec, because each runs a step in the child before exec: pipe, patch-helper, sandbox-helper and worker children keep only their standard descriptors ([#79](https://github.com/novelKR/CodeSpace/issues/79)), and a PTY child becomes the leader of a new session. On macOS, a child forked while another thread of the same process is initializing the system's notification library (libnotify) is killed by a signal inside `fork`, before the command runs. `process_status` reports a pipe command killed this way as `signaled`, and a `tty: true` command as an exit with code 1 ([Agent Loop integration](agent-integration.md)). So `codespace-mcp` and `codespace-codex-runtime` complete that initialization before they start any thread, and every one of these spawns completes it before it forks. This removes that one cause; it does not make forking from a multithreaded process safe in general.

Expand Down
4 changes: 2 additions & 2 deletions docs/translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@
"컨테이너-실험-구성과-검증",
"호스트와-worker-실행"
],
"source_sha256": "008e7c73e34c649962d3c61e6e59be0cd869199c8cf318ccc064b277b96c831e",
"translation_sha256": "931a196e8d75240874bad68981d3e75cd3a4d812a3740e2e2187e53891b729f7"
"source_sha256": "50fe627ce573f1b77b4a35a71a83d897d06ad62f17077425d36d01b32f8f51e4",
"translation_sha256": "69ac617af227ade40277d11c48dccda71d287caee9a0e0a6357eeddaa5da6c26"
},
{
"id": "error-codes",
Expand Down
Loading