From 601367b1bea81b25cc5535107675fd152157ac01 Mon Sep 17 00:00:00 2001 From: James Long Date: Fri, 30 Oct 2026 15:50:52 -0400 Subject: [PATCH 1/3] fix(process-daemon): stop cleanly on SIGTERM and SIGINT --- packages/process-daemon/Cargo.toml | 2 +- packages/process-daemon/src/daemon/server.rs | 35 ++++++++++++++++ packages/process-daemon/tests/ownership.rs | 42 ++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/packages/process-daemon/Cargo.toml b/packages/process-daemon/Cargo.toml index 35cc8547a8d7..698bbe1096e0 100644 --- a/packages/process-daemon/Cargo.toml +++ b/packages/process-daemon/Cargo.toml @@ -21,7 +21,7 @@ shell-words = "1.1" [target.'cfg(unix)'.dependencies] libc = "0.2" -nix = { version = "0.28", features = ["poll", "process", "term", "user"] } +nix = { version = "0.28", features = ["poll", "process", "signal", "term", "user"] } sha2 = "0.10" [target.'cfg(windows)'.dependencies] diff --git a/packages/process-daemon/src/daemon/server.rs b/packages/process-daemon/src/daemon/server.rs index 7f07ebaee8cf..6ecb44145102 100644 --- a/packages/process-daemon/src/daemon/server.rs +++ b/packages/process-daemon/src/daemon/server.rs @@ -24,9 +24,44 @@ pub fn run(directory: &Path) -> Result<()> { let (runtime, listener) = platform::Runtime::bind(directory)?; // Created after bind so bind I/O does not consume the acquire window. let control = Arc::new(Control::new(wake, Ownership::new(Instant::now()))); + #[cfg(unix)] + stop_on_signals(&control)?; serve(runtime, listener, control, Settings::default()) } +/// Turns SIGTERM and SIGINT into an orderly stop, so children are killed and the +/// runtime directory is removed instead of the daemon dying with both left behind. +/// +/// The signals are blocked on this thread before any worker exists, so every later +/// thread inherits the mask and only the waiter receives them. Children never +/// inherit it: std and portable-pty reset the signal mask before `exec`. +#[cfg(unix)] +fn stop_on_signals(control: &Arc) -> Result<()> { + use nix::sys::signal::{SigSet, Signal}; + + let mut signals = SigSet::empty(); + signals.add(Signal::SIGTERM); + signals.add(Signal::SIGINT); + signals + .thread_block() + .context("failed to block stop signals")?; + let control = Arc::clone(control); + thread::Builder::new() + .name("opencode-pty-signals".into()) + .spawn(move || { + if signals.wait().is_err() { + return; + } + control.request_stop(); + // A second signal means the orderly stop is stuck; exit like the default action. + if let Ok(signal) = signals.wait() { + std::process::exit(128 + signal as i32); + } + }) + .context("failed to start the signal thread")?; + Ok(()) +} + struct Settings { /// Longest the loop sleeps without a wake or deadline. safety_net: Duration, diff --git a/packages/process-daemon/tests/ownership.rs b/packages/process-daemon/tests/ownership.rs index f74f162c5cbe..157e0f8c4b6d 100644 --- a/packages/process-daemon/tests/ownership.rs +++ b/packages/process-daemon/tests/ownership.rs @@ -536,3 +536,45 @@ fn shutdown_keeps_a_replaced_socket() { drop(replacement); std::fs::remove_file(&daemon.registration.socket).unwrap(); } + +#[test] +fn stop_signals_stop_terminals_and_clean_up() { + for signal in [libc::SIGTERM, libc::SIGINT] { + let mut daemon = Daemon::start(); + let (_owner, response) = daemon.own(None); + assert!(matches!(response, Response::Owned)); + let terminal = daemon.terminal("exec sleep 1000"); + + // The daemon blocks these signals for its own threads; its children must not + // inherit that mask, so a terminal's process still dies from SIGTERM. + let probe = daemon.terminal("exec sleep 1000"); + // SAFETY: the probe PID belongs to this test's live terminal. + assert_eq!( + unsafe { libc::kill(probe.pid.unwrap() as i32, libc::SIGTERM) }, + 0 + ); + let deadline = Instant::now() + Duration::from_secs(3); + loop { + let Response::Terminals { terminals } = daemon.request(Request::List) else { + panic!("list failed"); + }; + let exited = terminals.iter().any(|value| { + value.id == probe.id + && !matches!( + value.lifecycle, + opencode_pty::service::TerminalLifecycle::Running + ) + }); + if exited { + break; + } + assert!(Instant::now() < deadline, "probe ignored SIGTERM"); + thread::sleep(Duration::from_millis(10)); + } + + // SAFETY: the PID belongs to the daemon this test spawned. + assert_eq!(unsafe { libc::kill(daemon.child.id() as i32, signal) }, 0); + daemon.wait(); + assert_terminal_stopped(&terminal); + } +} From c540d8ff1ebef0a72edad443d39d44a4d74fbe78 Mon Sep 17 00:00:00 2001 From: James Long Date: Fri, 30 Oct 2026 16:05:20 -0400 Subject: [PATCH 2/3] fix(process-daemon): block stop signals before startup threads and avoid nix sigwait --- packages/process-daemon/Cargo.toml | 2 +- packages/process-daemon/src/daemon/server.rs | 122 ++++++++++++++----- packages/process-daemon/tests/ownership.rs | 11 +- 3 files changed, 99 insertions(+), 36 deletions(-) diff --git a/packages/process-daemon/Cargo.toml b/packages/process-daemon/Cargo.toml index 698bbe1096e0..35cc8547a8d7 100644 --- a/packages/process-daemon/Cargo.toml +++ b/packages/process-daemon/Cargo.toml @@ -21,7 +21,7 @@ shell-words = "1.1" [target.'cfg(unix)'.dependencies] libc = "0.2" -nix = { version = "0.28", features = ["poll", "process", "signal", "term", "user"] } +nix = { version = "0.28", features = ["poll", "process", "term", "user"] } sha2 = "0.10" [target.'cfg(windows)'.dependencies] diff --git a/packages/process-daemon/src/daemon/server.rs b/packages/process-daemon/src/daemon/server.rs index 6ecb44145102..8c7ce9130683 100644 --- a/packages/process-daemon/src/daemon/server.rs +++ b/packages/process-daemon/src/daemon/server.rs @@ -19,47 +19,113 @@ const ACCEPT_BACKOFF: Duration = Duration::from_millis(100); const MAX_ACCEPTS_PER_PASS: usize = 64; pub fn run(directory: &Path) -> Result<()> { + // First, before bind starts the sweeper thread: every thread created after + // this inherits the mask, so only the signal thread ever receives them. + #[cfg(unix)] + let signals = signals::block()?; // Before bind, so a failure here leaves nothing to clean up. let wake = Wake::new()?; let (runtime, listener) = platform::Runtime::bind(directory)?; // Created after bind so bind I/O does not consume the acquire window. let control = Arc::new(Control::new(wake, Ownership::new(Instant::now()))); #[cfg(unix)] - stop_on_signals(&control)?; + if let Err(error) = signals::stop_on(signals, &control) { + let _ = runtime.finish(); + return Err(error); + } serve(runtime, listener, control, Settings::default()) } -/// Turns SIGTERM and SIGINT into an orderly stop, so children are killed and the -/// runtime directory is removed instead of the daemon dying with both left behind. +/// Turns SIGTERM and SIGINT into an orderly stop, so children are killed and +/// the runtime directory is removed instead of the daemon dying with both left +/// behind. /// -/// The signals are blocked on this thread before any worker exists, so every later -/// thread inherits the mask and only the waiter receives them. Children never -/// inherit it: std and portable-pty reset the signal mask before `exec`. +/// Children inherit the signal mask across `exec`. portable-pty resets it for +/// PTY children; anything else the daemon spawns must reset it itself. #[cfg(unix)] -fn stop_on_signals(control: &Arc) -> Result<()> { - use nix::sys::signal::{SigSet, Signal}; - - let mut signals = SigSet::empty(); - signals.add(Signal::SIGTERM); - signals.add(Signal::SIGINT); - signals - .thread_block() - .context("failed to block stop signals")?; - let control = Arc::clone(control); - thread::Builder::new() - .name("opencode-pty-signals".into()) - .spawn(move || { - if signals.wait().is_err() { - return; +mod signals { + use std::io; + use std::sync::Arc; + use std::thread; + use std::time::{Duration, Instant}; + + use anyhow::{Context, Result}; + + use super::Control; + + const STOP: [libc::c_int; 2] = [libc::SIGTERM, libc::SIGINT]; + /// A second signal this soon after the first is part of the same stop + /// (Ctrl-C plus a wrapper's SIGTERM), not a sign that the stop is stuck. + const REPEAT_GRACE: Duration = Duration::from_secs(1); + + pub(super) struct Blocked(libc::sigset_t); + + // SAFETY: a sigset_t is plain data. + unsafe impl Send for Blocked {} + + /// Blocks the stop signals on the calling thread, and restores their + /// default action: macOS never delivers an ignored signal to `sigwait`, + /// and a launcher may have left SIGINT ignored. + pub(super) fn block() -> Result { + // SAFETY: plain libc calls on a local, initialized signal set. + unsafe { + let mut set: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset(&mut set); + for signal in STOP { + libc::sigaddset(&mut set, signal); } - control.request_stop(); - // A second signal means the orderly stop is stuck; exit like the default action. - if let Ok(signal) = signals.wait() { - std::process::exit(128 + signal as i32); + let result = libc::pthread_sigmask(libc::SIG_BLOCK, &set, std::ptr::null_mut()); + if result != 0 { + return Err(io::Error::from_raw_os_error(result)) + .context("failed to block stop signals"); } - }) - .context("failed to start the signal thread")?; - Ok(()) + // Only after blocking, so there is no window where the default + // action could kill the daemon. + for signal in STOP { + libc::signal(signal, libc::SIG_DFL); + } + Ok(Blocked(set)) + } + } + + pub(super) fn stop_on(signals: Blocked, control: &Arc) -> Result<()> { + let control = Arc::clone(control); + thread::Builder::new() + .name("opencode-pty-signals".into()) + .spawn(move || { + wait(&signals); + let stopped = Instant::now(); + control.request_stop(); + loop { + let signal = wait(&signals); + if stopped.elapsed() >= REPEAT_GRACE { + // The orderly stop is stuck. Exit without running exit + // handlers, which other threads may be blocking. + // SAFETY: `_exit` is always safe to call. + unsafe { libc::_exit(128 + signal) }; + } + } + }) + .context("failed to start the signal thread")?; + Ok(()) + } + + fn wait(signals: &Blocked) -> libc::c_int { + loop { + let mut signal = 0; + // SAFETY: valid set and out pointer; `sigwait` returns an error + // number instead of setting errno, and fills `signal` on success. + let result = unsafe { libc::sigwait(&signals.0, &mut signal) }; + if result == 0 { + return signal; + } + // EINTR (after SIGSTOP and SIGCONT) just retries. Nothing else is + // expected for valid signals; pause so it can't spin. + if result != libc::EINTR { + thread::sleep(Duration::from_millis(100)); + } + } + } } struct Settings { diff --git a/packages/process-daemon/tests/ownership.rs b/packages/process-daemon/tests/ownership.rs index 157e0f8c4b6d..0cade5397a31 100644 --- a/packages/process-daemon/tests/ownership.rs +++ b/packages/process-daemon/tests/ownership.rs @@ -545,14 +545,11 @@ fn stop_signals_stop_terminals_and_clean_up() { assert!(matches!(response, Response::Owned)); let terminal = daemon.terminal("exec sleep 1000"); - // The daemon blocks these signals for its own threads; its children must not - // inherit that mask, so a terminal's process still dies from SIGTERM. + // The daemon blocks these signals on its own threads; its children must not + // inherit that mask, so a terminal's process still dies from the signal. let probe = daemon.terminal("exec sleep 1000"); // SAFETY: the probe PID belongs to this test's live terminal. - assert_eq!( - unsafe { libc::kill(probe.pid.unwrap() as i32, libc::SIGTERM) }, - 0 - ); + assert_eq!(unsafe { libc::kill(probe.pid.unwrap() as i32, signal) }, 0); let deadline = Instant::now() + Duration::from_secs(3); loop { let Response::Terminals { terminals } = daemon.request(Request::List) else { @@ -568,7 +565,7 @@ fn stop_signals_stop_terminals_and_clean_up() { if exited { break; } - assert!(Instant::now() < deadline, "probe ignored SIGTERM"); + assert!(Instant::now() < deadline, "probe ignored signal {signal}"); thread::sleep(Duration::from_millis(10)); } From 573c83f77dd2bf801d5d698222df035b60a49152 Mon Sep 17 00:00:00 2001 From: James Long Date: Fri, 30 Oct 2026 16:08:40 -0400 Subject: [PATCH 3/3] chore(process-daemon): drop a redundant Send impl --- packages/process-daemon/src/daemon/server.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/process-daemon/src/daemon/server.rs b/packages/process-daemon/src/daemon/server.rs index 8c7ce9130683..d54b4d1b3c60 100644 --- a/packages/process-daemon/src/daemon/server.rs +++ b/packages/process-daemon/src/daemon/server.rs @@ -60,9 +60,6 @@ mod signals { pub(super) struct Blocked(libc::sigset_t); - // SAFETY: a sigset_t is plain data. - unsafe impl Send for Blocked {} - /// Blocks the stop signals on the calling thread, and restores their /// default action: macOS never delivers an ignored signal to `sigwait`, /// and a launcher may have left SIGINT ignored.