From b74e5c24e0435619f9bff62728c9f319e90be6c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 11:03:04 +0000 Subject: [PATCH] server: end the uds worker's processes at gateway shutdown (CSRG-U6) At shutdown the gateway killed its worker outright, so the worker never ended its executions, and nothing gives a pipe execution a parent-death signal on macOS. The CS-RG harness's uds shutdown case found a governed `sleep 30` outliving the gateway on the qualification Mac. The gateway now closes the worker's connection and waits for the worker, which ends its processes as it exits, as an in-process runner's processes end when the gateway exits. A worker still running 5 seconds later is killed; the close is bounded by the same grace. main stops the runner it started before it returns instead of dropping it. The process_status description and a runner comment now say that a tty process launched through DevGuard reports a signal death as 128 plus the signal, as the harness found on the Mac. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/runner/src/process.rs | 3 +- crates/server/src/main.rs | 24 ++++++--- crates/server/src/mcp.rs | 2 +- crates/server/src/runtime.rs | 96 ++++++++++++++++++++++++++++++++++-- docs/ko/runner-isolation.md | 2 +- docs/runner-isolation.md | 2 +- docs/translations.json | 4 +- 7 files changed, 117 insertions(+), 16 deletions(-) diff --git a/crates/runner/src/process.rs b/crates/runner/src/process.rs index a98710b..6a289f9 100644 --- a/crates/runner/src/process.rs +++ b/crates/runner/src/process.rs @@ -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") diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 26554e9..2b214e8 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -41,13 +41,25 @@ async fn serve() -> Result<()> { Ok(()) } }; - drop(started); + started.stop().await; result } struct StartedRunner { runner: RuntimeBackend, - _process: Option, + process: Option, +} + +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) -> Result { @@ -60,7 +72,7 @@ async fn start_runner(cli: &Cli, store: Arc) -> Result { 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 { @@ -77,7 +89,7 @@ async fn start_runner(cli: &Cli, store: Arc) -> Result { .await?; return Ok(StartedRunner { runner: RuntimeBackend::Uds(runner), - _process: Some(process), + process: Some(process), }); } let (process, runner) = @@ -85,7 +97,7 @@ async fn start_runner(cli: &Cli, store: Arc) -> Result { .await?; return Ok(StartedRunner { runner: RuntimeBackend::Uds(runner), - _process: Some(process), + process: Some(process), }); } let socket = cli.runner_socket.as_ref().ok_or_else(|| { @@ -96,7 +108,7 @@ async fn start_runner(cli: &Cli, store: Arc) -> Result { let runner = RuntimeProcess::connect_existing(socket, on_release, store).await?; Ok(StartedRunner { runner: RuntimeBackend::Uds(runner), - _process: None, + process: None, }) } } diff --git a/crates/server/src/mcp.rs b/crates/server/src/mcp.rs index 14883a2..3cd4514 100644 --- a/crates/server/src/mcp.rs +++ b/crates/server/src/mcp.rs @@ -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, diff --git a/crates/server/src/runtime.rs b/crates/server/src/runtime.rs index 07fbc1b..80d6ee9 100644 --- a/crates/server/src/runtime.rs +++ b/crates/server/src/runtime.rs @@ -17,6 +17,8 @@ pub struct RuntimeProcess { dir: PathBuf, socket: PathBuf, pid: Option, + /// The gateway's connection to the worker, which `wait_exit` closes. + runner: UdsRunner, shutdown: Arc, wait: Option>, } @@ -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(); }); @@ -99,6 +98,7 @@ impl RuntimeProcess { dir, socket, pid, + runner: runner.clone(), shutdown, wait: Some(wait), }, @@ -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; @@ -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); @@ -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 { + 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] diff --git a/docs/ko/runner-isolation.md b/docs/ko/runner-isolation.md index 45409b4..f76f334 100644 --- a/docs/ko/runner-isolation.md +++ b/docs/ko/runner-isolation.md @@ -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하는 것을 일반적으로 안전하게 만들지는 않습니다. diff --git a/docs/runner-isolation.md b/docs/runner-isolation.md index c27ee98..1b80daf 100644 --- a/docs/runner-isolation.md +++ b/docs/runner-isolation.md @@ -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. diff --git a/docs/translations.json b/docs/translations.json index c46fc08..72c33b0 100644 --- a/docs/translations.json +++ b/docs/translations.json @@ -553,8 +553,8 @@ "컨테이너-실험-구성과-검증", "호스트와-worker-실행" ], - "source_sha256": "008e7c73e34c649962d3c61e6e59be0cd869199c8cf318ccc064b277b96c831e", - "translation_sha256": "931a196e8d75240874bad68981d3e75cd3a4d812a3740e2e2187e53891b729f7" + "source_sha256": "50fe627ce573f1b77b4a35a71a83d897d06ad62f17077425d36d01b32f8f51e4", + "translation_sha256": "69ac617af227ade40277d11c48dccda71d287caee9a0e0a6357eeddaa5da6c26" }, { "id": "error-codes",