From 549dc724091b0ca1e95f7c416f7a9d4ac3dbb5a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 01:51:40 +0000 Subject: [PATCH 1/2] fix(tui): end the Linux sleep inhibitor's command through a pipe, not a signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `systemd-inhibit` holds the idle lock around a child of its own, and the guard released it with SIGKILL — which is never forwarded — so every interactive turn on Linux left a `sleep infinity` orphan behind once the inhibitor died (the lock itself was released; the process was not). The command is now `cat` reading a pipe the guard holds: dropping the guard closes the pipe, `cat` exits on EOF, and `systemd-inhibit` follows, with `kill_on_drop` still sending the release signal immediately. A Linux test lists the inhibitor's children before the drop and asserts none outlives it; without logind the inhibitor exits at once and the list is empty, so the test bites where an inhibitor actually runs. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0134iUMxmGuXiG1LzPgfZVnv --- crates/tui/src/sleep_guard.rs | 48 ++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/crates/tui/src/sleep_guard.rs b/crates/tui/src/sleep_guard.rs index ad9f811d4f..6290d6abf4 100644 --- a/crates/tui/src/sleep_guard.rs +++ b/crates/tui/src/sleep_guard.rs @@ -32,6 +32,11 @@ //! `kill_on_drop` sends the release signal, and the runtime reaps the child — //! no `wait` runs inline on a worker (#6149). `hold` therefore has to be //! called from within a Tokio runtime context. +//! +//! On Linux the lock is held by `systemd-inhibit` around a child of its own, +//! and a kill is never forwarded to that grandchild. The command is therefore +//! `cat` reading a pipe this guard holds: dropping the guard closes the pipe, +//! `cat` exits on EOF, and `systemd-inhibit` follows — nothing is left behind. #[cfg(unix)] use tokio::process::Child; @@ -95,7 +100,9 @@ fn start_inhibitor() -> Option { } /// `--what=idle` only: an explicit suspend or a closed lid is still honoured. -/// `sleep infinity` is the command whose lifetime holds the block open. +/// `cat` on the guard's pipe is the command whose lifetime holds the block +/// open: it exits on EOF when the guard drops, which no signal sent to +/// `systemd-inhibit` could make a `sleep infinity` grandchild do. #[cfg(target_os = "linux")] fn start_inhibitor() -> Option { spawn( @@ -104,8 +111,7 @@ fn start_inhibitor() -> Option { "--what=idle", "--why=Codewhale turn in flight", "--mode=block", - "sleep", - "infinity", + "cat", ], ) } @@ -120,7 +126,9 @@ fn start_inhibitor() -> Option { fn spawn(program: &str, args: &[&str]) -> Option { Command::new(program) .args(args) - .stdin(Stdio::null()) + // The pipe is never written to: closing it when the guard drops is + // what ends an inhibitor's own child (see the Linux inhibitor). + .stdin(Stdio::piped()) .stdout(Stdio::null()) .stderr(Stdio::null()) // Killing the inhibitor is what releases the assertion; the runtime @@ -173,6 +181,38 @@ mod tests { ); } + /// Linux: `systemd-inhibit` holds the lock around a child of its own and a + /// kill never reaches that grandchild — the guard's pipe is what ends it. + /// Without logind the inhibitor exits at once and the list is empty, so + /// this proves something only where an inhibitor really runs. + #[tokio::test] + #[cfg(target_os = "linux")] + async fn a_released_guard_leaves_no_grandchild_behind() { + let guard = SleepGuard::hold(); + let pid = guard + .inhibitor_pid() + .expect("this platform starts an inhibitor"); + // Give the inhibitor a moment to fork its command. + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + let grandchildren: Vec = + tokio::fs::read_to_string(format!("/proc/{pid}/task/{pid}/children")) + .await + .unwrap_or_default() + .split_whitespace() + .filter_map(|child| child.parse().ok()) + .collect(); + + drop(guard); + + assert!(released(pid).await, "the inhibitor itself must be gone"); + for grandchild in grandchildren { + assert!( + released(grandchild).await, + "process {grandchild} outlived the guard: the inhibitor's command must end with the guard's pipe" + ); + } + } + #[tokio::test] #[cfg(any(target_os = "macos", target_os = "linux"))] async fn holding_twice_holds_two_independent_inhibitors() { From dfa989115c019cface460a2c7f8cae61ca415789 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 01:51:40 +0000 Subject: [PATCH 2/2] fix(runtime-api): bound how many terminal route calls occupy blocking threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `with_session` moved every terminal route onto the blocking pool, but an unbounded number of them: one write to a child that stopped reading holds the session lock indefinitely, and each further call — including ones whose client has since disconnected, since a started blocking task cannot be cancelled — would occupy another pool thread waiting on that lock, until the runtime's unrelated blocking work stalled too. A static semaphore now admits eight session touches at a time; the rest wait asynchronously in the handler, where a disconnect simply drops them. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0134iUMxmGuXiG1LzPgfZVnv --- crates/tui/src/runtime_api/terminal.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/tui/src/runtime_api/terminal.rs b/crates/tui/src/runtime_api/terminal.rs index 9faf040891..2d1abccc5e 100644 --- a/crates/tui/src/runtime_api/terminal.rs +++ b/crates/tui/src/runtime_api/terminal.rs @@ -198,12 +198,21 @@ fn open_session( .ok_or_else(|| ApiError::not_found(format!("no live terminal session named '{name}'"))) } +/// How many terminal route calls may occupy blocking threads at once. The +/// rest wait in `with_session` asynchronously, so a client that disconnects +/// while queued simply disappears instead of holding a pool thread, and one +/// stuck write (a child that stopped reading, lock held) can stall terminal +/// routes but never the runtime's other blocking work. +#[cfg(all(unix, not(target_env = "ohos")))] +static ROUTE_GATE: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(8); + /// Run one operation against the locked session on the blocking pool. /// /// The session mutex and the PTY behind it are synchronous: the agent's own /// tool holds the lock across a whole command, and a write to a child that /// stopped reading blocks until the kernel buffer drains. Neither may park a -/// runtime worker (#6149), so a route never touches the session inline. +/// runtime worker (#6149), so a route never touches the session inline, and +/// `ROUTE_GATE` bounds how many such touches can be in flight. #[cfg(all(unix, not(target_env = "ohos")))] async fn with_session( session: terminal_session::SharedSession, @@ -214,7 +223,12 @@ async fn with_session( where T: Send + 'static, { + let permit = ROUTE_GATE + .acquire() + .await + .map_err(|_| ApiError::internal("terminal route gate closed"))?; tokio::task::spawn_blocking(move || { + let _permit = permit; let mut guard = session .lock() .map_err(|_| ApiError::internal("terminal session lock poisoned"))?;