diff --git a/packages/env/CHANGELOG.md b/packages/env/CHANGELOG.md index c484db660..96bd4e91e 100644 --- a/packages/env/CHANGELOG.md +++ b/packages/env/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - `exec` and `pty` fail with `spawn_error` instead of hanging when the daemon cannot start a helper thread (for example at a cgroup task limit); the started command is killed and reaped ([#11433](https://github.com/earendil-works/pi/issues/11433)). +- `Connection.close()` on Unix no longer leaves running commands behind: the daemon used to die from the client's `SIGTERM` before killing its process groups. It now kills them on `SIGTERM`, `SIGINT` or `SIGHUP` before exiting. ## [1.4.0] - 2026-10-21 diff --git a/packages/env/daemon/src/main.rs b/packages/env/daemon/src/main.rs index 4457fef4d..ea720eabb 100644 --- a/packages/env/daemon/src/main.rs +++ b/packages/env/daemon/src/main.rs @@ -470,6 +470,15 @@ fn serve_frames(server: &Arc, input: &mut impl Read) -> io::Result<()> { } fn serve(token: &str) -> io::Result<()> { + let groups: exec::Groups = Arc::new(Mutex::new(Default::default())); + { + // A client that stops the daemon (Node's `child.kill()`), or a hangup, leaves nothing running either. + let groups = groups.clone(); + sys::on_exit_signal(move |signal| { + kill_all(&groups); + std::process::exit(128 + signal); + }); + } { let mut stdout = io::stdout().lock(); writeln!(stdout, "PI-ENV {token}")?; @@ -488,7 +497,7 @@ fn serve(token: &str) -> io::Result<()> { handles: Mutex::new(HashMap::new()), next_handle: AtomicU64::new(1), controls: Mutex::new(HashMap::new()), - groups: Arc::new(Mutex::new(Default::default())), + groups, terminals: Arc::new(Mutex::new(HashMap::new())), }); let last_seen = Arc::new(AtomicU64::new(now_ms())); diff --git a/packages/env/daemon/src/sys/unix.rs b/packages/env/daemon/src/sys/unix.rs index 764cad1d1..4b0637f0e 100644 --- a/packages/env/daemon/src/sys/unix.rs +++ b/packages/env/daemon/src/sys/unix.rs @@ -283,6 +283,29 @@ pub fn configure( pub fn after_spawn(_child: &Child) {} +/// Run `on_signal` with the signal on its own thread when `SIGTERM`, `SIGINT` or `SIGHUP` arrives, instead of dying +/// at once. Call before starting any other thread: threads inherit the blocked signals, so only this one receives +/// them. Commands reset their signal mask before they run. +pub fn on_exit_signal(on_signal: impl FnOnce(i32) + Send + 'static) { + // SAFETY: a zeroed sigset_t initialized by sigemptyset, and plain signal mask calls. + let set = unsafe { + let mut set: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset(&mut set); + for signal in [libc::SIGTERM, libc::SIGINT, libc::SIGHUP] { + libc::sigaddset(&mut set, signal); + } + libc::pthread_sigmask(libc::SIG_BLOCK, &set, std::ptr::null_mut()); + set + }; + std::thread::spawn(move || { + let mut signal = 0; + // SAFETY: waits for one of the signals blocked above. + if unsafe { libc::sigwait(&set, &mut signal) } == 0 { + on_signal(signal); + } + }); +} + /// Kill the command's process group, or the process if the group is gone. pub fn kill_tree(pid: u32) { let pid = pid as i32; diff --git a/packages/env/daemon/src/sys/windows.rs b/packages/env/daemon/src/sys/windows.rs index f43a4bacf..de39fd4c3 100644 --- a/packages/env/daemon/src/sys/windows.rs +++ b/packages/env/daemon/src/sys/windows.rs @@ -550,6 +550,9 @@ pub fn after_spawn(child: &Child) { } } +/// Nothing to do: the daemon cannot catch `TerminateProcess`, and the job object kills its commands with it. +pub fn on_exit_signal(_on_signal: impl FnOnce(i32) + Send + 'static) {} + /// Node's `killProcessTree` on Windows: `taskkill /F /T`, not waited for. pub fn kill_tree(pid: u32) { let system_root = std::env::var("SystemRoot").unwrap_or_else(|_| "C:\\Windows".into()); diff --git a/packages/env/docs/protocol.md b/packages/env/docs/protocol.md index ae43fb767..f8ca5b4e1 100644 --- a/packages/env/docs/protocol.md +++ b/packages/env/docs/protocol.md @@ -62,7 +62,8 @@ command as a full pipe would. ## Liveness Each side sends `ping` every 5 seconds. The daemon kills every process group it started and exits after 30 seconds -without receiving any bytes, or when stdin ends. A dropped connection therefore stops remote commands. The client +without receiving any bytes, when stdin ends, or on `SIGTERM`, `SIGINT` or `SIGHUP` (on Windows, a job object kills +its commands with it). A dropped connection or a stopped daemon therefore stops remote commands. The client fails requests of a lost connection with `{ code: "unknown", lost: true }` and starts a new daemon on the next request; handles belong to the daemon that opened them, so requests on them fail instead of reaching the new one. diff --git a/packages/env/test/remote.test.ts b/packages/env/test/remote.test.ts index 4b1241148..4f7370679 100644 --- a/packages/env/test/remote.test.ts +++ b/packages/env/test/remote.test.ts @@ -117,6 +117,31 @@ describe("RemoteExecutionEnv", () => { expect(Date.now() - started).toBeLessThan(3000); }); + // The daemon used to die on Connection.close()'s SIGTERM before killing its commands. + it.skipIf(process.platform === "win32")("kills running commands when the connection closes", async () => { + const { env, connection } = environment(); + const pid = Promise.withResolvers(); + const running = env.exec( + ["sh", "-c", "echo $$; exec sleep 30"], + { onOutput: (text: string) => pid.resolve(Number.parseInt(text, 10)) }, + context, + ); + const command = await pid.promise; + connection.close(); + await running; + const alive = () => { + try { + process.kill(command, 0); + return true; + } catch { + return false; + } + }; + const deadline = Date.now() + 5000; + while (alive() && Date.now() < deadline) await new Promise((done) => setTimeout(done, 50)); + expect(alive()).toBe(false); + }); + it("fails requests on handles of a lost connection instead of reaching the new daemon", async () => { const { env, connection } = environment(); getOrThrow(await env.writeFile("a.txt", "AAA", context));