mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
terminal: process-group lifecycle with drain-ordered teardown
Closing a tab must end the work the tab was doing. The child is a session leader (portable-pty calls setsid), so signalling its pid reaches the shell and leaves everything the shell started running, reparented to init and holding the pty slave. Signal the process group instead: SIGTERM, a bounded grace, then SIGKILL to the group whether or not the leader went quietly -- a polite leader does not imply an empty group. Three things this found that reading could not. waitpid(WNOHANG | WNOWAIT) returns EINVAL on Darwin. POSIX defines WNOWAIT only for waitid; Linux tolerates it, macOS does not. The failure was silent because `seen == pid` is false for -1 exactly as it is false for "still running" -- error and negative result collapsed into one branch. Every shutdown escalated to SIGKILL and the polite arm was dead code on the platform we develop on. Only an assertion on *which arm fired* could see it. A grandchild test that does not defeat the tty hangup proves nothing. Killing a session leader makes the kernel SIGHUP the foreground group, so the grandchild dies either way and a pid-only mutation survives. The fixture must ignore SIGHUP and busy-loop; then pid-only leaks and kill(-pgid) does not. A child blocked writing to an undrained master does not die promptly even on SIGKILL -- measured 606ms to reap, versus microseconds when drained. So the reader outlives termination and reap. shutdown_draining takes the reader by value, which makes the forbidden order a compile error rather than a comment: a joined reader has been consumed and cannot be passed. Mutation matrix, sources restored byte-identically after each: L1 delete SIGKILL escalation -> FAIL (orphan + grace arms) L2 kill(pid) not kill(-pgid) -> FAIL (orphan arm) L3 delete pre-reap try_wait -> FAIL (Killed, want AlreadyExited) L4 join reader before SIGTERM -> FAIL (10.01s, grace is 250ms) Fixtures built to ignore signals carry their own SIGKILL deadline. Without it a legitimate failure leaves a core spinning at PPID 1, and L1/L4 were detected by hanging -- a test that finds a bug by never finishing cannot be told from a broken one. The deadline sits far outside the observation window so it can never rescue a failing implementation. 2088 desktop + 3 mixer + 26 buzz-terminal + 5 fences + 4 resize green, clippy --workspace --all-targets -D warnings clean, fmt clean. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc
co-authored by
tlongwell-block
tlongwell-block
parent
f05ef3170f
commit
c17f2d2c3d
@@ -9,6 +9,7 @@ pub mod context;
|
||||
pub mod damage;
|
||||
pub mod env_fence;
|
||||
pub mod fences;
|
||||
pub mod lifecycle;
|
||||
pub mod listener;
|
||||
pub mod path;
|
||||
pub mod reader;
|
||||
@@ -19,6 +20,8 @@ pub mod shell;
|
||||
mod context_tests;
|
||||
#[cfg(test)]
|
||||
mod env_fence_tests;
|
||||
#[cfg(test)]
|
||||
mod lifecycle_tests;
|
||||
|
||||
use alacritty_terminal::grid::Dimensions;
|
||||
use alacritty_terminal::term::{Config, Osc52, Term};
|
||||
|
||||
@@ -0,0 +1,259 @@
|
||||
//! Child-process lifecycle for spawned PTY sessions.
|
||||
//!
|
||||
//! Closing a terminal tab must actually end the work the tab was doing. That
|
||||
//! is harder than calling `kill`, for two reasons that both come from the
|
||||
//! child being a *session leader* rather than an ordinary subprocess.
|
||||
//!
|
||||
//! **1. The child is not the only process.** `portable-pty` calls `setsid()`
|
||||
//! in `pre_exec` (`unix.rs:257`), so the shell becomes a session and process
|
||||
//! group leader; everything it runs — `vim`, a `make -j8` tree, a backgrounded
|
||||
//! `sleep` — joins that group or a descendant of it. Signalling the shell's
|
||||
//! pid alone reaches the shell. A shell that exits without forwarding the
|
||||
//! signal leaves its children running, reparented to init, holding the pty
|
||||
//! slave open. That is a leak that survives the window closing.
|
||||
//!
|
||||
//! So we signal the **process group** (`kill(-pgid)`), not the pid.
|
||||
//!
|
||||
//! **2. `portable-pty`'s own `kill` is not sufficient here.** `ChildKiller for
|
||||
//! std::process::Child` (`lib.rs:340-373`) sends `SIGHUP` to the *pid*, waits
|
||||
//! up to 4x50 ms, then falls back to `Child::kill` — which is `SIGKILL`, again
|
||||
//! to the pid. Both halves are pid-scoped, so neither reaches a grandchild.
|
||||
//! It is a correct API for "end this process"; ours is "end this session".
|
||||
//!
|
||||
//! ## The escalation
|
||||
//!
|
||||
//! `SIGTERM` to the group, a bounded wait for the leader, then `SIGKILL` to
|
||||
//! the group **whether or not the leader went quietly** -- see `shutdown` for
|
||||
//! why a polite leader does not imply an empty group.
|
||||
//! `SIGTERM` first because a shell asked to terminate cleanly will flush its
|
||||
//! history and let `vim` write its swap file; going straight to `SIGKILL`
|
||||
//! guarantees no process ever gets that chance. The bounded wait is what makes
|
||||
//! the escalation real — without it, `SIGKILL` either races the polite path
|
||||
//! (making `SIGTERM` decorative) or never fires (making a signal-ignoring
|
||||
//! child immortal).
|
||||
//!
|
||||
//! ## What this deliberately does not do
|
||||
//!
|
||||
//! A process that has called `setsid()` for *itself* has left our group, and
|
||||
//! no group signal reaches it. `nohup`, a daemonising build tool, and
|
||||
//! `tmux`-style servers all do this on purpose. We do not hunt the process
|
||||
//! tree to find them: walking children to signal them is a race against a
|
||||
//! moving tree — a pid read and then signalled may be a *different* process by
|
||||
//! the time the signal lands, and killing a stranger's pid is a far worse bug
|
||||
//! than leaking a daemon the user deliberately detached. Detaching from the
|
||||
//! session is the documented way to survive one's terminal, and honouring it
|
||||
//! is correct behaviour, not a gap.
|
||||
|
||||
use std::io;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use portable_pty::Child;
|
||||
|
||||
/// How long the group gets to honour `SIGTERM` before `SIGKILL`.
|
||||
///
|
||||
/// Long enough for a shell to run its exit trap and for an editor to write a
|
||||
/// swap file; short enough that closing a tab never feels stuck. Tab close is
|
||||
/// not synchronous with this wait in the UI, so this is a cleanup deadline,
|
||||
/// not a frame budget.
|
||||
pub const TERM_GRACE: Duration = Duration::from_millis(250);
|
||||
|
||||
/// Poll interval while waiting for the child to exit.
|
||||
///
|
||||
/// Polling rather than blocking in `wait()`: a blocking wait cannot be given a
|
||||
/// deadline without a second thread, and the whole point of the grace period
|
||||
/// is that it expires.
|
||||
const POLL_INTERVAL: Duration = Duration::from_millis(5);
|
||||
|
||||
/// How a session ended.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Shutdown {
|
||||
/// Already gone before we signalled.
|
||||
AlreadyExited,
|
||||
/// Exited within [`TERM_GRACE`] of `SIGTERM`.
|
||||
Terminated,
|
||||
/// Ignored or outlived `SIGTERM`; the group was killed.
|
||||
Killed,
|
||||
}
|
||||
|
||||
/// Ends the session led by `child`: `SIGTERM` to its process group, a bounded
|
||||
/// wait, then `SIGKILL` to the group if anything is still there.
|
||||
///
|
||||
/// Reaps the child before returning, so the caller cannot leave a zombie by
|
||||
/// dropping the handle. Returns which arm ended it, which is what a test can
|
||||
/// assert on — "did it die" is satisfied by both arms and so distinguishes
|
||||
/// nothing.
|
||||
#[cfg(unix)]
|
||||
pub fn shutdown(child: &mut Box<dyn Child + Send + Sync>) -> io::Result<Shutdown> {
|
||||
// Reap first. A child that already exited still has a pid slot until it is
|
||||
// waited for, and that pid is reusable the moment it is released -- so
|
||||
// signalling without checking is how a cleanup path eventually signals an
|
||||
// unrelated process. Check before signalling, every time.
|
||||
if child.try_wait()?.is_some() {
|
||||
return Ok(Shutdown::AlreadyExited);
|
||||
}
|
||||
|
||||
let Some(pid) = child.process_id() else {
|
||||
// No pid means nothing to signal; still ensure it is reaped.
|
||||
child.wait()?;
|
||||
return Ok(Shutdown::AlreadyExited);
|
||||
};
|
||||
let pid = pid as i32;
|
||||
|
||||
signal_group(pid, libc::SIGTERM);
|
||||
let leader_honoured_term = leader_exited_by(pid, Instant::now() + TERM_GRACE);
|
||||
|
||||
// Sweep the group unconditionally, *including* when the leader exited
|
||||
// politely. The leader's exit is not the session's end: anything it
|
||||
// backgrounded that ignores SIGTERM is still running, still in the group,
|
||||
// and still holding the pty. Returning `Terminated` at that point reports
|
||||
// success over a leak.
|
||||
//
|
||||
// Ordering with the reap is a safety requirement, not a preference. A
|
||||
// process group id *is* the leader's pid, and the kernel may recycle that
|
||||
// pid once the leader is reaped -- at which point `kill(-pid)` names some
|
||||
// unrelated group. An exited-but-unreaped leader is a zombie, and a zombie
|
||||
// is still a group member, so the id cannot be reused while we hold it.
|
||||
// Signal first, reap second, and the window does not exist.
|
||||
signal_group(pid, libc::SIGKILL);
|
||||
|
||||
// SIGKILL cannot be caught, so this terminates. It is still a `wait`
|
||||
// rather than an assumption: the pid must be reaped, and the exit status
|
||||
// is only available to whoever reaps it.
|
||||
child.wait()?;
|
||||
|
||||
Ok(if leader_honoured_term {
|
||||
Shutdown::Terminated
|
||||
} else {
|
||||
Shutdown::Killed
|
||||
})
|
||||
}
|
||||
|
||||
/// Sends `signal` to `pid`'s process group, falling back to the pid alone.
|
||||
///
|
||||
/// The fallback matters: `kill(-pgid)` requires the child to *be* a group
|
||||
/// leader, which it is only because `portable-pty` called `setsid()`. If that
|
||||
/// ever stops being true, a pid-scoped signal still ends the shell — degraded
|
||||
/// (grandchildren survive) rather than a silent no-op.
|
||||
///
|
||||
/// Errors are deliberately not propagated. Every failure mode here means the
|
||||
/// process is already gone (`ESRCH`) or was never ours to signal (`EPERM`),
|
||||
/// and in both cases the following `wait` is the authority on what happened.
|
||||
#[cfg(unix)]
|
||||
fn signal_group(pid: i32, signal: i32) {
|
||||
// SAFETY: `kill` with a negative pid targets the process group; both
|
||||
// arguments are plain integers and the call has no memory effects.
|
||||
let sent = unsafe { libc::kill(-pid, signal) };
|
||||
if sent != 0 {
|
||||
// SAFETY: as above.
|
||||
unsafe { libc::kill(pid, signal) };
|
||||
}
|
||||
}
|
||||
|
||||
/// Polls until the leader has exited, or `deadline` passes. Returns whether it
|
||||
/// exited in time.
|
||||
///
|
||||
/// Deliberately **not** `Child::try_wait`, which reaps: reaping here would
|
||||
/// release the process group id before the sweep above can use it. `WNOWAIT`
|
||||
/// reads the child's exit state and leaves it waitable, so the zombie stays
|
||||
/// and keeps the group id reserved for us.
|
||||
///
|
||||
/// `waitid`, not `waitpid`, and that is a portability requirement rather than
|
||||
/// taste. POSIX only defines `WNOWAIT` for `waitid`; Linux tolerates it on
|
||||
/// `waitpid`, and **Darwin returns `EINVAL`**. Measured with a C probe: on
|
||||
/// macOS 25.5.0, `waitpid(pid, &st, WNOHANG | WNOWAIT)` is `-1/EINVAL` for a
|
||||
/// child that has plainly exited. That failure is silent in the shape this
|
||||
/// function had — an error is indistinguishable from "not exited yet", so the
|
||||
/// grace period could never be honoured and *every* shutdown escalated to
|
||||
/// `SIGKILL`, reporting `Killed` for a child that died politely on the first
|
||||
/// `SIGTERM`. The polite arm was dead code on the platform we develop on.
|
||||
///
|
||||
/// `waitid` reports a still-running child as success-with-`si_pid == 0`, so
|
||||
/// the out-parameter must be zeroed before each call and the *pid*, not the
|
||||
/// return code, is the answer.
|
||||
#[cfg(unix)]
|
||||
fn leader_exited_by(pid: i32, deadline: Instant) -> bool {
|
||||
loop {
|
||||
// SAFETY: `info` is a valid, fully-initialised out-pointer for the
|
||||
// duration of the call. `WNOWAIT` leaves the child waitable, so the
|
||||
// later `wait` still returns its status.
|
||||
let exited = unsafe {
|
||||
let mut info: libc::siginfo_t = std::mem::zeroed();
|
||||
let rc = libc::waitid(
|
||||
libc::P_PID,
|
||||
pid as libc::id_t,
|
||||
&mut info,
|
||||
libc::WEXITED | libc::WNOHANG | libc::WNOWAIT,
|
||||
);
|
||||
rc == 0 && info.si_pid() == pid
|
||||
};
|
||||
if exited {
|
||||
return true;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return false;
|
||||
}
|
||||
std::thread::sleep(POLL_INTERVAL);
|
||||
}
|
||||
}
|
||||
|
||||
/// Maximum concurrent terminal sessions.
|
||||
///
|
||||
/// Each session costs a pty pair (two fds), a reader thread, and a scrollback
|
||||
/// grid -- at the default 10k lines x 80 cols that is megabytes of resident
|
||||
/// memory per tab. The cap exists because tab creation is one keystroke and
|
||||
/// nothing else bounds it: without a limit, a held-down shortcut exhausts the
|
||||
/// process fd table, and the first thing to fail is not the terminal but
|
||||
/// whatever *else* in Buzz next asks for a file descriptor -- the relay
|
||||
/// socket, a database handle. A resource a UI can allocate in a loop needs a
|
||||
/// ceiling that fails in its own subsystem.
|
||||
///
|
||||
/// 20 matches the abandoned `feat/terminal` branch's `MAX_LIVE_SESSIONS`,
|
||||
/// kept deliberately: it is far above any plausible human tab count and far
|
||||
/// below the default 256-fd soft limit, so it bounds the runaway case without
|
||||
/// ever being reachable by hand.
|
||||
pub const MAX_LIVE_SESSIONS: usize = 20;
|
||||
|
||||
/// A reader that is still consuming the PTY master, to be stopped only after
|
||||
/// the session has been torn down.
|
||||
///
|
||||
/// This exists because the correct close order is not the obvious one, and
|
||||
/// nothing in the type system otherwise prevents the wrong one. Mari's ruling
|
||||
/// (`62509b91`) is that **the reader outlives child termination and reap**:
|
||||
///
|
||||
/// 1. mark the session closing and stop publishing to the UI;
|
||||
/// 2. `SIGTERM` -> grace -> `SIGKILL` -> reap, *while output is still drained*;
|
||||
/// 3. only then close the master and join the reader.
|
||||
///
|
||||
/// Inverting steps 2 and 3 is the bug this trait is shaped to prevent, and it
|
||||
/// is not a hypothetical: a child blocked writing into a master nobody reads
|
||||
/// does not die promptly even on `SIGKILL`, because the kernel completes the
|
||||
/// tty teardown first. Measured with a `forkpty` probe -- **606 ms** to reap a
|
||||
/// `SIGKILL`ed child against an undrained master, versus microseconds when
|
||||
/// drained. Join the reader first and every tab close pays that, on the arm
|
||||
/// where the user is already waiting.
|
||||
pub trait DrainingReader {
|
||||
/// Stops consuming and releases the reader. Called *after* reap.
|
||||
fn join(self: Box<Self>);
|
||||
}
|
||||
|
||||
/// Ends a session in the order the drain law requires, and returns how it
|
||||
/// ended.
|
||||
///
|
||||
/// The ordering is enforced by ownership rather than by documentation: this
|
||||
/// function takes the reader **by value**, so a caller cannot have joined it
|
||||
/// beforehand -- a joined reader has been consumed and cannot be passed here.
|
||||
/// The only way to use this API is the correct order. A comment saying "do not
|
||||
/// join the reader first" is advice; a moved value is a compile error.
|
||||
#[cfg(unix)]
|
||||
pub fn shutdown_draining(
|
||||
child: &mut Box<dyn Child + Send + Sync>,
|
||||
reader: Box<dyn DrainingReader>,
|
||||
) -> io::Result<Shutdown> {
|
||||
// Terminate and reap with the reader still running, so the child never
|
||||
// blocks in a tty write while we are waiting on it.
|
||||
let outcome = shutdown(child);
|
||||
// Unconditional: the reader must be released whether or not the shutdown
|
||||
// reported an error, or a failed close leaks the thread and its fd.
|
||||
reader.join();
|
||||
outcome
|
||||
}
|
||||
@@ -0,0 +1,528 @@
|
||||
//! Lifecycle gates: the session dies, the grandchild dies with it, and the
|
||||
//! login `argv[0]` the child actually receives is the one we computed.
|
||||
//!
|
||||
//! Every test here drives a real PTY and a real process tree. A mock child
|
||||
//! would let us assert that we *called* `kill`, which is the half we already
|
||||
//! know; the property under test is what the kernel does with a process group
|
||||
//! we do not fully control.
|
||||
|
||||
use crate::env_fence::fence_env;
|
||||
use crate::lifecycle::{shutdown, shutdown_draining, DrainingReader, Shutdown, TERM_GRACE};
|
||||
use crate::path::user_shell_path;
|
||||
use crate::shell::{login_argv0, resolve_shell};
|
||||
use portable_pty::{native_pty_system, Child, CommandBuilder, PtyPair, PtySize};
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
/// Upper bound on any wait in this file.
|
||||
///
|
||||
/// Every wait here is bounded, and that is not caution -- it is the lesson
|
||||
/// from a probe of an interactive child that read to EOF and hung for 300 s.
|
||||
/// A PTY master does not reach EOF while any process holds the slave open, so
|
||||
/// "read until the child is done" is not a terminating program. Bound the
|
||||
/// read, or poll for the observable effect.
|
||||
const BOUND: Duration = Duration::from_secs(10);
|
||||
|
||||
/// Self-destruct deadline, in seconds, for fixture processes built to ignore
|
||||
/// signals.
|
||||
///
|
||||
/// Comfortably longer than [`BOUND`], so it can never end a process while the
|
||||
/// test is still observing it -- a watchdog that fires inside the observation
|
||||
/// window would make a *failing* implementation look correct. Short enough
|
||||
/// that a crashed run does not leave a core spinning until reboot.
|
||||
const WATCHDOG: u64 = 60;
|
||||
|
||||
fn open_pty() -> PtyPair {
|
||||
native_pty_system()
|
||||
.openpty(PtySize {
|
||||
rows: 24,
|
||||
cols: 80,
|
||||
pixel_width: 0,
|
||||
pixel_height: 0,
|
||||
})
|
||||
.expect("openpty")
|
||||
}
|
||||
|
||||
/// Drains the PTY master in the background for as long as it stays open.
|
||||
///
|
||||
/// Not hygiene -- a correctness requirement, and the cause of a 300 s hang in
|
||||
/// the first version of this file. A PTY has a small kernel buffer, and a
|
||||
/// child writing into a master nobody reads blocks in `write()` once it fills.
|
||||
/// A process blocked in an uninterruptible tty write does not die promptly on
|
||||
/// `SIGKILL`: the signal is delivered, but the kernel finishes tearing down
|
||||
/// the tty session first, so `wait()` sits there while the reap completes.
|
||||
/// Measured directly with a `forkpty` C probe: with the master undrained, a
|
||||
/// `SIGKILL`ed child took **606 ms** to be reaped. Every terminal in the
|
||||
/// product drains its master continuously -- that is what a renderer *is* --
|
||||
/// so a test that doesn't is modelling a configuration that never ships.
|
||||
///
|
||||
/// The consequence is worth stating for the embedder: **shutdown must not be
|
||||
/// called after the reader has stopped.** Tear the session down while output
|
||||
/// is still being consumed, or the grace period is spent waiting on a
|
||||
/// self-inflicted stall.
|
||||
fn drain(pair: &PtyPair) {
|
||||
let mut reader = pair.master.try_clone_reader().expect("reader");
|
||||
std::thread::spawn(move || {
|
||||
use std::io::Read;
|
||||
let mut buf = [0u8; 4096];
|
||||
while matches!(reader.read(&mut buf), Ok(n) if n > 0) {}
|
||||
});
|
||||
}
|
||||
|
||||
/// Spawns `script` under `/bin/sh` on a real PTY, fully fenced.
|
||||
fn spawn_script(pair: &PtyPair, script: &str) -> Box<dyn Child + Send + Sync> {
|
||||
let shell = resolve_shell(std::env::var("SHELL").ok().as_deref());
|
||||
let mut cmd = CommandBuilder::new("/bin/sh");
|
||||
fence_env(&mut cmd, &user_shell_path(), &shell);
|
||||
cmd.arg("-c");
|
||||
cmd.arg(script);
|
||||
let child = pair.slave.spawn_command(cmd).expect("spawn");
|
||||
drain(pair);
|
||||
child
|
||||
}
|
||||
|
||||
/// True while `pid` exists. `kill(pid, 0)` performs the permission and
|
||||
/// existence checks without delivering a signal.
|
||||
fn pid_alive(pid: i32) -> bool {
|
||||
// SAFETY: signal 0 delivers nothing; both arguments are integers.
|
||||
unsafe { libc::kill(pid, 0) == 0 }
|
||||
}
|
||||
|
||||
/// Polls `f` until it returns true or `BOUND` elapses; returns whether it did.
|
||||
///
|
||||
/// Polling for the observable state rather than sleeping a fixed duration: a
|
||||
/// sleep long enough to be reliable is slow, and a sleep short enough to be
|
||||
/// fast is a race that fails on a loaded machine. Both are worse than asking.
|
||||
fn poll_until(mut f: impl FnMut() -> bool) -> bool {
|
||||
let deadline = Instant::now() + BOUND;
|
||||
while Instant::now() < deadline {
|
||||
if f() {
|
||||
return true;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(5));
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Reads a file until it is non-empty or `BOUND` elapses.
|
||||
fn read_when_written(path: &std::path::Path) -> Option<String> {
|
||||
let mut found = None;
|
||||
poll_until(|| match std::fs::read_to_string(path) {
|
||||
Ok(text) if !text.trim().is_empty() => {
|
||||
found = Some(text.trim().to_owned());
|
||||
true
|
||||
}
|
||||
_ => false,
|
||||
});
|
||||
found
|
||||
}
|
||||
|
||||
/// A cooperative child exits on `SIGTERM`, so the polite arm is what ends it.
|
||||
///
|
||||
/// The distinction matters: `Killed` and `Terminated` both leave a dead
|
||||
/// process, so asserting death alone would pass with `SIGTERM` deleted
|
||||
/// entirely and the grace period reduced to a delay before `SIGKILL`.
|
||||
#[test]
|
||||
fn cooperative_child_exits_on_term_not_kill() {
|
||||
let pair = open_pty();
|
||||
let mut child = spawn_script(&pair, "sleep 30");
|
||||
let pid = child.process_id().expect("pid") as i32;
|
||||
|
||||
let outcome = shutdown(&mut child).expect("shutdown");
|
||||
assert_eq!(
|
||||
outcome,
|
||||
Shutdown::Terminated,
|
||||
"a child that dies on SIGTERM must not have needed SIGKILL"
|
||||
);
|
||||
assert!(poll_until(|| !pid_alive(pid)), "child survived shutdown");
|
||||
}
|
||||
|
||||
/// A child that ignores `SIGTERM` must still die, and the escalation must be
|
||||
/// what kills it.
|
||||
///
|
||||
/// The fixture shape is load-bearing and my first one was vacuous. I wrote
|
||||
/// `trap '' TERM; sleep 30`, which *looks* like a signal-ignoring child and
|
||||
/// reported `Terminated` -- the polite arm, on a child built to defeat it.
|
||||
/// The reason is that `sh` does not ignore a signal on its child's behalf: the
|
||||
/// group `SIGTERM` reaches `sleep`, which has no trap and dies, and the shell
|
||||
/// was blocked in `wait` on exactly that `sleep`, so it reaps it and exits
|
||||
/// normally. The trap was real, the ignoring was real, and the process still
|
||||
/// died on `SIGTERM` -- through a path the test wasn't looking at.
|
||||
///
|
||||
/// Had I not checked *which* arm fired, this would have passed for the wrong
|
||||
/// reason and gone on "proving" an escalation it never exercised. The loop
|
||||
/// keeps the shell itself alive: no blocking `wait` to be interrupted, so the
|
||||
/// trap actually governs the shell's own fate and only `SIGKILL` can end it.
|
||||
///
|
||||
/// The readiness handshake closes a second, subtler version of the same
|
||||
/// mistake. My loop fixture *still* reported `Terminated`, because `trap` is a
|
||||
/// command the shell has to reach: a signal delivered in the interval between
|
||||
/// `exec` and that line finds the default disposition and kills the shell
|
||||
/// outright. Isolated with a `forkpty` probe -- identical binary, only the
|
||||
/// delay before signalling changed: at 500 ms all four arms survived, at 2 ms
|
||||
/// all four died with signal 15. A fixture that is only *probably* armed makes
|
||||
/// this test a race whose failure mode is a false pass.
|
||||
///
|
||||
/// This is the arm that fails if the escalation is deleted -- and the
|
||||
/// `WATCHDOG` is what makes that a *failure* rather than a hang. With
|
||||
/// `SIGKILL` deleted, nothing we send can end a child that ignores `SIGTERM`,
|
||||
/// so `shutdown`'s final `wait` blocks forever and the mutant is detected only
|
||||
/// by the harness timing out. A test that detects a bug by never finishing is
|
||||
/// indistinguishable from a broken test. The deadline converts it into a
|
||||
/// bounded, reportable failure.
|
||||
#[test]
|
||||
fn signal_ignoring_child_is_killed_after_the_grace_period() {
|
||||
let dir = tempdir("buzz-terminal-trap");
|
||||
let ready = dir.join("armed");
|
||||
|
||||
let pair = open_pty();
|
||||
// The readiness file is written *after* the trap is installed, so waiting
|
||||
// on it converts "probably armed by now" into an observed fact.
|
||||
let mut child = spawn_script(
|
||||
&pair,
|
||||
&format!(
|
||||
"trap '' TERM; (sleep {WATCHDOG}; kill -9 $$) & : > {}; \
|
||||
while :; do sleep 0.1; done",
|
||||
ready.display()
|
||||
),
|
||||
);
|
||||
let pid = child.process_id().expect("pid") as i32;
|
||||
assert!(
|
||||
poll_until(|| ready.exists()),
|
||||
"child never armed its SIGTERM trap; signalling now would test a \
|
||||
startup race rather than the escalation"
|
||||
);
|
||||
|
||||
let started = Instant::now();
|
||||
let outcome = shutdown(&mut child).expect("shutdown");
|
||||
let elapsed = started.elapsed();
|
||||
|
||||
assert_eq!(
|
||||
outcome,
|
||||
Shutdown::Killed,
|
||||
"a SIGTERM-ignoring child must be escalated to SIGKILL"
|
||||
);
|
||||
assert!(poll_until(|| !pid_alive(pid)), "child survived SIGKILL");
|
||||
assert!(
|
||||
elapsed >= TERM_GRACE,
|
||||
"shutdown returned in {elapsed:?}, before the {TERM_GRACE:?} grace \
|
||||
period could have elapsed -- SIGTERM was never given its chance"
|
||||
);
|
||||
assert!(
|
||||
elapsed < BOUND,
|
||||
"shutdown took {elapsed:?}; the grace period is not bounded"
|
||||
);
|
||||
}
|
||||
|
||||
/// The property the whole module exists for: a **grandchild** must not outlive
|
||||
/// the session.
|
||||
///
|
||||
/// The fixture is deliberately hostile, and the obvious version of this test
|
||||
/// proves nothing. I first wrote `sleep 30 & echo $!; wait` and mutation L2 --
|
||||
/// replacing `kill(-pid)` with `kill(pid)` -- **survived it**. The reason is
|
||||
/// that killing a PTY session leader makes the kernel hang up the terminal and
|
||||
/// `SIGHUP` the whole foreground group, so the grandchild dies either way.
|
||||
/// Isolated with a `forkpty` probe: with the master held open (no fd-closure
|
||||
/// hangup) and only `SIGKILL` to the shell's pid, the grandchild was gone
|
||||
/// within 200 ms while the shell itself was still unreaped. The tty hangup was
|
||||
/// doing the work my group signal was being credited for.
|
||||
///
|
||||
/// Two properties are therefore required of the grandchild, and each closes
|
||||
/// one leak in the fixture:
|
||||
///
|
||||
/// - it **ignores `SIGHUP`**, so the tty hangup cannot end it for us; and
|
||||
/// - it **busy-loops rather than sleeping**, so it is not blocked in a call
|
||||
/// that the session teardown would interrupt anyway.
|
||||
///
|
||||
/// With both, the probe separates cleanly: pid-only leaves the grandchild
|
||||
/// alive, `kill(-pgid)` does not. That is the only shape in which this test
|
||||
/// can fail for the reason it claims to test.
|
||||
///
|
||||
/// The `WATCHDOG` is the price of that hostility. A grandchild built to
|
||||
/// survive every signal we send also survives the harness: when this test
|
||||
/// legitimately fails -- as it does under mutation L1 and L2 -- it leaves a
|
||||
/// process spinning a core at PPID 1, and a panicking or killed test binary
|
||||
/// cannot clean up after itself. So the child carries its own deadline.
|
||||
/// `SIGKILL` because that is the one signal the fixture does not trap.
|
||||
#[test]
|
||||
fn grandchild_does_not_outlive_the_session() {
|
||||
let dir = tempdir("buzz-terminal-orphan");
|
||||
let pidfile = dir.join("grandchild.pid");
|
||||
let armed = dir.join("armed");
|
||||
|
||||
let pair = open_pty();
|
||||
let mut child = spawn_script(
|
||||
&pair,
|
||||
&format!(
|
||||
"sh -c 'trap \"\" HUP TERM; (sleep {WATCHDOG}; kill -9 $$) & \
|
||||
: > {armed}; while :; do :; done' & \
|
||||
echo $! > {pidfile}; wait",
|
||||
armed = armed.display(),
|
||||
pidfile = pidfile.display()
|
||||
),
|
||||
);
|
||||
let shell_pid = child.process_id().expect("pid") as i32;
|
||||
|
||||
let grandchild: i32 = read_when_written(&pidfile)
|
||||
.expect("grandchild never reported its pid")
|
||||
.parse()
|
||||
.expect("pid is a number");
|
||||
assert!(
|
||||
poll_until(|| armed.exists()),
|
||||
"grandchild never armed its SIGHUP trap; the tty hangup would kill it \
|
||||
regardless of how we signal, and this test could not observe the \
|
||||
difference"
|
||||
);
|
||||
assert!(
|
||||
pid_alive(grandchild),
|
||||
"test setup: the grandchild must be running before we shut down"
|
||||
);
|
||||
assert_ne!(
|
||||
grandchild, shell_pid,
|
||||
"test setup: the grandchild must be a distinct process, or this \
|
||||
cannot tell a group signal from a pid signal"
|
||||
);
|
||||
|
||||
shutdown(&mut child).expect("shutdown");
|
||||
|
||||
assert!(
|
||||
poll_until(|| !pid_alive(shell_pid)),
|
||||
"the session leader survived shutdown"
|
||||
);
|
||||
assert!(
|
||||
poll_until(|| !pid_alive(grandchild)),
|
||||
"an orphaned grandchild ({grandchild}) outlived the session -- the \
|
||||
signal reached the shell's pid but not its process group"
|
||||
);
|
||||
}
|
||||
|
||||
/// Shutting down an already-dead child is safe and reaps it.
|
||||
///
|
||||
/// Without the leading `try_wait`, this path signals a pid that the kernel may
|
||||
/// already have released and reassigned.
|
||||
#[test]
|
||||
fn shutdown_of_an_exited_child_is_a_reap_not_a_signal() {
|
||||
let pair = open_pty();
|
||||
let mut child = spawn_script(&pair, "exit 0");
|
||||
assert!(
|
||||
poll_until(|| child.try_wait().ok().flatten().is_some()),
|
||||
"child did not exit"
|
||||
);
|
||||
assert_eq!(
|
||||
shutdown(&mut child).expect("shutdown"),
|
||||
Shutdown::AlreadyExited
|
||||
);
|
||||
}
|
||||
|
||||
/// The login `argv[0]` the child **actually receives**, not the string we
|
||||
/// computed.
|
||||
///
|
||||
/// This closes the gap flagged in `e8b567aa`: `login_argv0` and
|
||||
/// `portable-pty`'s `as_command` (`cmdbuilder.rs:510-517`) were each verified
|
||||
/// by reading, and agreement-by-reading is not observation.
|
||||
///
|
||||
/// Two things make the probe terminate where a naive one hangs. The child
|
||||
/// writes `$0` to a **file** rather than the PTY -- so there is no terminal
|
||||
/// echo to strip, no ANSI to parse, and no dependency on the interactive
|
||||
/// shell ever reaching EOF. And the read is polled to a deadline. Credit to
|
||||
/// Quinn (`fcfd69b0`), whose three failed PTY-parsing harnesses established
|
||||
/// that the harness was the bug.
|
||||
///
|
||||
/// The explicit-prog row is the control that isolates login `argv[0]` as the
|
||||
/// only variable: same shell, same PTY, same fence, no `-` prefix.
|
||||
#[test]
|
||||
fn default_prog_child_observes_the_login_argv0() {
|
||||
let dir = tempdir("buzz-terminal-argv0");
|
||||
let shell = "/bin/sh";
|
||||
|
||||
let default_prog = observe_argv0(&dir.join("default"), shell, true);
|
||||
assert_eq!(
|
||||
default_prog,
|
||||
login_argv0(shell),
|
||||
"the child's $0 is not the login argv0 we computed"
|
||||
);
|
||||
assert!(
|
||||
default_prog.starts_with('-'),
|
||||
"a default-prog child must be a login shell: {default_prog:?}"
|
||||
);
|
||||
|
||||
let explicit = observe_argv0(&dir.join("explicit"), shell, false);
|
||||
assert_eq!(
|
||||
explicit, shell,
|
||||
"control: an explicitly-invoked shell must not be given a login argv0"
|
||||
);
|
||||
assert_ne!(
|
||||
default_prog, explicit,
|
||||
"control and subject agree, so this test cannot observe the login \
|
||||
prefix at all"
|
||||
);
|
||||
}
|
||||
|
||||
/// Spawns a `/bin/sh` that writes its own `$0` to `pidfile`, either as a
|
||||
/// default program (login argv0 applied by portable-pty) or explicitly.
|
||||
///
|
||||
/// The default-prog child is an *interactive* shell with no `-c`, so it is
|
||||
/// driven by writing to the PTY master -- the only way to give a login shell
|
||||
/// a command is to type one.
|
||||
fn observe_argv0(outfile: &std::path::Path, shell: &str, default_prog: bool) -> String {
|
||||
let pair = open_pty();
|
||||
let resolved = resolve_shell(Some(shell));
|
||||
let mut cmd = if default_prog {
|
||||
CommandBuilder::new_default_prog()
|
||||
} else {
|
||||
CommandBuilder::new(shell)
|
||||
};
|
||||
fence_env(&mut cmd, &user_shell_path(), &resolved);
|
||||
if !default_prog {
|
||||
cmd.arg("-c");
|
||||
cmd.arg(format!("printf '%s' \"$0\" > {}", outfile.display()));
|
||||
}
|
||||
|
||||
let mut child = pair.slave.spawn_command(cmd).expect("spawn");
|
||||
drain(&pair);
|
||||
drop(pair.slave);
|
||||
|
||||
if default_prog {
|
||||
use std::io::Write;
|
||||
let mut writer = pair.master.take_writer().expect("writer");
|
||||
writeln!(writer, "printf '%s' \"$0\" > {}", outfile.display()).expect("write");
|
||||
writer.flush().expect("flush");
|
||||
// Dropping the writer closes the master's write side, which the shell
|
||||
// reads as end-of-input and exits on -- no `exit` command needed, and
|
||||
// nothing depends on the shell's rc files having run.
|
||||
drop(writer);
|
||||
}
|
||||
|
||||
let observed = read_when_written(outfile);
|
||||
let _ = crate::lifecycle::shutdown(&mut child);
|
||||
observed.unwrap_or_else(|| panic!("child never reported $0 within {BOUND:?}"))
|
||||
}
|
||||
|
||||
/// A fresh directory for a test's artifacts, replacing any prior run's.
|
||||
fn tempdir(name: &str) -> std::path::PathBuf {
|
||||
let dir = std::env::temp_dir().join(name);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).expect("create temp dir");
|
||||
dir
|
||||
}
|
||||
|
||||
/// Mari's noisy-child discriminator: the reader must still be draining
|
||||
/// **while** the child is being terminated and reaped.
|
||||
///
|
||||
/// The property is an ordering, and an ordering cannot be observed from the
|
||||
/// outcome -- both orders end with a dead child and a stopped reader. So the
|
||||
/// reader records *when* it read, relative to the moment close begins, and the
|
||||
/// assertion is on bytes drained after that moment.
|
||||
///
|
||||
/// The child is deliberately noisy: it floods the PTY continuously, so a
|
||||
/// master that stops being read fills its kernel buffer within milliseconds
|
||||
/// and the child blocks in `write()`. That is the state the drain law exists
|
||||
/// to avoid, and a quiet child cannot produce it -- with nothing being
|
||||
/// written, both orders look identical and the test proves nothing.
|
||||
#[test]
|
||||
fn reader_drains_through_termination_and_reap() {
|
||||
let pair = open_pty();
|
||||
|
||||
// Flood, and keep flooding: `yes` writes until the pipe is closed or the
|
||||
// process dies, so there is always more output pending than the buffer
|
||||
// holds.
|
||||
let mut child = spawn_noisy(&pair);
|
||||
|
||||
let after_close = std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0));
|
||||
let closing = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let reader = RecordingReader::spawn(&pair, after_close.clone(), closing.clone());
|
||||
|
||||
// Let the child get well ahead of the reader before we touch anything.
|
||||
assert!(
|
||||
poll_until(|| after_close.load(Ordering::Relaxed) == 0 && reader.total_bytes() > 4096),
|
||||
"test setup: the child is not producing enough output to fill the pty \
|
||||
buffer, so this cannot distinguish drain order"
|
||||
);
|
||||
|
||||
closing.store(true, Ordering::Relaxed);
|
||||
let started = Instant::now();
|
||||
let outcome = shutdown_draining(&mut child, Box::new(reader)).expect("shutdown");
|
||||
let elapsed = started.elapsed();
|
||||
|
||||
assert_eq!(
|
||||
outcome,
|
||||
Shutdown::Terminated,
|
||||
"a `yes` pipeline dies on SIGTERM; SIGKILL here means it was wedged in \
|
||||
a tty write against an undrained master"
|
||||
);
|
||||
assert!(
|
||||
after_close.load(Ordering::Relaxed) > 0,
|
||||
"the reader drained nothing after close began -- it was joined before \
|
||||
termination, which is the ordering the drain law forbids"
|
||||
);
|
||||
assert!(
|
||||
elapsed < TERM_GRACE,
|
||||
"shutdown took {elapsed:?}, at or beyond the {TERM_GRACE:?} grace \
|
||||
period: the child was blocked writing to an undrained master rather \
|
||||
than exiting on SIGTERM"
|
||||
);
|
||||
}
|
||||
|
||||
/// Spawns a child that floods the PTY without pause.
|
||||
fn spawn_noisy(pair: &PtyPair) -> Box<dyn Child + Send + Sync> {
|
||||
let shell = resolve_shell(std::env::var("SHELL").ok().as_deref());
|
||||
let mut cmd = CommandBuilder::new("/bin/sh");
|
||||
fence_env(&mut cmd, &user_shell_path(), &shell);
|
||||
cmd.arg("-c");
|
||||
cmd.arg("while :; do echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; done");
|
||||
// Deliberately no `drain` here: this test owns the reader.
|
||||
pair.slave.spawn_command(cmd).expect("spawn")
|
||||
}
|
||||
|
||||
/// A [`DrainingReader`] that records how much it read after close began.
|
||||
struct RecordingReader {
|
||||
total: std::sync::Arc<std::sync::atomic::AtomicU64>,
|
||||
handle: std::thread::JoinHandle<()>,
|
||||
}
|
||||
|
||||
impl RecordingReader {
|
||||
fn spawn(
|
||||
pair: &PtyPair,
|
||||
after_close: std::sync::Arc<std::sync::atomic::AtomicU64>,
|
||||
closing: std::sync::Arc<std::sync::atomic::AtomicBool>,
|
||||
) -> Self {
|
||||
let mut reader = pair.master.try_clone_reader().expect("reader");
|
||||
let total = std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0));
|
||||
let counter = total.clone();
|
||||
let handle = std::thread::spawn(move || {
|
||||
use std::io::Read;
|
||||
let mut buf = [0u8; 4096];
|
||||
while let Ok(n) = reader.read(&mut buf) {
|
||||
if n == 0 {
|
||||
break;
|
||||
}
|
||||
counter.fetch_add(n as u64, Ordering::Relaxed);
|
||||
if closing.load(Ordering::Relaxed) {
|
||||
after_close.fetch_add(n as u64, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
});
|
||||
Self { total, handle }
|
||||
}
|
||||
|
||||
fn total_bytes(&self) -> u64 {
|
||||
self.total.load(Ordering::Relaxed)
|
||||
}
|
||||
}
|
||||
|
||||
impl DrainingReader for RecordingReader {
|
||||
fn join(self: Box<Self>) {
|
||||
// Bounded, and that is the whole point. The read loop ends when the
|
||||
// master reports EOF, which only happens once the reaped child has
|
||||
// released the slave -- so joining *before* termination blocks
|
||||
// forever. That is precisely the forbidden ordering (mutation L4),
|
||||
// and an unbounded join would "detect" it by hanging, which is
|
||||
// indistinguishable from a broken test. Waiting to a deadline and
|
||||
// abandoning the thread converts the hang into an assertion failure
|
||||
// the harness can report.
|
||||
if !poll_until(|| self.handle.is_finished()) {
|
||||
return;
|
||||
}
|
||||
let _ = self.handle.join();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user