mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(env): kill the daemon's commands when it is terminated
Connection.close() ends stdin and sends SIGTERM at once. The daemon did not handle SIGTERM, so it died before its end-of-input kill_all, and running commands were reparented to init. The daemon now blocks SIGTERM, SIGINT and SIGHUP before starting any thread and waits for them on its own thread, which kills every process group it started and exits. On Windows the job object already kills commands with the daemon.
This commit is contained in:
parent
391e83fb17
commit
b6d5c2ecdc
6 changed files with 64 additions and 2 deletions
1
packages/env/CHANGELOG.md
vendored
1
packages/env/CHANGELOG.md
vendored
|
|
@ -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
|
||||
|
||||
|
|
|
|||
11
packages/env/daemon/src/main.rs
vendored
11
packages/env/daemon/src/main.rs
vendored
|
|
@ -470,6 +470,15 @@ fn serve_frames(server: &Arc<Server>, 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()));
|
||||
|
|
|
|||
23
packages/env/daemon/src/sys/unix.rs
vendored
23
packages/env/daemon/src/sys/unix.rs
vendored
|
|
@ -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;
|
||||
|
|
|
|||
3
packages/env/daemon/src/sys/windows.rs
vendored
3
packages/env/daemon/src/sys/windows.rs
vendored
|
|
@ -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());
|
||||
|
|
|
|||
3
packages/env/docs/protocol.md
vendored
3
packages/env/docs/protocol.md
vendored
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
25
packages/env/test/remote.test.ts
vendored
25
packages/env/test/remote.test.ts
vendored
|
|
@ -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<number>();
|
||||
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));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue