From d896365d20b02e76a98c86b2985ddb9ea08be25e Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:33:00 +0200 Subject: [PATCH 1/8] tests: every process the harness has to end dies with the harness A QEMU the harness started ran for hours after its harness was gone: the harness stops QEMU only in `Drop`, and a SIGKILLed harness runs none, so its guest was reparented to init and ran on. The HTTPS judge's servers had the same shape. `toyos_build::tether::spawn` makes the child the controlling process of a pseudo-terminal whose master only the harness holds. The kernel closes the master when the harness dies by any signal, and the hangup sends the child SIGHUP, on macOS and Linux alike. QEMU and the judge's servers are spawned through it; `Drop`'s kill stays, so a clean end never waits on the hangup. Measured on this host (QEMU 11.1.1, a held `-S` guest, the holder SIGKILLed): a stdin pipe's EOF leaves QEMU running (alive 5 s later), a process group of its own leaves it running, `-run-with exit-with-parent=on` ends it in 3.7 ms, the pty ends it in 3.1 ms. The slave has to stay open in the child (closed at exec, QEMU survived), and SIGHUP has to be unblocked (inherited blocked, QEMU survived) and reset to default (inherited ignored, a child with no handler survived). `exit-with-parent` was rejected: on Linux it is PR_SET_PDEATHSIG, which follows the spawning thread, and it ends QEMU alone. `a_tethered_child_dies_with_its_owner` is the host arm; the harness's `guest_dies_with_its_harness` is the guest arm, whose owner is the harness itself re-run with `--hold`. Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 1 + src/sourcegate.rs | 12 ++- src/testargs.rs | 19 ++++ src/tether.rs | 236 +++++++++++++++++++++++++++++++++++++++++ tests/common/https.rs | 13 +-- tests/common/mod.rs | 1 + tests/common/orphan.rs | 34 ++++++ tests/common/qemu.rs | 11 +- tests/toyos.rs | 7 ++ 9 files changed, 324 insertions(+), 10 deletions(-) create mode 100644 src/tether.rs create mode 100644 tests/common/orphan.rs diff --git a/src/lib.rs b/src/lib.rs index 2ef6e4d30dc..66fb763cb58 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,6 +44,7 @@ pub mod sourcegate; pub mod stamps; pub mod sysroot; pub mod testargs; +pub mod tether; pub mod tiers; pub mod toolchain; pub mod userlandhost; diff --git a/src/sourcegate.rs b/src/sourcegate.rs index ba7fdb0624a..ba93cde64cc 100644 --- a/src/sourcegate.rs +++ b/src/sourcegate.rs @@ -630,9 +630,15 @@ const HOST_SPAWNS: &[Spawn] = &[ }, Spawn { arg: "std::env::current_exe().unwrap()", - sites: &[("src/buildlock.rs", 2), ("toyos-tmpdir/tests/reclaim.rs", 1)], - why: "a test binary re-running itself: the build system under the lock, and a \ - scratch holder whose death is what is judged", + sites: &[ + ("src/buildlock.rs", 2), + ("src/tether.rs", 1), + ("tests/common/orphan.rs", 1), + ("toyos-tmpdir/tests/reclaim.rs", 1), + ], + why: "a test binary re-running itself: the build system under the lock, and an \ + owner whose death is what is judged — of its scratch, its tethered child and \ + its guest", }, Spawn { arg: "env!(\"CARGO_BIN_EXE_toyos-cc\")", diff --git a/src/testargs.rs b/src/testargs.rs index 4eaaf0e4abc..bfb96dd1344 100644 --- a/src/testargs.rs +++ b/src/testargs.rs @@ -165,6 +165,9 @@ declare_flags!(pub SUITE = { /// machine is not touched**: the run builds the images and writes down what /// to run on them, or judges readbacks a driver already left there. pub METAL_READBACK = "--metal-readback", Next; + /// The owner `guest_dies_with_its_harness` kills: one guest, held until + /// stdin ends. Alone on its line. + pub HOLD = "--hold", None; }); /// Validate the harness's argv and return the run's filter. @@ -228,6 +231,13 @@ pub fn parse(args: &[String]) -> Result, String> { .to_string(), ); } + if has(&HOLD) && args.len() != 1 { + return Err( + "--hold boots one guest and holds it, and reads nothing else on the line; every \ + other word would be dropped in silence" + .to_string(), + ); + } Ok(filter) } @@ -493,6 +503,7 @@ mod tests { vec!["--metal"], vec!["--metal", "--metal-readback", "target/metal"], vec!["--metal", "--nightly"], + vec!["--hold"], ] { assert!(parse_owned(&argv).is_ok(), "{argv:?}"); } @@ -508,4 +519,12 @@ mod tests { let refusal = parse_owned(&["--metal", "--audio-gate", "30"]).unwrap_err(); assert!(refusal.contains("cannot be combined"), "{refusal}"); } + + #[test] + fn hold_is_alone_on_its_line() { + for argv in [&["--hold", "boot"][..], &["--hold", "--nightly"], &["-j", "2", "--hold"]] { + let refusal = parse_owned(argv).unwrap_err(); + assert!(refusal.contains("--hold"), "{argv:?}: {refusal}"); + } + } } diff --git a/src/tether.rs b/src/tether.rs new file mode 100644 index 00000000000..1c86d6a1a23 --- /dev/null +++ b/src/tether.rs @@ -0,0 +1,236 @@ +//! A host process the harness has to end cannot outlive the harness, however +//! the harness ends — `SIGKILL` included, which runs no `Drop`. +//! +//! [`spawn`] makes the child the controlling process of a pseudo-terminal whose +//! master only the spawner holds. The kernel closes the master when the +//! spawner dies, by any signal, and a terminal whose master closes is hung up: +//! its controlling process gets `SIGHUP`, on Linux and on macOS. +//! `PR_SET_PDEATHSIG` is Linux's alone and follows the spawning thread rather +//! than the process, and QEMU's `exit-with-parent` is that on Linux and ends +//! QEMU alone. +//! +//! A process that ends on its own — a build, a one-shot client — is not +//! spawned here: a build ended mid-way can leave a toolchain half-written. + +use std::io::{self, BufRead, BufReader, Read}; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; +use std::os::unix::process::CommandExt; +use std::process::{Child, ChildStdout, Command, Stdio}; +use std::sync::mpsc::{self, Receiver}; +use std::time::{Duration, Instant}; + +/// The master: dropping it hangs up the child's terminal. +pub struct Tether { + _master: OwnedFd, +} + +/// Spawn `cmd` as the controlling process of a terminal the returned +/// [`Tether`] holds. +pub fn spawn(mut cmd: Command) -> io::Result<(Child, Tether)> { + let flags = libc::O_RDWR | libc::O_NOCTTY | libc::O_CLOEXEC; + // SAFETY: a NUL-terminated path, and the descriptor is checked before it is owned. + let master = unsafe { + let fd = libc::open(c"/dev/ptmx".as_ptr(), flags); + if fd < 0 { + return Err(io::Error::last_os_error()); + } + OwnedFd::from_raw_fd(fd) + }; + // SAFETY: `master` is an open pseudo-terminal master. + if unsafe { libc::grantpt(master.as_raw_fd()) != 0 || libc::unlockpt(master.as_raw_fd()) != 0 } { + return Err(io::Error::last_os_error()); + } + let slave = peer(&master, flags)?; + let fd = slave.as_raw_fd(); + // SAFETY: system calls on the child's own state, between `fork` and `exec`, + // allocating nothing. + unsafe { + cmd.pre_exec(move || { + // A mask and a disposition both survive `exec`, and a child that + // installs no handler of its own would ignore a blocked or ignored + // hangup. + let mut hup: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset(&mut hup); + libc::sigaddset(&mut hup, libc::SIGHUP); + if libc::sigprocmask(libc::SIG_UNBLOCK, &hup, std::ptr::null_mut()) != 0 + || libc::signal(libc::SIGHUP, libc::SIG_DFL) == libc::SIG_ERR + || libc::setsid() < 0 + || libc::ioctl(fd, libc::TIOCSCTTY as _, 0) < 0 + // Open across `exec`: macOS hangs up only a terminal somebody + // holds open. + || libc::fcntl(fd, libc::F_SETFD, 0) < 0 + { + return Err(io::Error::last_os_error()); + } + // No hangup precedes the terminal becoming this child's: the child + // holds its own copy of the master until `exec` closes it. + Ok(()) + }); + } + let child = cmd.spawn()?; + Ok((child, Tether { _master: master })) +} + +/// The slave of `master`, opened with `flags`. +#[cfg(target_os = "linux")] +fn peer(master: &OwnedFd, flags: libc::c_int) -> io::Result { + // SAFETY: `master` is an unlocked pseudo-terminal master. + let fd = unsafe { libc::ioctl(master.as_raw_fd(), libc::TIOCGPTPEER, flags) }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: a descriptor the ioctl just opened. + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) +} + +/// The slave of `master`, opened with `flags`. +#[cfg(target_os = "macos")] +fn peer(master: &OwnedFd, flags: libc::c_int) -> io::Result { + let mut name = [0 as libc::c_char; 128]; + // SAFETY: `TIOCPTYGNAME` writes a NUL-terminated name of at most 128 bytes. + let fd = unsafe { + if libc::ioctl(master.as_raw_fd(), libc::TIOCPTYGNAME as _, name.as_mut_ptr()) < 0 { + return Err(io::Error::last_os_error()); + } + libc::open(name.as_ptr(), flags) + }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: a descriptor `open` just returned. + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) +} + +/// How long a tethered child may take to exit once its owner is gone: far +/// above a hangup's exit, and a child that outlives its owner never ends. +pub const WITHIN: Duration = Duration::from_secs(10); + +/// An owner whose tethered children a test watches die with it. +pub struct Owner { + child: Child, + said: BufReader, + /// The owner's stderr, read to its end and sent here: its end is every + /// process that holds it exited, the owner's children among them. + closed: Receiver, +} + +impl Owner { + /// Spawn `cmd` as an owner: its stdin a pipe only this process writes, so + /// it ends when this process does; its stdout read by [`Self::said`]; and + /// `SIGHUP` blocked and ignored, the worst a harness can hand down to what + /// it spawns. + pub fn spawn(mut cmd: Command) -> Result { + let (mut stderr, write) = io::pipe().map_err(|e| format!("a pipe for the owner's stderr: {e}"))?; + cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(write); + // SAFETY: two system calls on the child's own state, allocating nothing. + unsafe { + cmd.pre_exec(|| { + let mut hup: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset(&mut hup); + libc::sigaddset(&mut hup, libc::SIGHUP); + if libc::sigprocmask(libc::SIG_BLOCK, &hup, std::ptr::null_mut()) != 0 + || libc::signal(libc::SIGHUP, libc::SIG_IGN) == libc::SIG_ERR + { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } + let mut child = cmd.spawn().map_err(|e| format!("spawn the owner: {e}"))?; + // Its copy of the write end, which would otherwise be a holder too. + drop(cmd); + let (tx, closed) = mpsc::channel(); + std::thread::spawn(move || { + let mut text = Vec::new(); + let _ = stderr.read_to_end(&mut text); + let _ = tx.send(String::from_utf8_lossy(&text).into_owned()); + }); + let said = BufReader::new(child.stdout.take().expect("a piped stdout")); + Ok(Owner { child, said, closed }) + } + + /// The rest of the first line the owner prints on stdout that starts with + /// `prefix`. + pub fn said(&mut self, prefix: &str) -> Result { + let mut line = String::new(); + loop { + line.clear(); + match self.said.read_line(&mut line) { + Ok(0) => { + let stderr = self.closed.recv_timeout(WITHIN).unwrap_or_default(); + return Err(format!("the owner ended without saying {prefix:?}:\n{stderr}")); + } + Ok(_) => {} + Err(e) => return Err(format!("read the owner's stdout: {e}")), + } + if let Some(rest) = line.strip_prefix(prefix) { + return Ok(rest.trim_end().to_string()); + } + } + } + + /// `SIGKILL` the owner, and how long every process holding its stderr took + /// to exit after it. `Err` if one outlived it by [`WITHIN`], naming which + /// of `pids` still run, and ending them. + pub fn killed(mut self, pids: &[u32]) -> Result { + let killed = Instant::now(); + self.child.kill().map_err(|e| format!("SIGKILL the owner: {e}"))?; + self.child.wait().map_err(|e| format!("reap the owner: {e}"))?; + if self.closed.recv_timeout(WITHIN).is_ok() { + return Ok(killed.elapsed()); + } + // SAFETY: signal 0 asks whether the pid exists and delivers nothing. + let alive: Vec = + pids.iter().copied().filter(|&pid| unsafe { libc::kill(pid as i32, 0) } == 0).collect(); + for &pid in &alive { + // SAFETY: a process of this test's own that still holds its pid. + unsafe { libc::kill(pid as i32, libc::SIGKILL) }; + } + Err(format!( + "a process holding the owner's stderr still ran {WITHIN:?} after the owner's SIGKILL; \ + of its tethered children {pids:?}, {alive:?} did, and were killed" + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const TETHERED: &str = "tethered "; + + /// This test binary, running the one test `name`. + fn this_test(name: &str) -> Command { + let mut cmd = Command::new(std::env::current_exe().unwrap()); + cmd.args(["--exact", name, "--include-ignored", "--nocapture"]); + cmd + } + + #[test] + #[ignore = "the owner `a_tethered_child_dies_with_its_owner` kills; never runs on its own"] + fn owner() { + let mut parked = this_test("tether::tests::parked"); + parked.stdout(Stdio::null()); + let (child, _tether) = spawn(parked).expect("spawn the tethered child"); + println!("{TETHERED}{}", child.id()); + io::stdin().read_to_end(&mut Vec::new()).expect("read the owner's stdin"); + } + + /// A child that never ends on its own while the test runs: its stdin is + /// the test's pipe, which the owner's death does not close. + #[test] + #[ignore = "the tethered child of `owner`; never runs on its own"] + fn parked() { + io::stdin().read_to_end(&mut Vec::new()).expect("read the parked child's stdin"); + } + + /// The owner's `SIGKILL` ends its tethered child, which inherited `SIGHUP` + /// blocked and ignored. + #[test] + fn a_tethered_child_dies_with_its_owner() { + let mut owner = Owner::spawn(this_test("tether::tests::owner")).unwrap_or_else(|e| panic!("{e}")); + let pid: u32 = owner.said(TETHERED).unwrap_or_else(|e| panic!("{e}")).parse().expect("a pid"); + let took = owner.killed(&[pid]).unwrap_or_else(|e| panic!("{e}")); + eprintln!("tethered child {pid} gone {took:?} after its owner's SIGKILL"); + } +} diff --git a/tests/common/https.rs b/tests/common/https.rs index 198a04988a3..3819d1e024c 100644 --- a/tests/common/https.rs +++ b/tests/common/https.rs @@ -14,6 +14,7 @@ use std::time::Duration; use super::qemu::{self, BootOptions, QemuInstance}; use super::{compile, serial}; +use toyos_build::tether::Tether; /// Where the host arm sees the servers the guest reaches at /// [`qemu::GUEST_VIEW_OF_HOST`]. The judge's certificate carries both. @@ -277,6 +278,8 @@ fn fetch_on_host(url: &str, ca: &Path) -> Result { /// The host servers, killed when this goes out of scope. struct Server { child: Child, + /// What ends the servers when this process dies without dropping this. + _tether: Tether, ca: PathBuf, body_bytes: usize, body_sha: String, @@ -287,11 +290,9 @@ impl Server { fn start() -> Result { let out = super::lane::dir().join("https-judge"); std::fs::create_dir_all(&out).map_err(|e| format!("create {}: {e}", out.display()))?; - let mut child = Command::new(toyos_build::build::https_test_server(&compile::repo_root())) - .arg("--out") - .arg(&out) - .stdout(Stdio::piped()) - .spawn() + let mut cmd = Command::new(toyos_build::build::https_test_server(&compile::repo_root())); + cmd.arg("--out").arg(&out).stdout(Stdio::piped()); + let (mut child, tether) = toyos_build::tether::spawn(cmd) .map_err(|e| format!("start the judge's servers: {e}"))?; let stdout = child.stdout.take().expect("a piped stdout"); @@ -322,7 +323,7 @@ impl Server { let _ = child.kill(); return Err("the judge's servers never announced a CA and a body".to_string()); }; - Ok(Server { child, ca, body_bytes, body_sha, ports }) + Ok(Server { child, _tether: tether, ca, body_bytes, body_sha, ports }) } fn port(&self, role: &str) -> Result { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 0c64deee567..a47a6918c62 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -41,6 +41,7 @@ pub mod logstream; pub mod metal; #[allow(dead_code)] pub mod origin; +pub mod orphan; #[allow(dead_code)] pub mod partclaim; #[allow(dead_code)] diff --git a/tests/common/orphan.rs b/tests/common/orphan.rs new file mode 100644 index 00000000000..bf52ccb809f --- /dev/null +++ b/tests/common/orphan.rs @@ -0,0 +1,34 @@ +//! A guest a `SIGKILL`ed harness would leave running: the owner, and the test +//! that kills it and watches its QEMU go. + +use std::io::Read; +use std::path::Path; +use std::process::Command; + +use toyos_build::tether::Owner; +use toyos_build::testargs; + +use super::qemu::QemuInstance; + +/// What the owner prints its QEMU's pid after. +const HELD: &str = "held: qemu "; + +/// `--hold`: boot one guest, print its QEMU's pid, and hold it until stdin +/// ends. +pub fn hold(test_config: &Path) { + let guest = QemuInstance::boot(test_config, &[], &[]); + println!("{HELD}{}", guest.pid()); + std::io::stdin().read_to_end(&mut Vec::new()).expect("read the owner's stdin"); +} + +/// The harness's `SIGKILL` ends its guest, whose QEMU inherited `SIGHUP` +/// blocked and ignored. +pub fn guest_dies_with_its_harness() -> Result<(), String> { + let mut owner = Command::new(std::env::current_exe().unwrap()); + owner.arg(testargs::HOLD.name); + let mut owner = Owner::spawn(owner)?; + let pid: u32 = owner.said(HELD)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; + let took = owner.killed(&[pid])?; + eprintln!(" [orphan] QEMU {pid} gone {took:?} after its harness's SIGKILL"); + Ok(()) +} diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index ed205a58079..72fab3ba9b1 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -10,6 +10,7 @@ use std::{fs, thread}; use super::compile; use toyos_build::arch::{Accel, Arch}; +use toyos_build::tether::Tether; /// The architecture every machine this suite builds and boots is: the suite's /// q35 shapes, i8042 and VT-d are x86-64's, and the aarch64 bring-up boots @@ -2637,6 +2638,8 @@ impl ConsoleStream { pub struct QemuInstance { child: Child, + /// What ends QEMU when this process dies without dropping this. + _tether: Tether, stdin: BufWriter>, rx: Receiver, console: ConsoleStream, @@ -3356,6 +3359,11 @@ impl QemuInstance { } } + /// The QEMU process's pid. + pub fn pid(&self) -> u32 { + self.child.id() + } + /// Every console line the guest printed before the ready marker. /// /// The kernel's own boot lines sit in the log ring until the scheduler @@ -4901,7 +4909,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) if VERBOSE.load(Ordering::Relaxed) { eprintln!("[qemu {seq}] Launching QEMU..."); } - let mut child = qemu.spawn().expect("Failed to launch QEMU"); + let (mut child, tether) = toyos_build::tether::spawn(qemu).expect("Failed to launch QEMU"); let stdin: Box = match input { Some(fifo) => Box::new(fifo), @@ -4972,6 +4980,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) LIVE.fetch_add(1, Ordering::SeqCst); QemuInstance { child, + _tether: tether, stdin, rx, _reader_thread: reader_thread, diff --git a/tests/toyos.rs b/tests/toyos.rs index f2ed0e90973..4e1a402bc1e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -667,6 +667,7 @@ const GRAFFITI: [u8; 3] = [0x00, 0xC0, 0x00]; /// tidy. const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ ("ioapic_topology", Sched::Parallel, Tier::Fast), + ("guest_dies_with_its_harness", Sched::Parallel, Tier::Fast), // The interrupt census adds up, and every device interrupt is still cpu0's. // **The second half is what makes this the track's instrument rather than a // tidiness check**: it states the present-state fact @@ -14213,6 +14214,7 @@ fn run_machine_test( control_regs(qemu.boot_log(), CPUS) } "control_regs_negative" => control_regs_negative(test_config, c_bins, rust_bins), + "guest_dies_with_its_harness" => common::orphan::guest_dies_with_its_harness(), "smp_roster_and_tsc_trail" => { // Eight, which is the T14's own count and this suite's ceiling. const CPUS: u32 = 8; @@ -20369,6 +20371,11 @@ fn main() { // this run's scratch, green or red; taking it reclaims what killed runs left. let run = common::lane::Run::begin(); + if SUITE.present(&args, &testargs::HOLD) { + common::orphan::hold(&Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/testcases")); + run.exit(0); + } + // How many guests may be up on the *host* at once, across every worktree. // `--jobs` is this run's demand; this is what the machine will supply, and // zero turns it off. From 70d36da11705cfc9911e6ee96adbaadb44882757 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:34:41 +0200 Subject: [PATCH 2/8] tether: hold the owner's stdin past the verdict `Child::wait` closes the child's stdin, and the tethered child the host arm parks reads that pipe to its end: with the mechanism reverted the child still exited 4.6 ms after its owner's SIGKILL, on the pipe's EOF rather than on a hangup, and the arm stayed green. Co-Authored-By: Claude Opus 5.5 --- src/tether.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tether.rs b/src/tether.rs index 1c86d6a1a23..fc10fb15399 100644 --- a/src/tether.rs +++ b/src/tether.rs @@ -175,6 +175,9 @@ impl Owner { pub fn killed(mut self, pids: &[u32]) -> Result { let killed = Instant::now(); self.child.kill().map_err(|e| format!("SIGKILL the owner: {e}"))?; + // Held past the verdict: `wait` would close it, and a child reading + // it would end on that instead of on its tether. + let _stdin = self.child.stdin.take(); self.child.wait().map_err(|e| format!("reap the owner: {e}"))?; if self.closed.recv_timeout(WITHIN).is_ok() { return Ok(killed.elapsed()); From aba9a955ed45cf1adbcb064b27bcb2deb320638c Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:41:44 +0200 Subject: [PATCH 3/8] CLAUDE.md: agents never run QEMU, and leave nothing running The owner's rules: only the orchestrator runs guest tests, and an agent cleans up every process it started before it reports. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 577449ced1b..066f1efc7e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,7 +69,7 @@ The bar is not yet the tree. The standing failures are declared rather than remo The testing rules live where they are enforced: known reds in `src/redlist.rs`, tiers in `src/tiers.rs`, the PR gate and the nightly in `.github/workflows/`. Operationally: - `cargo run` builds everything (toolchain, kernel, bootloader, userland, image) and launches QEMU; `--build-only` skips the launch. `cargo test` runs the QEMU harness; `cargo test --workspace --exclude toyos-build` runs every host-crate suite. -- **Agents verify through `cargo test`, never `cargo run`** — the run path opens a QEMU window on the owner's desktop by design; the harness runs headless. +- **Agents never run QEMU.** An agent verifies with host tests and builds the image at most; the orchestrator runs every guest test, one suite at a time, and an agent reports only once nothing it started is still running. - **Both produce large output**: run them in the background and read the output file — `[N characters truncated]` means data was lost. A full boot is under a second; incremental builds finish in seconds. ## Repository layout From 930396ed8e1ce5469168ccb7e5418456907cc51a Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 23:04:33 +0200 Subject: [PATCH 4/8] implementer.md: an implementer runs host tests only, and hands guest arms to the orchestrator Co-Authored-By: Claude Opus 5.5 --- .claude/agents/implementer.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.claude/agents/implementer.md b/.claude/agents/implementer.md index f1b1d6b9447..007f4c59d72 100644 --- a/.claude/agents/implementer.md +++ b/.claude/agents/implementer.md @@ -19,7 +19,8 @@ one clause; do not work around it. Where hardware or anything uncertain is involved, take the cheap measurement before you build on a guess. Then build, then test before anyone reviews: -- `cargo test`, never `cargo run`: the run path opens a window on the owner's desktop. +- Host tests only: an implementer never runs QEMU. Every guest arm goes to the orchestrator as + the exact command, the patch file and the expected outcome with its named reason. - A result is the command's own exit code: ` > 2>&1; echo EXIT=$?`. A grepped `test result` line is not one, and a gate you did not run is a gate you do not claim. - Long commands run in the background with output to a file under the job scratchpad the brief From c5afc870c3fa9bb694d9efc4e4c6a63dcf036657 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 23:18:17 +0200 Subject: [PATCH 5/8] tether: the owner's wait is bounded, and the guest arm's scratch goes with it `Owner::said` read the owner's stdout with `read_line`, unbounded, and matched with `strip_prefix`. With one libtest thread the pretty formatter starts the owner's line with `test tether::tests::owner ... `, so the prefix was never found and the host arm hung with no verdict: `RUST_TEST_THREADS=1` at 930396ed was still running after 60 s. The owner's stdout is now read by a thread into a channel, `said` takes its bound and waits with `recv_timeout`, and the prefix is matched with `split_once` wherever on the line it starts. The owner is run with `--test-threads 1` explicitly. An `Owner` dropped is killed and reaped, so an owner a failed wait left running goes with the test. The guest arm's owner took a `toyos_tmpdir` root in the inherited `$TMPDIR`, and its SIGKILL left it there for `ci guest`'s "nothing left in $TMPDIR" step. The owner's `$TMPDIR` is now a `TempDir` the test holds and drops after the verdict. The test also builds the image there and hands it to the owner (`--hold `, booted `Staged::Pristine`), so the owner builds nothing: its one wait is its boot, which its own `wait_for_ready` ceiling ends at 10 s x its width floor of 2 x a fresh process's host scale of 1 x the default two vCPUs' oversubscription of at most 2, 40 s, inside `qemu::GUEST_WEDGED`, which bounds `said`. `WITHIN` is private. `spawn`'s contract names what the child owes; the module's opening overclaim and `pid`'s narrating doc are gone; the invariant the guest arm's green rests on, QEMU inheriting the harness's stderr, is stated where it is set. Co-Authored-By: Claude Opus 5.5 --- src/testargs.rs | 23 ++++++++----- src/tether.rs | 75 +++++++++++++++++++++++++++--------------- tests/common/orphan.rs | 27 ++++++++++----- tests/common/qemu.rs | 2 +- tests/toyos.rs | 9 +++-- 5 files changed, 89 insertions(+), 47 deletions(-) diff --git a/src/testargs.rs b/src/testargs.rs index bfb96dd1344..1484005730e 100644 --- a/src/testargs.rs +++ b/src/testargs.rs @@ -165,9 +165,9 @@ declare_flags!(pub SUITE = { /// machine is not touched**: the run builds the images and writes down what /// to run on them, or judges readbacks a driver already left there. pub METAL_READBACK = "--metal-readback", Next; - /// The owner `guest_dies_with_its_harness` kills: one guest, held until - /// stdin ends. Alone on its line. - pub HOLD = "--hold", None; + /// The owner `guest_dies_with_its_harness` kills: the image it names, + /// booted and held until stdin ends. Alone on its line. + pub HOLD = "--hold", Next; }); /// Validate the harness's argv and return the run's filter. @@ -188,6 +188,7 @@ pub fn parse(args: &[String]) -> Result, String> { if let Some(refusal) = line.malformed() { return Err(refusal); } + let flags = line.seen.len(); let mut filter: Option<&str> = None; for word in line.positionals { @@ -231,10 +232,10 @@ pub fn parse(args: &[String]) -> Result, String> { .to_string(), ); } - if has(&HOLD) && args.len() != 1 { + if has(&HOLD) && (flags != 1 || filter.is_some()) { return Err( - "--hold boots one guest and holds it, and reads nothing else on the line; every \ - other word would be dropped in silence" + "--hold boots the image it names and holds it, and reads nothing else on the line; \ + every other word would be dropped in silence" .to_string(), ); } @@ -503,7 +504,8 @@ mod tests { vec!["--metal"], vec!["--metal", "--metal-readback", "target/metal"], vec!["--metal", "--nightly"], - vec!["--hold"], + vec!["--hold", "boot.img"], + vec!["--hold=boot.img"], ] { assert!(parse_owned(&argv).is_ok(), "{argv:?}"); } @@ -522,7 +524,12 @@ mod tests { #[test] fn hold_is_alone_on_its_line() { - for argv in [&["--hold", "boot"][..], &["--hold", "--nightly"], &["-j", "2", "--hold"]] { + for argv in [ + &["--hold", "boot.img", "boot"][..], + &["--hold", "boot.img", "--nightly"], + &["-j", "2", "--hold", "boot.img"], + &["--hold"], + ] { let refusal = parse_owned(argv).unwrap_err(); assert!(refusal.contains("--hold"), "{argv:?}: {refusal}"); } diff --git a/src/tether.rs b/src/tether.rs index fc10fb15399..9dcb80e0de3 100644 --- a/src/tether.rs +++ b/src/tether.rs @@ -1,6 +1,3 @@ -//! A host process the harness has to end cannot outlive the harness, however -//! the harness ends — `SIGKILL` included, which runs no `Drop`. -//! //! [`spawn`] makes the child the controlling process of a pseudo-terminal whose //! master only the spawner holds. The kernel closes the master when the //! spawner dies, by any signal, and a terminal whose master closes is hung up: @@ -15,8 +12,8 @@ use std::io::{self, BufRead, BufReader, Read}; use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; use std::os::unix::process::CommandExt; -use std::process::{Child, ChildStdout, Command, Stdio}; -use std::sync::mpsc::{self, Receiver}; +use std::process::{Child, Command, Stdio}; +use std::sync::mpsc::{self, Receiver, RecvTimeoutError}; use std::time::{Duration, Instant}; /// The master: dropping it hangs up the child's terminal. @@ -25,7 +22,8 @@ pub struct Tether { } /// Spawn `cmd` as the controlling process of a terminal the returned -/// [`Tether`] holds. +/// [`Tether`] holds; the child owes it an exit on `SIGHUP`, the inherited +/// descriptor kept open, and no session of its own. pub fn spawn(mut cmd: Command) -> io::Result<(Child, Tether)> { let flags = libc::O_RDWR | libc::O_NOCTTY | libc::O_CLOEXEC; // SAFETY: a NUL-terminated path, and the descriptor is checked before it is owned. @@ -101,14 +99,16 @@ fn peer(master: &OwnedFd, flags: libc::c_int) -> io::Result { Ok(unsafe { OwnedFd::from_raw_fd(fd) }) } -/// How long a tethered child may take to exit once its owner is gone: far -/// above a hangup's exit, and a child that outlives its owner never ends. -pub const WITHIN: Duration = Duration::from_secs(10); +/// How long a step that waits on nothing but process creation and exit may +/// take: far above a spawn's or a hangup's, and a step that is stuck never ends. +const WITHIN: Duration = Duration::from_secs(10); -/// An owner whose tethered children a test watches die with it. +/// An owner whose tethered children a test watches die with it. Dropped, it is +/// killed and reaped, so an owner a failed wait left running goes with the test. pub struct Owner { child: Child, - said: BufReader, + /// The owner's stdout, line by line; disconnected at its end. + said: Receiver>, /// The owner's stderr, read to its end and sent here: its end is every /// process that holds it exited, the owner's children among them. closed: Receiver, @@ -145,26 +145,38 @@ impl Owner { let _ = stderr.read_to_end(&mut text); let _ = tx.send(String::from_utf8_lossy(&text).into_owned()); }); - let said = BufReader::new(child.stdout.take().expect("a piped stdout")); + let stdout = child.stdout.take().expect("a piped stdout"); + let (tx, said) = mpsc::channel(); + std::thread::spawn(move || { + for line in BufReader::new(stdout).lines() { + if tx.send(line).is_err() { + return; + } + } + }); Ok(Owner { child, said, closed }) } - /// The rest of the first line the owner prints on stdout that starts with - /// `prefix`. - pub fn said(&mut self, prefix: &str) -> Result { - let mut line = String::new(); + /// The rest of the first line the owner prints on stdout after `prefix`, + /// wherever on the line it starts: libtest may have begun the line. `Err` + /// if the owner ends first, or has not said it `within`. + pub fn said(&mut self, prefix: &str, within: Duration) -> Result { + let deadline = Instant::now() + within; loop { - line.clear(); - match self.said.read_line(&mut line) { - Ok(0) => { + match self.said.recv_timeout(deadline.saturating_duration_since(Instant::now())) { + Ok(Ok(line)) => { + if let Some((_, rest)) = line.split_once(prefix) { + return Ok(rest.to_string()); + } + } + Ok(Err(e)) => return Err(format!("read the owner's stdout: {e}")), + Err(RecvTimeoutError::Timeout) => { + return Err(format!("the owner had not said {prefix:?} within {within:?}")); + } + Err(RecvTimeoutError::Disconnected) => { let stderr = self.closed.recv_timeout(WITHIN).unwrap_or_default(); return Err(format!("the owner ended without saying {prefix:?}:\n{stderr}")); } - Ok(_) => {} - Err(e) => return Err(format!("read the owner's stdout: {e}")), - } - if let Some(rest) = line.strip_prefix(prefix) { - return Ok(rest.trim_end().to_string()); } } } @@ -196,16 +208,24 @@ impl Owner { } } +impl Drop for Owner { + fn drop(&mut self) { + // `Ok` on an owner already reaped: its pid is not signalled again. + self.child.kill().expect("SIGKILL the owner"); + self.child.wait().expect("reap the owner"); + } +} + #[cfg(test)] mod tests { use super::*; const TETHERED: &str = "tethered "; - /// This test binary, running the one test `name`. + /// This test binary, running the one test `name` on one thread. fn this_test(name: &str) -> Command { let mut cmd = Command::new(std::env::current_exe().unwrap()); - cmd.args(["--exact", name, "--include-ignored", "--nocapture"]); + cmd.args(["--exact", name, "--include-ignored", "--nocapture", "--test-threads", "1"]); cmd } @@ -232,7 +252,8 @@ mod tests { #[test] fn a_tethered_child_dies_with_its_owner() { let mut owner = Owner::spawn(this_test("tether::tests::owner")).unwrap_or_else(|e| panic!("{e}")); - let pid: u32 = owner.said(TETHERED).unwrap_or_else(|e| panic!("{e}")).parse().expect("a pid"); + let pid: u32 = + owner.said(TETHERED, WITHIN).unwrap_or_else(|e| panic!("{e}")).parse().expect("a pid"); let took = owner.killed(&[pid]).unwrap_or_else(|e| panic!("{e}")); eprintln!("tethered child {pid} gone {took:?} after its owner's SIGKILL"); } diff --git a/tests/common/orphan.rs b/tests/common/orphan.rs index bf52ccb809f..61eb6f5f0ee 100644 --- a/tests/common/orphan.rs +++ b/tests/common/orphan.rs @@ -7,27 +7,38 @@ use std::process::Command; use toyos_build::tether::Owner; use toyos_build::testargs; +use toyos_tmpdir::TempDir; -use super::qemu::QemuInstance; +use super::qemu::{self, BootOptions, QemuInstance, Staged}; /// What the owner prints its QEMU's pid after. const HELD: &str = "held: qemu "; -/// `--hold`: boot one guest, print its QEMU's pid, and hold it until stdin -/// ends. -pub fn hold(test_config: &Path) { - let guest = QemuInstance::boot(test_config, &[], &[]); +/// `--hold `: boot `image`, print its QEMU's pid, and hold it until +/// stdin ends. +pub fn hold(test_config: &Path, image: &Path) { + let options = BootOptions { boot_image: Some(Staged::Pristine(image.to_path_buf())), ..Default::default() }; + let guest = QemuInstance::boot_with_options(test_config, &[], &[], options); println!("{HELD}{}", guest.pid()); std::io::stdin().read_to_end(&mut Vec::new()).expect("read the owner's stdin"); } /// The harness's `SIGKILL` ends its guest, whose QEMU inherited `SIGHUP` /// blocked and ignored. -pub fn guest_dies_with_its_harness() -> Result<(), String> { +pub fn guest_dies_with_its_harness(test_config: &Path) -> Result<(), String> { + // The owner's `$TMPDIR` and the image it boots: what its `SIGKILL` leaves + // there goes when this does, after the owner. + let tmp = TempDir::new("orphan"); + let image = tmp.join("boot.img"); + std::fs::write(&image, qemu::build_boot_image(test_config, &[], &[], &[])) + .map_err(|e| format!("write {}: {e}", image.display()))?; let mut owner = Command::new(std::env::current_exe().unwrap()); - owner.arg(testargs::HOLD.name); + owner.arg(testargs::HOLD.name).arg(&image).env("TMPDIR", &tmp); let mut owner = Owner::spawn(owner)?; - let pid: u32 = owner.said(HELD)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; + // The owner builds nothing, so its one wait is its boot, whose own ceiling + // ends it well inside the backstop on any wait on a guest. + let pid: u32 = + owner.said(HELD, qemu::GUEST_WEDGED)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; let took = owner.killed(&[pid])?; eprintln!(" [orphan] QEMU {pid} gone {took:?} after its harness's SIGKILL"); Ok(()) diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 72fab3ba9b1..7f904038ada 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -3359,7 +3359,6 @@ impl QemuInstance { } } - /// The QEMU process's pid. pub fn pid(&self) -> u32 { self.child.id() } @@ -4894,6 +4893,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) console_file, } = files; + // Inherited: `orphan` reads QEMU's exit as the end of its harness's stderr. qemu.stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()); diff --git a/tests/toyos.rs b/tests/toyos.rs index 4e1a402bc1e..4a626517d04 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -14214,7 +14214,7 @@ fn run_machine_test( control_regs(qemu.boot_log(), CPUS) } "control_regs_negative" => control_regs_negative(test_config, c_bins, rust_bins), - "guest_dies_with_its_harness" => common::orphan::guest_dies_with_its_harness(), + "guest_dies_with_its_harness" => common::orphan::guest_dies_with_its_harness(test_config), "smp_roster_and_tsc_trail" => { // Eight, which is the T14's own count and this suite's ceiling. const CPUS: u32 = 8; @@ -20371,8 +20371,11 @@ fn main() { // this run's scratch, green or red; taking it reclaims what killed runs left. let run = common::lane::Run::begin(); - if SUITE.present(&args, &testargs::HOLD) { - common::orphan::hold(&Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/testcases")); + if let Some(image) = SUITE.value(&args, &testargs::HOLD) { + common::orphan::hold( + &Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/testcases"), + Path::new(image), + ); run.exit(0); } From 6d82ff9fad2890bb81ce4a2f707a51bdf7f27614 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 01:00:04 +0200 Subject: [PATCH 6/8] tether: the guest arm takes the owner's /tmp root, and a survivor is killed through the owner's group Review round 2 of #555. B4: on main every boot takes `TempDir::short("boot")`, so the SIGKILLed `--hold` owner leaves `/tmp/toyos-tmp--`, and the parent has already made its one sweep of `/tmp`; `left_behind` in the guest job then reds. `TempDir::adopt(base, pid)` moves every gone root of `pid` under `base` into a directory of the caller's, under the base's lock, so it goes when that does; `guest_dies_with_its_harness` adopts the owner's roots into a `TempDir::short` after its verdict, on every path, the owner already reaped. `an_adopted_root_goes_with_its_adopter` covers it: a live root and a gone root of pid `0` stay. Controls, as checked patches that build: `adopt` moving nothing reds with "was not adopted"; the prefix without its trailing `-` reds with "another pid's root was adopted". Owner: the stderr pipe is std's `Stdio::piped()`, taken from the child, so the verdict no longer rests on a dropped `Command` closing a write end. The owner runs in a process group of its own. On a failed wait it is not yet reaped, so its pid, and the group it leads, can be no other process's; the group is SIGKILLed rather than `pids` by number, whose owner's death let init reap them and their numbers go free. The refusal then says whether every holder of the stderr exited. Control, a checked patch that builds: the unit test's owner spawning `parked` untethered exits 101 in 10.02 s naming the child's pid, "the owner's process group was killed, and every holder of its stderr then exited", and no `parked` process remains. The orphan test's comment claiming its SIGKILL's leavings go with its `$TMPDIR` is deleted: false once the owner has a `/tmp` root. Filed: the macOS window before a pipe is close-on-exec can red the tether tests; `src/tether.rs` as a fourth Unix-only subsystem in the Windows issue. Co-Authored-By: Claude Opus 5.5 --- ...-sibling-spawn-and-red-the-tether-tests.md | 29 ++++++++++ ...uild-system-does-not-compile-on-windows.md | 2 + src/tether.rs | 57 ++++++++++++------- tests/common/orphan.rs | 20 ++++--- toyos-tmpdir/src/lib.rs | 17 ++++++ toyos-tmpdir/tests/reclaim.rs | 31 ++++++++++ 6 files changed, 130 insertions(+), 26 deletions(-) create mode 100644 issues/build/a-pipe-made-on-macos-can-leak-into-a-sibling-spawn-and-red-the-tether-tests.md diff --git a/issues/build/a-pipe-made-on-macos-can-leak-into-a-sibling-spawn-and-red-the-tether-tests.md b/issues/build/a-pipe-made-on-macos-can-leak-into-a-sibling-spawn-and-red-the-tether-tests.md new file mode 100644 index 00000000000..758b6f8813c --- /dev/null +++ b/issues/build/a-pipe-made-on-macos-can-leak-into-a-sibling-spawn-and-red-the-tether-tests.md @@ -0,0 +1,29 @@ +--- +status: open +kind: tooling +opened: 2026-09-29 +--- + +# A pipe made on macOS can leak into a sibling's spawn and red the tether tests + +`src/tether.rs`'s `Owner` judges its tethered children by the end of the +owner's stderr pipe: the end is every holder exited. On macOS, std makes that +pipe with `pipe()` and marks it close-on-exec afterwards +(`library/std/src/sys/pipe/unix.rs`: `pipe2` is only used on the targets that +have it, and macOS does not). A process another thread of the same test binary +spawns in that window inherits the write end and holds it until it exits. + +Both arms run beside sibling spawns: `a_tethered_child_dies_with_its_owner` +in `cargo test -p toyos-build --lib`, whose other tests spawn processes, and +`guest_dies_with_its_harness`, a `Sched::Parallel` test in the harness, beside +compiles and other guests' QEMUs. Such a leak turns a working tether into a +red: the pipe does not end within `WITHIN`, the owner's group is killed, and +the refusal says a holder outside that group still ran. +`toyos-tmpdir/tests/reclaim.rs` serialises its own spawns (`SPAWNING`) against +exactly this, and nothing here does. + +Owner: whoever next changes `src/tether.rs`. Exit condition: the verdict no +longer depends on the end of a pipe made on macOS without close-on-exec — the +pipe made atomically close-on-exec, or every spawn in each process that runs an +`Owner` serialised against its making, or a verdict read off something other +than a pipe's end. diff --git a/issues/build/the-build-system-does-not-compile-on-windows.md b/issues/build/the-build-system-does-not-compile-on-windows.md index ab6133ff861..6239c53df52 100644 --- a/issues/build/the-build-system-does-not-compile-on-windows.md +++ b/issues/build/the-build-system-does-not-compile-on-windows.md @@ -25,6 +25,8 @@ there. Every other crate in the graph, first-party and third-party, checked clean. The `#[cfg(unix)]` at `src/ci.rs:489` is still the only conditional compilation in the build system. +`src/tether.rs` is a fourth: `std::os::unix` and a pseudo-terminal per child, behind a Linux and macOS `cfg` pair with no Windows arm; `portability-windows` in run 36351950439 fails on it. + ## The judge, and it needs no Windows host and no download ``` diff --git a/src/tether.rs b/src/tether.rs index 9dcb80e0de3..29eef55d35f 100644 --- a/src/tether.rs +++ b/src/tether.rs @@ -116,12 +116,12 @@ pub struct Owner { impl Owner { /// Spawn `cmd` as an owner: its stdin a pipe only this process writes, so - /// it ends when this process does; its stdout read by [`Self::said`]; and - /// `SIGHUP` blocked and ignored, the worst a harness can hand down to what - /// it spawns. + /// it ends when this process does; its stdout read by [`Self::said`]; in a + /// process group of its own, which a tethered child leaves and an + /// untethered one stays in; and `SIGHUP` blocked and ignored, the worst a + /// harness can hand down to what it spawns. pub fn spawn(mut cmd: Command) -> Result { - let (mut stderr, write) = io::pipe().map_err(|e| format!("a pipe for the owner's stderr: {e}"))?; - cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(write); + cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).process_group(0); // SAFETY: two system calls on the child's own state, allocating nothing. unsafe { cmd.pre_exec(|| { @@ -137,8 +137,7 @@ impl Owner { }); } let mut child = cmd.spawn().map_err(|e| format!("spawn the owner: {e}"))?; - // Its copy of the write end, which would otherwise be a holder too. - drop(cmd); + let mut stderr = child.stderr.take().expect("a piped stderr"); let (tx, closed) = mpsc::channel(); std::thread::spawn(move || { let mut text = Vec::new(); @@ -181,30 +180,50 @@ impl Owner { } } + /// The owner's pid, which names what it leaves behind. + pub fn pid(&self) -> u32 { + self.child.id() + } + /// `SIGKILL` the owner, and how long every process holding its stderr took /// to exit after it. `Err` if one outlived it by [`WITHIN`], naming which - /// of `pids` still run, and ending them. + /// of `pids` still answer; the owner's group is then killed, and the + /// refusal says whether every holder went with it. pub fn killed(mut self, pids: &[u32]) -> Result { let killed = Instant::now(); self.child.kill().map_err(|e| format!("SIGKILL the owner: {e}"))?; // Held past the verdict: `wait` would close it, and a child reading // it would end on that instead of on its tether. let _stdin = self.child.stdin.take(); + let verdict = match self.closed.recv_timeout(WITHIN) { + Ok(_) => Ok(killed.elapsed()), + Err(_) => Err(self.survived(pids)), + }; self.child.wait().map_err(|e| format!("reap the owner: {e}"))?; - if self.closed.recv_timeout(WITHIN).is_ok() { - return Ok(killed.elapsed()); - } + verdict + } + + /// What outlived the owner, asked before it is reaped: until then no other + /// process can hold its pid, so its group is what it spawned untethered. + fn survived(&self, pids: &[u32]) -> String { // SAFETY: signal 0 asks whether the pid exists and delivers nothing. - let alive: Vec = + let answered: Vec = pids.iter().copied().filter(|&pid| unsafe { libc::kill(pid as i32, 0) } == 0).collect(); - for &pid in &alive { - // SAFETY: a process of this test's own that still holds its pid. - unsafe { libc::kill(pid as i32, libc::SIGKILL) }; - } - Err(format!( + // SAFETY: the group the unreaped owner leads. + let group = match unsafe { libc::killpg(self.child.id() as i32, libc::SIGKILL) } { + 0 => "was killed".to_string(), + _ => format!("could not be killed: {}", io::Error::last_os_error()), + }; + let after = if self.closed.recv_timeout(WITHIN).is_ok() { + "every holder of its stderr then exited".to_string() + } else { + format!("a holder of its stderr outside that group still ran {WITHIN:?} later") + }; + format!( "a process holding the owner's stderr still ran {WITHIN:?} after the owner's SIGKILL; \ - of its tethered children {pids:?}, {alive:?} did, and were killed" - )) + of its tethered children {pids:?}, {answered:?} still answered; the owner's process \ + group {group}, and {after}" + ) } } diff --git a/tests/common/orphan.rs b/tests/common/orphan.rs index 61eb6f5f0ee..ab9d97a731b 100644 --- a/tests/common/orphan.rs +++ b/tests/common/orphan.rs @@ -26,20 +26,26 @@ pub fn hold(test_config: &Path, image: &Path) { /// The harness's `SIGKILL` ends its guest, whose QEMU inherited `SIGHUP` /// blocked and ignored. pub fn guest_dies_with_its_harness(test_config: &Path) -> Result<(), String> { - // The owner's `$TMPDIR` and the image it boots: what its `SIGKILL` leaves - // there goes when this does, after the owner. let tmp = TempDir::new("orphan"); + // Where the owner's roots under `/tmp` go once it is dead: this process's + // one sweep of `/tmp` is behind it by this line, before the owner makes one. + let short = TempDir::short("orphan"); let image = tmp.join("boot.img"); std::fs::write(&image, qemu::build_boot_image(test_config, &[], &[], &[])) .map_err(|e| format!("write {}: {e}", image.display()))?; let mut owner = Command::new(std::env::current_exe().unwrap()); owner.arg(testargs::HOLD.name).arg(&image).env("TMPDIR", &tmp); let mut owner = Owner::spawn(owner)?; - // The owner builds nothing, so its one wait is its boot, whose own ceiling - // ends it well inside the backstop on any wait on a guest. - let pid: u32 = - owner.said(HELD, qemu::GUEST_WEDGED)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; - let took = owner.killed(&[pid])?; + let owner_pid = owner.pid(); + let verdict = (|| { + // The owner builds nothing, so its one wait is its boot, whose own + // ceiling ends it well inside the backstop on any wait on a guest. + let pid: u32 = + owner.said(HELD, qemu::GUEST_WEDGED)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; + owner.killed(&[pid]).map(|took| (pid, took)) + })(); + short.adopt(Path::new(toyos_tmpdir::SHORT_BASE), owner_pid); + let (pid, took) = verdict?; eprintln!(" [orphan] QEMU {pid} gone {took:?} after its harness's SIGKILL"); Ok(()) } diff --git a/toyos-tmpdir/src/lib.rs b/toyos-tmpdir/src/lib.rs index efa210f8a7e..abdaa114890 100644 --- a/toyos-tmpdir/src/lib.rs +++ b/toyos-tmpdir/src/lib.rs @@ -110,6 +110,23 @@ impl TempDir { pub fn path(&self) -> &Path { &self.path } + + /// Move every root under `base` that process `pid` made and is gone from + /// into this directory, so it goes when this does: for a caller that + /// `SIGKILL`ed `pid` after this process's one sweep of `base`. A live root + /// is left alone, whatever its name. + pub fn adopt(&self, base: &Path, pid: u32) { + let prefix = format!("{ROOT_PREFIX}{pid}-"); + let _global = global(base); + for root in gone_under(base) { + let name = root.file_name().expect("a root has a name").to_string_lossy(); + if name.starts_with(&prefix) { + let to = self.path.join(format!("reap-{name}")); + fs::rename(&root, &to) + .unwrap_or_else(|e| panic!("move {} to {}: {e}", root.display(), to.display())); + } + } + } } impl Deref for TempDir { diff --git a/toyos-tmpdir/tests/reclaim.rs b/toyos-tmpdir/tests/reclaim.rs index 45786f84f90..5d0a7dbe0d2 100644 --- a/toyos-tmpdir/tests/reclaim.rs +++ b/toyos-tmpdir/tests/reclaim.rs @@ -314,3 +314,34 @@ fn a_directory_the_sweep_cannot_remove_is_reported_and_the_next_process_still_wo perms.set_mode(0o755); std::fs::set_permissions(&stuck_locked, perms).unwrap(); } + +/// A killed process's root goes into the directory that adopts its pid, and +/// goes when that does; a live root, and a gone one of a pid that merely +/// starts with the same digits, stay. +#[test] +fn an_adopted_root_goes_with_its_adopter() { + let _spawning = spawning(); + let tmp = TempDir::new("adopt"); + let live = Holder::start(&tmp); + let killed = Holder::start(&tmp); + let (pid, root, held) = (killed.child.id(), killed.root(), killed.dir.clone()); + killed.kill(); + let other = tmp.join(format!("{ROOT_PREFIX}{pid}0-0")); + std::fs::create_dir(&other).unwrap(); + + let adopter = TempDir::new("adopter"); + adopter.adopt(&tmp, pid); + assert!(!root.exists(), "{} was not adopted", root.display()); + let moved = adopter + .join(format!("reap-{}", root.file_name().unwrap().to_string_lossy())) + .join(held.file_name().unwrap()) + .join("image.img"); + assert!(moved.exists(), "{} holds no {}", adopter.display(), moved.display()); + assert!(other.exists(), "another pid's root was adopted"); + assert!(live.dir.join("image.img").exists(), "a live root was adopted"); + + let adopted = adopter.to_path_buf(); + drop(adopter); + assert!(!adopted.exists(), "{} outlived its TempDir", adopted.display()); + live.finish(); +} From 46fd9167a05eba18857048a94c818eedd569dd8b Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 05:26:12 +0200 Subject: [PATCH 7/8] Round 3 review: prove adopt's liveness filter, stop timing a QEMU exit B6: an_adopted_root_goes_with_its_adopter added a live root whose name matches the adopted pid's prefix, so the test only passes because adopt's liveness filter actually keeps it out; deleting that filter now turns it red. adopt and State::sweep share the rename-into-reap- step through one reap_into helper, since writing both filters next to each other is what made the gap visible. NOTE: following main's #562, no QEMU test measures how long something took. Owner::killed no longer times the exit; it returns Result<(), String> and the verdict rests on the event alone. Deleted the two REMOVE'd lines without rewriting them. Co-Authored-By: Claude Opus 5.5 --- src/tether.rs | 17 +++++++------- tests/common/orphan.rs | 8 +++---- toyos-tmpdir/src/lib.rs | 42 ++++++++++++++++++----------------- toyos-tmpdir/tests/reclaim.rs | 14 ++++++++++++ 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/src/tether.rs b/src/tether.rs index 29eef55d35f..6f3cdb57754 100644 --- a/src/tether.rs +++ b/src/tether.rs @@ -185,18 +185,17 @@ impl Owner { self.child.id() } - /// `SIGKILL` the owner, and how long every process holding its stderr took - /// to exit after it. `Err` if one outlived it by [`WITHIN`], naming which - /// of `pids` still answer; the owner's group is then killed, and the - /// refusal says whether every holder went with it. - pub fn killed(mut self, pids: &[u32]) -> Result { - let killed = Instant::now(); + /// `SIGKILL` the owner. `Err` if a process holding its stderr is still + /// there [`WITHIN`] afterward, naming which of `pids` still answer; the + /// owner's group is then killed, and the refusal says whether every + /// holder went with it. + pub fn killed(mut self, pids: &[u32]) -> Result<(), String> { self.child.kill().map_err(|e| format!("SIGKILL the owner: {e}"))?; // Held past the verdict: `wait` would close it, and a child reading // it would end on that instead of on its tether. let _stdin = self.child.stdin.take(); let verdict = match self.closed.recv_timeout(WITHIN) { - Ok(_) => Ok(killed.elapsed()), + Ok(_) => Ok(()), Err(_) => Err(self.survived(pids)), }; self.child.wait().map_err(|e| format!("reap the owner: {e}"))?; @@ -273,7 +272,7 @@ mod tests { let mut owner = Owner::spawn(this_test("tether::tests::owner")).unwrap_or_else(|e| panic!("{e}")); let pid: u32 = owner.said(TETHERED, WITHIN).unwrap_or_else(|e| panic!("{e}")).parse().expect("a pid"); - let took = owner.killed(&[pid]).unwrap_or_else(|e| panic!("{e}")); - eprintln!("tethered child {pid} gone {took:?} after its owner's SIGKILL"); + owner.killed(&[pid]).unwrap_or_else(|e| panic!("{e}")); + eprintln!("tethered child {pid} gone after its owner's SIGKILL"); } } diff --git a/tests/common/orphan.rs b/tests/common/orphan.rs index ab9d97a731b..643ba749a8c 100644 --- a/tests/common/orphan.rs +++ b/tests/common/orphan.rs @@ -27,8 +27,6 @@ pub fn hold(test_config: &Path, image: &Path) { /// blocked and ignored. pub fn guest_dies_with_its_harness(test_config: &Path) -> Result<(), String> { let tmp = TempDir::new("orphan"); - // Where the owner's roots under `/tmp` go once it is dead: this process's - // one sweep of `/tmp` is behind it by this line, before the owner makes one. let short = TempDir::short("orphan"); let image = tmp.join("boot.img"); std::fs::write(&image, qemu::build_boot_image(test_config, &[], &[], &[])) @@ -42,10 +40,10 @@ pub fn guest_dies_with_its_harness(test_config: &Path) -> Result<(), String> { // ceiling ends it well inside the backstop on any wait on a guest. let pid: u32 = owner.said(HELD, qemu::GUEST_WEDGED)?.parse().map_err(|e| format!("the owner's QEMU pid: {e}"))?; - owner.killed(&[pid]).map(|took| (pid, took)) + owner.killed(&[pid]).map(|()| pid) })(); short.adopt(Path::new(toyos_tmpdir::SHORT_BASE), owner_pid); - let (pid, took) = verdict?; - eprintln!(" [orphan] QEMU {pid} gone {took:?} after its harness's SIGKILL"); + let pid = verdict?; + eprintln!(" [orphan] QEMU {pid} gone after its harness's SIGKILL"); Ok(()) } diff --git a/toyos-tmpdir/src/lib.rs b/toyos-tmpdir/src/lib.rs index abdaa114890..8756b7a93f6 100644 --- a/toyos-tmpdir/src/lib.rs +++ b/toyos-tmpdir/src/lib.rs @@ -118,14 +118,9 @@ impl TempDir { pub fn adopt(&self, base: &Path, pid: u32) { let prefix = format!("{ROOT_PREFIX}{pid}-"); let _global = global(base); - for root in gone_under(base) { - let name = root.file_name().expect("a root has a name").to_string_lossy(); - if name.starts_with(&prefix) { - let to = self.path.join(format!("reap-{name}")); - fs::rename(&root, &to) - .unwrap_or_else(|e| panic!("move {} to {}: {e}", root.display(), to.display())); - } - } + reap_into(base, &self.path, |root| { + root.file_name().expect("a root has a name").to_string_lossy().starts_with(&prefix) + }); } } @@ -301,18 +296,7 @@ impl State { self.swept = true; let root = self.root.as_ref().expect("a sweep runs from a live root"); let _global = global(&root.tmp); - let mut reap = Vec::new(); - for path in gone_under(&root.tmp) { - if path == root.dir { - continue; - } - let name = path.file_name().expect("a root has a name").to_string_lossy(); - let to = root.dir.join(format!("reap-{name}")); - fs::rename(&path, &to) - .unwrap_or_else(|e| panic!("move {} to {}: {e}", path.display(), to.display())); - reap.push(to); - } - reap + reap_into(&root.tmp, &root.dir, |path| path != root.dir) } } @@ -323,6 +307,24 @@ pub fn gone_roots(base: &Path) -> Vec { gone_under(base) } +/// Every gone root under `base` that `keep` passes, renamed into `dest` as +/// `reap-`; the caller holds [`GLOBAL`]. Both `adopt` and `sweep` +/// are this with a different filter and destination. +fn reap_into(base: &Path, dest: &Path, keep: impl Fn(&Path) -> bool) -> Vec { + let mut reap = Vec::new(); + for path in gone_under(base) { + if !keep(&path) { + continue; + } + let name = path.file_name().expect("a root has a name").to_string_lossy(); + let to = dest.join(format!("reap-{name}")); + fs::rename(&path, &to) + .unwrap_or_else(|e| panic!("move {} to {}: {e}", path.display(), to.display())); + reap.push(to); + } + reap +} + /// [`gone_roots`], with [`GLOBAL`] held by the caller. fn gone_under(base: &Path) -> Vec { let entries = fs::read_dir(base).unwrap_or_else(|e| panic!("read {}: {e}", base.display())); diff --git a/toyos-tmpdir/tests/reclaim.rs b/toyos-tmpdir/tests/reclaim.rs index 5d0a7dbe0d2..b689a3caffa 100644 --- a/toyos-tmpdir/tests/reclaim.rs +++ b/toyos-tmpdir/tests/reclaim.rs @@ -329,6 +329,18 @@ fn an_adopted_root_goes_with_its_adopter() { let other = tmp.join(format!("{ROOT_PREFIX}{pid}0-0")); std::fs::create_dir(&other).unwrap(); + // The adopted pid reused, so its name matches the prefix `adopt` filters + // on; only its owner's lock says it is live, and that must still hold. + let reused = tmp.join(format!("{ROOT_PREFIX}{pid}-9")); + std::fs::create_dir(&reused).unwrap(); + let reused_owner = OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(reused.join(OWNER)) + .unwrap(); + reused_owner.lock().unwrap(); + let adopter = TempDir::new("adopter"); adopter.adopt(&tmp, pid); assert!(!root.exists(), "{} was not adopted", root.display()); @@ -339,7 +351,9 @@ fn an_adopted_root_goes_with_its_adopter() { assert!(moved.exists(), "{} holds no {}", adopter.display(), moved.display()); assert!(other.exists(), "another pid's root was adopted"); assert!(live.dir.join("image.img").exists(), "a live root was adopted"); + assert!(reused.exists(), "a live root of a reused pid was adopted"); + drop(reused_owner); let adopted = adopter.to_path_buf(); drop(adopter); assert!(!adopted.exists(), "{} outlived its TempDir", adopted.display()); From 30ea04cec818f76c5aaf338dec48403443bbc2c7 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 08:34:35 +0200 Subject: [PATCH 8/8] PR #555 round 5: name reap_into's predicate for what it takes, drop its caller list. Co-Authored-By: Claude Opus 5.5 --- toyos-tmpdir/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/toyos-tmpdir/src/lib.rs b/toyos-tmpdir/src/lib.rs index 8756b7a93f6..456e1000eb6 100644 --- a/toyos-tmpdir/src/lib.rs +++ b/toyos-tmpdir/src/lib.rs @@ -307,13 +307,12 @@ pub fn gone_roots(base: &Path) -> Vec { gone_under(base) } -/// Every gone root under `base` that `keep` passes, renamed into `dest` as -/// `reap-`; the caller holds [`GLOBAL`]. Both `adopt` and `sweep` -/// are this with a different filter and destination. -fn reap_into(base: &Path, dest: &Path, keep: impl Fn(&Path) -> bool) -> Vec { +/// Every gone root under `base` that `take` passes, renamed into `dest` as +/// `reap-`; the caller holds [`GLOBAL`]. +fn reap_into(base: &Path, dest: &Path, take: impl Fn(&Path) -> bool) -> Vec { let mut reap = Vec::new(); for path in gone_under(base) { - if !keep(&path) { + if !take(&path) { continue; } let name = path.file_name().expect("a root has a name").to_string_lossy();