diff --git a/Cargo.toml b/Cargo.toml index ccc0d35460..e94a0a7407 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,7 +144,7 @@ toyos-swap = { path = "toyos-swap" } # The watchdog's parameter name, so the gate over `kernel/src/params.rs` reads # the same constant the kernel and the bootloader do. toyos-tco = { path = "toyos-tco" } -# The one scratch directory under `$TMPDIR`: the harness's run takes one, and so does +# The one scratch directory, under `$TMPDIR` or, for a socket, `/tmp`: the harness's run takes one, and so does # every test here; `--ci host` holds the host tests to leaving nothing behind. toyos-tmpdir = { path = "toyos-tmpdir" } toyos-blackbox = { path = "toyos-blackbox" } diff --git a/issues/boot-media/partition-claim-departure-exits-clean-with-none-of-its-refusals-said.md b/issues/boot-media/partition-claim-departure-exits-clean-with-none-of-its-refusals-said.md new file mode 100644 index 0000000000..d529626606 --- /dev/null +++ b/issues/boot-media/partition-claim-departure-exits-clean-with-none-of-its-refusals-said.md @@ -0,0 +1,72 @@ +--- +status: expected-red +kind: tooling +opened: 2026-09-27 +--- + +# `partition_claim_departure` exits 0 having said none of its refusals + +Seen on 2026-09-27 in the nightly run 36336701867, job `guest (5)`, on PR +#541's head `1c0f0c753fb2c4c2d01caf51dc67b7f92ca4cd8e` — a diff that touches +none of `tests/common/partclaim.rs`, `tests/toyos-rust-tests/src/bin/partition_claimant.rs` +or the kernel's partition-claim code: + +``` +FAIL partition_claim_departure: departure: the guest exited 0 having said 0 of its 1 refusals: + + FAIL partition_claim_departure (2s) +``` + +Re-run alone, immediately after, in the same session: green, with every role's +own line printed — + +``` + [partclaim] departure: 1 told; [kernel 0.757 cpu0] usb-quiesce: disk 0 SYNCHRONIZE CACHE ok + [partclaim] silent: 1 told; [kernel 0.710 cpu0] usb-quiesce: disk 0 SYNCHRONIZE CACHE ok + [partclaim] untold: 0 told; ... + PASS partition_claim_departure (7s) + ALONE partition_claim_departure: GREEN, and it was alone both times — nothing + the harness controls differed, so it failed once and passed once. That is a + rate and not a classification. +``` + +`cargo run -- --known-red partition_claim_departure` answered NO before this +row. + +## What is known + +The failure is `guest_verdict`'s (`tests/common/partclaim.rs`), on the +`departure` role — the first of the three `departed()` iterations in +`partition_claim_departure`. Its message, `"the guest exited 0 having said +{said} of its {refusals} refusals:\n{stdout}"`, prints with an empty tail: the +guest's own captured `stdout` held nothing at all — not one of the `departure` +role's own `println!`s (`"a write is reported and not flushed"`, `"the write +the device left under completed"`, the `refused with` lines `said()` prints, +or `"partition_claimant: PASS"`), yet the guest's exit code was 0. + +An exit of 0 rules out a panic on a wrong assertion (`departure()`'s +`assert_eq!`s all `panic!` on mismatch, and a panic does not exit 0), so the +guest's own logic is not shown to have run into an unexpected state. + +This is the same family flagged in +`issues/boot-media/partition-claim-gives-up-reds-beside-other-guests-and-is-green-alone.md` +— same test area, and that file already widens its scope to +`partition_claim_departure`. It is filed separately rather than folded in because the failure signature +differs: that file's evidence is a kernel-log line count coming up short +(`"{count} flushes were told of the loss, not {told}"`, matched against +lines the kernel actually printed) on the `silent` role, attributed to a +hypothesised unscoped global fsync deadman race; this is a `guest_verdict` +failure on the `departure` role with the guest's entire stdout capture +missing, which that hypothesis does not by itself explain. + +## Exit condition + +`guest_verdict`'s "exited 0 having said" refusal (`tests/common/partclaim.rs`) +carries the kernel window, as its non-zero-exit refusal already does, so the +next sighting is not blind; the mechanism named and fixed; and a test that +turns red on it deterministically. Then this row and its `src/redlist.rs` +entry are deleted. + +## Owner + +The partition-claim code, held by the orchestrator. diff --git a/issues/build/a-lane-s-tap-socket-path-outgrows-sun-len-on-the-dev-host.md b/issues/build/a-lane-s-tap-socket-path-outgrows-sun-len-on-the-dev-host.md deleted file mode 100644 index 7e81a9cf3d..0000000000 --- a/issues/build/a-lane-s-tap-socket-path-outgrows-sun-len-on-the-dev-host.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-09-26 ---- - -# A lane's tap socket path outgrows `SUN_LEN` on the dev host - -`lan_mdns_answer` reds on the macOS dev host, wide and alone: - -``` -connect to QEMU's /private/var/folders/gr/mr4_fg4n34jb417sx1g5cgxc0000gp/T/toyos-tmp-89085-0/tests-0/lane-3/tap-out-0.sock: path must be shorter than SUN_LEN -``` - -That path is 104 bytes, past the 103 a `sockaddr_un` holds before its -terminating NUL on macOS. `tests/common/segment.rs`'s `Tap::in_lane` puts both -sockets in `lane::dir()`, which since `toyos-tmpdir` is -`$TMPDIR/toyos-tmp--/tests-/lane-/`, and the dev host's -`$TMPDIR` resolves to 57 bytes (`/private/var/folders/…/T/`) before any of -that. A five-digit pid is enough to cross the limit. - -Seen three times on 2026-09-26, each on a tree whose diff touches neither the -lane nor the tap: - -- on `wt/toyos-layout` after it merged `origin/main` at `e48604c0` (pid 89085, - `lane-3`, the capture above); -- in the fast tier at `origin/main` checked out in the `toyos-guiplat` - worktree: alone with this message, wide as `QEMU died before ===READY===`; -- in the fast tier at PR #528's head after it merged `e48604c0` (pid 70685, - `lane-7`), this message wide and alone. - -`cargo run -- --known-red lan_mdns_answer` answers NO. - -## Exit condition - -A tap socket's path fits `sun_path` on every host the suite runs on, wherever -the scratch directory is, and `lan_mdns_answer` is green on the dev host. diff --git a/issues/build/the-tmp-leftover-check-can-name-another-worktrees-killed-run.md b/issues/build/the-tmp-leftover-check-can-name-another-worktrees-killed-run.md new file mode 100644 index 0000000000..5b69c9f47e --- /dev/null +++ b/issues/build/the-tmp-leftover-check-can-name-another-worktrees-killed-run.md @@ -0,0 +1,35 @@ +--- +status: open +kind: tooling +opened: 2026-09-28 +--- + +# The `/tmp` leftover check can name another worktree's killed run + +`src/ci.rs`'s `left_behind` names every root `toyos_tmpdir::gone_roots(short)` +finds that `before` does not: a `toyos_tmpdir::TempDir::short` whose owning +process died during the steps that ran between the two calls. `gone_roots` +reads `SHORT_BASE` (`/tmp`), and every worktree on the host shares that base +and its `GLOBAL` lock — the reclaim is deliberately cross-worktree +(`toyos-tmpdir/src/lib.rs`'s module header: "Every process that shares a +base — every worktree on the host — shares the lock file"). Nothing in +`gone_roots` or in `left_behind`'s call sites records which pids belong to +*this* job's own steps, so a root left by another worktree's harness, +SIGKILLed on the same host in the same window, is named as this job's own +leftover. + +**Evidence:** `toyos-tmpdir`'s own tests demonstrate the mechanism the check +relies on — a SIGKILLed holder's root is reclaimed by the next process to make +a directory in the same base, whichever process that is +(`toyos-tmpdir/tests/reclaim.rs`'s `a_killed_process_is_reclaimed_and_a_live_one_is_never_touched`). +`left_behind` has no notion of a job or a worktree; it is a diff of +`gone_roots(short)` against a snapshot taken before the steps ran, over a base +every process on the host writes into. On the hosted runners the gate uses, +each job has its own host, so the check is exact there; a developer running +`cargo run -- --ci host` on a machine also running another worktree's harness +can see a red that is not this job's. + +**Exit condition:** `left_behind` names only roots this job's own processes +created, or the check is scoped to a base this job does not share with another +worktree's harness; a test demonstrates the narrowed check passing a killed +run made outside the job's own process tree. diff --git a/issues/hardware/i8042-mouse-ends-four-packets-short-with-a-clean-exit.md b/issues/hardware/i8042-mouse-ends-four-packets-short-with-a-clean-exit.md new file mode 100644 index 0000000000..edc2bb5832 --- /dev/null +++ b/issues/hardware/i8042-mouse-ends-four-packets-short-with-a-clean-exit.md @@ -0,0 +1,65 @@ +--- +status: expected-red +kind: tooling +opened: 2026-09-27 +--- + +# `i8042_mouse` ends four packets short with a clean exit; mechanism not known + +Seen on 2026-09-27 in the orchestrator's Fast-tier run on PR #537's head +`06b926b1`, a diff that touches neither the i8042 driver nor this test: + +``` +FAIL i8042_mouse: 872 pointer events reached userland out of 876 packets injected, never more than 4 of them (12 bytes) outstanding against a 16-byte device queue + FAIL i8042_mouse (17s) +``` + +`cargo run -- --known-red i8042_mouse` answered NO. Earlier sightings of this +message are in `issues/build/parallel-tests-red-under-other-suites.md`'s +`i8042_mouse` entry. + +## What is known + +- **The run ended cleanly.** This message is reached only when + `run_test_paced` returned no error, so the test runner printed + `===TEST_END test_rs_i8042_mouse` with a tail other than `error=`: `exit=`, + none, or one it cannot parse. A stall, a ceiling or a runner error ends in + the `STALLED` message instead. +- **The guest stopped reading mid-burst.** 876 injected is the four lead-in + packets plus 872 of `BURST`'s 1000. The shortfall, 4, equals `MOUSE_LEAD`: + the host always refills to `arrived + MOUSE_LEAD`, so any guest that stops + reading mid-burst leaves exactly that many unread. +- **Not the guest's `RUN_CEILING`.** The test took 17 s, and the ceiling is + 60 s from `===I8042_MOUSE_READY===`. +- **The capture that would tell is not kept.** The message carries neither + the guest's stdout, the kernel's serial window, nor the exit code, and + `i8042_mouse` never reads `exit_code` before this count. So the log's missing + `mev done` line says nothing. The boot is `Profile::Metal`, whose 16550 is + the console on stdio, so it writes no `uart-*.log`. No `Boot parameter:` line + in the run's kept serial logs carries `i8042-trace`. + +Two paths in the tree end the guest this way, and nothing captured tells +them apart: + +- **The guest's own end rule, met by a misframed packet.** + `tests/toyos-rust-tests/src/bin/i8042_mouse.rs` exits 0 on the first event + without the right button after one with it. `toyos_ps2::mouse`'s decoder + resets to a head on any gap between bytes longer than `PACKET_GAP_NS` (5 ms). + Measured on the host by feeding the burst's bytes to `MouseDecoder`: one such + gap between a −1 packet's head `0x18` and its `dx` `0xFF` takes `0xFF` as a + head. The decoder emits `buttons=0x07`, discards the next packet's `0x01 0x00`, + and then emits the following −1 with `buttons=0x00`. That is a press and + release of the right button. Whether a gap that long in guest time lands + inside a packet on this host is not measured. +- **A non-zero exit**, which this test does not read. + +## Exit condition + +The shortfall refusal (`i8042_mouse` in `tests/toyos.rs`) carries +`result.stdout` and `result.exit_code`, so the next sighting is not blind; the +mechanism is named, and a deterministic test is red on it. Then this file and +its `src/redlist.rs` row are deleted. + +## Owner + +The i8042/input path, held by the orchestrator. diff --git a/src/ci.rs b/src/ci.rs index eb7700dce9..19a6b3c951 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -26,7 +26,7 @@ //! open; `cargo run` only notes one, because a build must not stop for brew. use std::io::{BufRead, BufReader, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use crate::arch::Arch; @@ -427,6 +427,8 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); + let short = Path::new(toyos_tmpdir::SHORT_BASE); + let before = toyos_tmpdir::gone_roots(short); // Before any thread: nothing in this process reads the environment // concurrently with the write, and every child inherits it. std::env::set_var("TMPDIR", tmp.path()); @@ -491,18 +493,19 @@ fn host(root: &Path) -> Vec { steps.push(step("the toyos SDK", || { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); - steps.push(step("nothing left in $TMPDIR", || left_behind(&tmp))); + steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); steps } -/// What `tmp` holds but the lock `toyos_tmpdir` keeps in it, as a refusal. +/// What `tmp` holds but the lock `toyos_tmpdir` keeps in it, and every root +/// under `short` whose process is gone that `before` does not name. /// /// Refuses if `tmp` holds no [`toyos_tmpdir::GLOBAL`] at all: every step above /// makes at least one `toyos_tmpdir::TempDir`, which always writes that lock /// file first, so its absence means this `$TMPDIR` never saw the steps at /// all — the guard reading an empty directory it was never given, rather than /// one every test actually cleaned. -fn left_behind(tmp: &Path) -> Result { +fn left_behind(tmp: &Path, short: &Path, before: &[PathBuf]) -> Result { let mut left: Vec = std::fs::read_dir(tmp) .map_err(|e| format!("read {}: {e}", tmp.display()))? .map(|e| e.map(|e| e.file_name().to_string_lossy().into_owned())) @@ -518,15 +521,28 @@ fn left_behind(tmp: &Path) -> Result { } left.retain(|name| name != toyos_tmpdir::GLOBAL); left.sort(); - if left.is_empty() { + let mut dead: Vec = toyos_tmpdir::gone_roots(short) + .into_iter() + .filter(|root| !before.contains(root)) + .map(|root| root.display().to_string()) + .collect(); + dead.sort(); + let mut said = Vec::new(); + if !left.is_empty() { + said.push(format!( + "left in {} by the steps above, each written past a `toyos_tmpdir::TempDir` or held \ + past its test: {}", + tmp.display(), + left.join(", ") + )); + } + if !dead.is_empty() { + said.push(format!("left by a process that died during the steps above: {}", dead.join(", "))); + } + if said.is_empty() { return Ok("every test took its scratch with it".into()); } - Err(format!( - "left in {} by the steps above, each written past a `toyos_tmpdir::TempDir` or held past \ - its test: {}", - tmp.display(), - left.join(", ") - )) + Err(said.join("; ")) } /// What protects `main` is configured outside the repository, so it is read @@ -610,6 +626,8 @@ fn suite_args(args: &[&str]) -> Vec { /// every boot image — survives past the last step, which reds on it. fn guest(root: &Path, suite: &[String]) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-guest"); + let short = Path::new(toyos_tmpdir::SHORT_BASE); + let before = toyos_tmpdir::gone_roots(short); // Before any thread, same as `host`: every child this process spawns below // inherits this, and nothing here reads the environment concurrently with // the write. @@ -634,7 +652,7 @@ fn guest(root: &Path, suite: &[String]) -> Vec { } // Unconditional: whatever stopped earlier, this $TMPDIR is still this // process's own to judge, and a leak past a failing suite is still a leak. - steps.push(step("nothing left in $TMPDIR", || left_behind(&tmp))); + steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); steps } @@ -861,7 +879,6 @@ fn at_tip(ls_remote: &str, head: &str) -> Result<(), String> { #[cfg(test)] mod tests { use super::*; - use std::path::PathBuf; /// A deterministic control on `host`'s `std::env::set_var("TMPDIR", ...)`: /// delete that line and every child writes to the real `$TMPDIR` instead of @@ -870,7 +887,9 @@ mod tests { #[test] fn left_behind_refuses_a_tmpdir_that_never_saw_the_lock() { let tmp = toyos_tmpdir::TempDir::new("left-behind-blind"); - let refusal = left_behind(&tmp).expect_err("an untouched $TMPDIR is a red, not a pass"); + let short = toyos_tmpdir::TempDir::new("left-behind-blind-short"); + let refusal = + left_behind(&tmp, &short, &[]).expect_err("an untouched $TMPDIR is a red, not a pass"); assert!(refusal.contains(toyos_tmpdir::GLOBAL), "{refusal}"); } @@ -878,13 +897,31 @@ mod tests { #[test] fn the_host_job_names_what_its_tests_left_behind() { let tmp = toyos_tmpdir::TempDir::new("left-behind"); + let short = toyos_tmpdir::TempDir::new("left-behind-short"); std::fs::write(tmp.join(toyos_tmpdir::GLOBAL), b"").unwrap(); - assert!(left_behind(&tmp).is_ok()); + assert!(left_behind(&tmp, &short, &[]).is_ok()); std::fs::create_dir(tmp.join("forkcheck-1-current")).unwrap(); - let refusal = left_behind(&tmp).expect_err("a directory left behind is a red"); + let refusal = left_behind(&tmp, &short, &[]).expect_err("a directory left behind is a red"); assert!(refusal.contains("forkcheck-1-current"), "{refusal}"); } + /// A short root whose process died while the steps ran is named; one already + /// dead before them is not the steps'. + #[test] + fn the_job_names_a_short_root_a_step_left_when_it_died() { + let tmp = toyos_tmpdir::TempDir::new("left-behind-died"); + let short = toyos_tmpdir::TempDir::new("left-behind-died-short"); + std::fs::write(tmp.join(toyos_tmpdir::GLOBAL), b"").unwrap(); + let earlier = format!("{}1-0", toyos_tmpdir::ROOT_PREFIX); + std::fs::create_dir(short.join(&earlier)).unwrap(); + let before = toyos_tmpdir::gone_roots(&short); + assert!(left_behind(&tmp, &short, &before).is_ok()); + let died = format!("{}2-0", toyos_tmpdir::ROOT_PREFIX); + std::fs::create_dir(short.join(&died)).unwrap(); + let refusal = left_behind(&tmp, &short, &before).expect_err("a dead step's root is a red"); + assert!(refusal.contains(&died) && !refusal.contains(&earlier), "{refusal}"); + } + fn repo_root() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) } diff --git a/src/qemu.rs b/src/qemu.rs index b40eb51d86..a9b799e743 100644 --- a/src/qemu.rs +++ b/src/qemu.rs @@ -25,10 +25,9 @@ //! //! # QMP, and the machine that has already stopped //! -//! The socket is `/tmp/toyos-qmp.sock`. A harness test booted with -//! `BootOptions { qmp: true }` leaves one under -//! the run's lane directory, `$TMPDIR/toyos-tmp--*/tests-*/lane-/` -//! while the run lives (`tests/common/lane.rs`), which is how a frozen guest is read +//! The socket is `/tmp/toyos-qmp.sock`, and a harness boot's is +//! `/tmp/toyos-tmp--*/boot-/qmp.sock` (`toyos_tmpdir::TempDir::short`) +//! while its guest lives, which is how a frozen guest is read //! without a `cargo run` at all: `human-monitor-command` with `info registers //! -a` gives every vCPU's `RIP`, `RFL` and `HLT`, and that is what tells a //! halted-awaiting-interrupt machine from a wedged one. diff --git a/src/redlist.rs b/src/redlist.rs index f941c3eacc..9adcffd4c9 100644 --- a/src/redlist.rs +++ b/src/redlist.rs @@ -41,8 +41,16 @@ pub const DISABLED: &[Disabled] = &[ }, Disabled { test: "handle_transfer", issue: "issues/kernel/deferred-release-outlives-its-syscall.md" }, Disabled { test: "hda_tone", issue: "issues/audio/hda-tone-phase-check.md" }, + Disabled { + test: "i8042_mouse", + issue: "issues/hardware/i8042-mouse-ends-four-packets-short-with-a-clean-exit.md", + }, Disabled { test: "kill_while_blocked", issue: "issues/kernel/deferred-release-outlives-its-syscall.md" }, Disabled { test: "latency_wake", issue: "issues/build/latency-wake-reds-on-the-dev-host-at-a-rate.md" }, + Disabled { + test: "partition_claim_departure", + issue: "issues/boot-media/partition-claim-departure-exits-clean-with-none-of-its-refusals-said.md", + }, Disabled { test: "quiesce_dump_holds_the_stopped", issue: "issues/kernel/quiesce-dump-holds-the-stopped-reds-wide-with-usb-transport-breaks.md", diff --git a/tests/common/lane.rs b/tests/common/lane.rs index 9046d26985..fbe4a98240 100644 --- a/tests/common/lane.rs +++ b/tests/common/lane.rs @@ -62,7 +62,7 @@ pub fn dir() -> PathBuf { static RUN: OnceLock = OnceLock::new(); /// This run's hold on its scratch directory, from before the first boot to the -/// exit: every image, boot log, screendump and socket of every lane is under +/// exit: every image, boot log and screendump of every lane is under /// it, and it is gone when the run is, green or red (`toyos_tmpdir` is the /// policy, and what reclaims the directory of a run that was killed). /// diff --git a/tests/common/origin.rs b/tests/common/origin.rs index 2a3cd88b13..3b3df827d5 100644 --- a/tests/common/origin.rs +++ b/tests/common/origin.rs @@ -647,13 +647,12 @@ pub fn carrier_forgery(c_bins: &[(String, Vec)], rust_bins: &[(String, Vec)], rust_bins: &[(String, Vec)]) -> Result<(), String> { - let tap = segment::Tap::in_lane(); - let options = BootOptions { profile: VIRTIO.profile, segment: Some(tap.clone()), ..Default::default() }; + let options = BootOptions { profile: VIRTIO.profile, segment: true, ..Default::default() }; let config = compile::repo_root().join(VIRTIO.config); let mut guest = QemuInstance::boot_with_options(&config, c_bins, rust_bins, options); let mut console = guest.boot_log().to_string(); qemu::await_marker(&mut guest, &mut console, "netd: DHCP: lease ", "netd's lease")?; - let mut wire = tap.open()?; + let mut wire = guest.segment()?; let deadline = || Instant::now() + Duration::from_secs(10); wire.send(&segment::arp_request(NEIGHBOUR_MAC, NEIGHBOUR, GUEST))?; diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index c9aa24b63a..09e7bf0eeb 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_tmpdir::TempDir; /// 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 @@ -2422,8 +2423,9 @@ pub struct BootOptions { pub console_file: bool, /// Put the host on the guest's own segment (`super::segment`): frames /// it writes reach the NIC as if off the cable, and it sees every frame the - /// guest sends. Refused by name on a profile with no NIC. - pub segment: Option, + /// guest sends, through [`QemuInstance::segment`]. Refused by name on a + /// profile with no NIC. + pub segment: bool, /// Forward this host port to the guest's TCP 22. **slirp is one-way /// without it**: nothing on the host can open a connection into the guest /// unless QEMU is told which port to translate. A profile with no NIC @@ -2524,7 +2526,7 @@ impl Default for BootOptions { extra_root_files: Vec::new(), log_port: None, console_file: false, - segment: None, + segment: false, ssh_port: None, wire_dump: None, userland_nvme: None, @@ -2634,7 +2636,7 @@ pub struct QemuInstance { uart_log: PathBuf, nvme: NvmeClaim, usb_images: Vec, - qmp_socket: Option, + sockets: Sockets, screendump: PathBuf, /// The image this boot built for itself, which is the only one it may /// delete: a [`BootOptions::boot_image`] belongs to the test that staged it @@ -3143,10 +3145,7 @@ impl QemuInstance { let audio_wav = test_dir.join(format!("audio-{seq}.wav")); let _ = fs::remove_file(&audio_wav); - let qmp_socket = options.qmp.then(|| test_dir.join(format!("qmp-{seq}.sock"))); - if let Some(path) = &qmp_socket { - let _ = fs::remove_file(path); - } + let sockets = Sockets::new(&options); let screendump = test_dir.join(format!("screen-{seq}.ppm")); // Per-instance, not a fixed /tmp path: the audio gate boots dozens of @@ -3162,7 +3161,7 @@ impl QemuInstance { &usb_images, &audio_wav, &uart_log, - qmp_socket.as_deref(), + &sockets.dir, &options, ); spawn_and_wait_ready( @@ -3174,7 +3173,7 @@ impl QemuInstance { uart_log, nvme, usb_images, - qmp_socket, + sockets, screendump, own_boot_image, carried, @@ -3191,7 +3190,7 @@ impl QemuInstance { /// command that answers, so what a test judges is memory QEMU dumped and not /// a report the guest wrote about itself. pub fn guest_memory(&mut self, phys: u64, bytes: usize) -> Result, String> { - let socket = self.qmp_socket.clone().expect("guest_memory needs BootOptions { qmp: true }"); + let socket = self.sockets.qmp.clone().expect("guest_memory needs BootOptions { qmp: true }"); // Beside the screendump, which is this instance's own scratch path. let out = self.screendump.with_extension(format!("mem-{phys:#x}")); let _ = fs::remove_file(&out); @@ -3218,10 +3217,7 @@ impl QemuInstance { /// the file itself, so the only synchronization needed is the command's /// own reply. pub fn screendump(&mut self) -> super::screen::Ppm { - let socket = self - .qmp_socket - .clone() - .expect("screendump needs BootOptions { qmp: true }"); + let socket = self.sockets.qmp.clone().expect("screendump needs BootOptions { qmp: true }"); let out = self.screendump.clone(); let _ = fs::remove_file(&out); @@ -3536,7 +3532,12 @@ impl QemuInstance { /// The QMP socket this instance opened. Injection needs it, and it needs /// `BootOptions { qmp: true }`. pub fn qmp_socket(&self) -> &Path { - self.qmp_socket.as_ref().expect("qmp_socket needs BootOptions { qmp: true }") + self.sockets.qmp.as_deref().expect("qmp_socket needs BootOptions { qmp: true }") + } + + /// Stand on this guest's segment; it needs `BootOptions { segment: true }`. + pub fn segment(&self) -> Result { + self.sockets.segment.as_ref().expect("segment needs BootOptions { segment: true }").open() } /// [`budget`] for a host-side wait on *this* guest, widened by the guest's @@ -3596,9 +3597,6 @@ impl QemuInstance { writeln!(self.stdin, "run {name}").expect("Failed to write to QEMU stdin"); self.stdin.flush().expect("Failed to flush QEMU stdin"); - let mut fire = - |line: &str, socket: Option<&PathBuf>| step(socket.map(PathBuf::as_path), line); - // `run [args...]`, and the markers carry only the binary name. let want = name.split_whitespace().next().unwrap_or(name); if let Some(carried) = &self.carried { @@ -3667,7 +3665,7 @@ impl QemuInstance { Ok(line) => { last_line = Instant::now(); lines += 1; - fire(&line, self.qmp_socket.as_ref()); + step(self.sockets.qmp.as_deref(), &line); if dying.is_none() && super::serial::died(&line) == Some(super::serial::Died::Kernel) { @@ -3805,14 +3803,12 @@ impl Drop for QemuInstance { // reads as "there was nothing to keep" rather than "it was deleted // before the step ran". let _ = fs::remove_file(&self.screendump); - if let Some(socket) = &self.qmp_socket { - let _ = fs::remove_file(socket); - } // A per-boot image is hundreds of megabytes and a full run makes ~76 of // them; the shared name used to make that one file. if let Some(image) = &self.own_boot_image { let _ = fs::remove_file(image); } + // `sockets` goes with the fields, after QEMU is reaped. LIVE.fetch_sub(1, Ordering::SeqCst); } } @@ -4318,7 +4314,7 @@ impl QmpDevices { pub fn profile_argv(options: &BootOptions) -> Vec { let p = Path::new("/nonexistent"); let usb: Vec = options.profile.usb_disks().iter().map(|_| p.to_path_buf()).collect(); - qemu_command(p, p, &usb, p, p, None, options) + qemu_command(p, p, &usb, p, p, p, options) .get_args() .map(|a| a.to_string_lossy().into_owned()) .collect() @@ -4344,9 +4340,10 @@ fn qemu_command( usb_images: &[PathBuf], audio_wav: &Path, uart_log: &Path, - qmp_socket: Option<&Path>, + socket_dir: &Path, options: &BootOptions, ) -> Command { + let (qmp_socket, segment) = socket_names(socket_dir, options); let shape = options.profile.shape(); let console_file = options.console_file.then(|| ConsoleFile::of(uart_log)); assert!( @@ -4724,7 +4721,7 @@ fn qemu_command( qemu.arg("-object") .arg(format!("filter-dump,id=wire,netdev=net0,file={}", at.display())); } - if let Some(tap) = &options.segment { + if let Some(tap) = &segment { assert!( !matches!(shape.nic, Nic::Absent), "this profile carries no NIC, so there is no `net0` segment to stand on" @@ -4786,6 +4783,32 @@ fn qemu_command( qemu } +/// A boot's Unix sockets, in a directory of its own under `/tmp` rather than +/// the lane's: `sun_path` is 104 bytes on Darwin, and `$TMPDIR`'s depth is the +/// host's. The directory goes with this, after QEMU is reaped. +struct Sockets { + dir: TempDir, + qmp: Option, + segment: Option, +} + +impl Sockets { + fn new(options: &BootOptions) -> Sockets { + let dir = TempDir::short("boot"); + let (qmp, segment) = socket_names(&dir, options); + Sockets { dir, qmp, segment } + } +} + +/// The QMP and segment sockets `options` asks for, named in `dir`. +fn socket_names( + dir: &Path, + options: &BootOptions, +) -> (Option, Option) { + let qmp = options.qmp.then(|| dir.join("qmp.sock")); + (qmp, options.segment.then(|| super::segment::Tap::in_dir(dir))) +} + /// Every file one boot owns, so that adding another does not lengthen a /// parameter list eight paths long. struct Files { @@ -4794,7 +4817,7 @@ struct Files { uart_log: PathBuf, nvme: NvmeClaim, usb_images: Vec, - qmp_socket: Option, + sockets: Sockets, screendump: PathBuf, own_boot_image: Option, carried: Option>, @@ -4868,7 +4891,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) uart_log, nvme, usb_images, - qmp_socket, + sockets, screendump, own_boot_image, carried, @@ -4968,7 +4991,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) uart_log, nvme, usb_images, - qmp_socket, + sockets, screendump, own_boot_image, boot_log, diff --git a/tests/common/segment.rs b/tests/common/segment.rs index 64f90d0330..4716b3f9c2 100644 --- a/tests/common/segment.rs +++ b/tests/common/segment.rs @@ -12,34 +12,24 @@ use std::io::{Read, Write}; use std::os::unix::net::UnixStream; -use std::path::PathBuf; -use std::sync::atomic::{AtomicU32, Ordering}; +use std::path::{Path, PathBuf}; use std::sync::mpsc::{self, Receiver, RecvTimeoutError}; use std::time::Instant; use toyos_build::icmp::checksum; -/// The two sockets QEMU serves the segment on, which [`super::qemu::BootOptions`] -/// carries into the argv. -#[derive(Clone, Debug)] +/// The two sockets QEMU serves the segment on, in the socket directory of the +/// [`super::qemu::QemuInstance`] that booted with them. +#[derive(Debug)] pub struct Tap { into_guest: PathBuf, from_guest: PathBuf, } impl Tap { - /// Two socket paths of this boot's own, in this thread's scratch directory. - pub fn in_lane() -> Self { - static SEQ: AtomicU32 = AtomicU32::new(0); - let n = SEQ.fetch_add(1, Ordering::Relaxed); - let dir = super::lane::dir(); - let tap = Self { - into_guest: dir.join(format!("tap-in-{n}.sock")), - from_guest: dir.join(format!("tap-out-{n}.sock")), - }; - let _ = std::fs::remove_file(&tap.into_guest); - let _ = std::fs::remove_file(&tap.from_guest); - tap + /// The two sockets' names in a boot's socket directory `dir`. + pub fn in_dir(dir: &Path) -> Self { + Self { into_guest: dir.join("tap-in.sock"), from_guest: dir.join("tap-out.sock") } } /// QEMU's half: two listening sockets, one filter each, both on `net0`. A diff --git a/toyos-tmpdir/src/lib.rs b/toyos-tmpdir/src/lib.rs index 40c3e76c5c..efa210f8a7 100644 --- a/toyos-tmpdir/src/lib.rs +++ b/toyos-tmpdir/src/lib.rs @@ -6,21 +6,25 @@ //! `cargo run -- --ci host` runs every host test against a `$TMPDIR` of its own //! and reds on anything left in it. //! +//! [`TempDir::short`] is the same directory under [`SHORT_BASE`] instead, for +//! a Unix socket: `sockaddr_un.sun_path` is 104 bytes on Darwin, and `$TMPDIR`'s +//! depth is the host's to choose. Everything below holds of each base apart. +//! //! **A process that dies without unwinding is reclaimed by the next one.** //! Every directory a process holds lives under its root, -//! `$TMPDIR/toyos-tmp--/`, beside an [`OWNER`] file the process holds an +//! `/toyos-tmp--/`, beside an [`OWNER`] file the process holds an //! exclusive lock on for as long as the root exists. The kernel lets go of that //! lock when the process dies, by any signal, `SIGKILL` included. The first -//! directory a process makes sweeps `$TMPDIR`: a root whose owner can be locked -//! belongs to a process that is gone, and is removed. +//! directory a process makes under a base sweeps it: a root whose owner can be +//! locked belongs to a process that is gone, and is removed. //! //! **A live process's root is never touched.** Making a root, removing one, and //! the sweep each hold [`GLOBAL`] exclusively, so no sweep sees a root between //! its `mkdir` and its owner's lock, or halfway through its removal. Liveness //! is the lock and never the pid, so a reused pid cannot make a dead root look //! live or a live one look dead; the pid only keeps two roots' names apart. -//! Every process that shares a `$TMPDIR` — every worktree on the host — shares -//! the lock file, because it is in that `$TMPDIR`. A hold on it past +//! Every process that shares a base — every worktree on the host — shares +//! the lock file, because it is in that base. A hold on it past //! `GLOBAL_PATIENCE` panics naming the pid that holds it, rather than hanging //! every worktree on the host behind a stopped process. //! @@ -40,34 +44,48 @@ use std::time::{Duration, Instant}; pub const ROOT_PREFIX: &str = "toyos-tmp-"; /// The lock every root's making and removal and every sweep holds, in -/// `$TMPDIR`. Never removed: a lock file deleted while another process waits +/// its base. Never removed: a lock file deleted while another process waits /// on it would let two holders in at once. pub const GLOBAL: &str = "toyos-tmp.lock"; /// The file in a root its process holds locked for the root's whole life. pub const OWNER: &str = "owner"; -/// A directory of its own under `$TMPDIR`, removed with everything in it when +/// Where [`TempDir::short`] makes its roots. +pub const SHORT_BASE: &str = "/tmp"; + +/// A directory of its own under its base, removed with everything in it when /// this is dropped. #[derive(Debug)] pub struct TempDir { path: PathBuf, + base: Base, } impl TempDir { - /// A fresh, empty directory whose name starts with `label`, which is one - /// path component. Its path is resolved: `/private/var/…` on macOS, never - /// the `/var/…` symlink, so git and a canonicalized comparison agree with it. + /// A fresh, empty directory under `$TMPDIR` whose name starts with `label`, + /// which is one path component. Its path is resolved: `/private/var/…` on + /// macOS, never the `/var/…` symlink, so git and a canonicalized comparison + /// agree with it. pub fn new(label: &str) -> TempDir { + Self::under(Base::TmpDir, label) + } + + /// [`TempDir::new`] under [`SHORT_BASE`], whatever `$TMPDIR` is. + pub fn short(label: &str) -> TempDir { + Self::under(Base::Short, label) + } + + fn under(base: Base, label: &str) -> TempDir { assert!( !label.is_empty() && !label.contains(['/', '\\']) && label != "." && label != "..", "a scratch label is one path component, not {label:?}" ); - let mut state = STATE.lock().unwrap_or_else(PoisonError::into_inner); + let mut state = base.state().lock().unwrap_or_else(PoisonError::into_inner); if state.root.is_none() { - let tmp = std::env::temp_dir(); + let tmp = base.dir(); let tmp = fs::canonicalize(&tmp) - .unwrap_or_else(|e| panic!("resolve $TMPDIR {}: {e}", tmp.display())); + .unwrap_or_else(|e| panic!("resolve {}: {e}", tmp.display())); let root = Root::make(&tmp, &mut state.roots); state.root = Some(root); } @@ -86,7 +104,7 @@ impl TempDir { stuck(&tmp, &dir, e); } } - TempDir { path } + TempDir { path, base } } pub fn path(&self) -> &Path { @@ -117,7 +135,7 @@ impl AsRef for TempDir { impl Drop for TempDir { fn drop(&mut self) { let removed = fs::remove_dir_all(&self.path); - let mut state = STATE.lock().unwrap_or_else(PoisonError::into_inner); + let mut state = self.base.state().lock().unwrap_or_else(PoisonError::into_inner); let root = state.root.as_mut().expect("a TempDir outlived its root"); root.holders -= 1; let last = if root.holders == 0 { state.root.take() } else { None }; @@ -142,7 +160,7 @@ fn fail(what: String) { } /// A gone process's directory the sweep moved into this process's own root but -/// could not then remove: reported once, and moved back out to `$TMPDIR` under +/// could not then remove: reported once, and moved back out to its base under /// a name no sweep reads as a root — `ROOT_PREFIX` is not a prefix of it — so /// this is the only process it ever costs, and no later process on the host /// inherits it and panics in turn. `tmp` is the shared directory the reap @@ -165,18 +183,43 @@ fn stuck(tmp: &Path, dir: &Path, e: std::io::Error) { } } +/// A directory roots are made in. +#[derive(Clone, Copy, Debug)] +enum Base { + TmpDir, + Short, +} + +impl Base { + /// This process's hold on it. + fn state(self) -> &'static Mutex { + static TMPDIR: Mutex = Mutex::new(State { root: None, roots: 0, swept: false }); + static SHORT: Mutex = Mutex::new(State { root: None, roots: 0, swept: false }); + match self { + Base::TmpDir => &TMPDIR, + Base::Short => &SHORT, + } + } + + /// The directory, before it is resolved. + fn dir(self) -> PathBuf { + match self { + Base::TmpDir => std::env::temp_dir(), + Base::Short => PathBuf::from(SHORT_BASE), + } + } +} + struct State { root: Option, /// Roots this process has made, so one made after the last was removed /// does not take its name. roots: u64, - /// Whether this process has swept `$TMPDIR`: once is what reclaims every + /// Whether this process has swept its base: once is what reclaims every /// root a process that died left. swept: bool, } -static STATE: Mutex = Mutex::new(State { root: None, roots: 0, swept: false }); - struct Root { tmp: PathBuf, dir: PathBuf, @@ -198,7 +241,8 @@ impl Root { match fs::create_dir(&dir) { Ok(()) => break dir, // A process that had this pid before this one and died without - // removing its root: the sweep's, not in the way. + // removing its root, the sweep's; or this process's root of the + // other base, where `$TMPDIR` is `/tmp`. Neither is in the way. Err(e) if e.kind() == ErrorKind::AlreadyExists => {} Err(e) => panic!("create {}: {e}", dir.display()), } @@ -232,7 +276,7 @@ impl Root { } impl State { - /// Move every root under `$TMPDIR` whose process is gone into this + /// Move every root under its base whose process is gone into this /// process's own root, and return where each went: the caller removes them /// holding nothing, and a caller that dies meanwhile leaves them in a root /// the next sweep takes. @@ -240,17 +284,13 @@ 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 entries = - fs::read_dir(&root.tmp).unwrap_or_else(|e| panic!("read {}: {e}", root.tmp.display())); let mut reap = Vec::new(); - for entry in entries { - let entry = entry.unwrap_or_else(|e| panic!("read {}: {e}", root.tmp.display())); - let path = entry.path(); - let is_root = entry.file_name().to_str().is_some_and(|n| n.starts_with(ROOT_PREFIX)); - if !is_root || path == root.dir || !gone(&path) { + for path in gone_under(&root.tmp) { + if path == root.dir { continue; } - let to = root.dir.join(format!("reap-{}", entry.file_name().to_string_lossy())); + 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); @@ -259,6 +299,27 @@ impl State { } } +/// Every root under `base` whose process is gone: what a process that died +/// there left, until the next sweep of `base` takes it. +pub fn gone_roots(base: &Path) -> Vec { + let _global = global(base); + gone_under(base) +} + +/// [`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())); + let mut roots = Vec::new(); + for entry in entries { + let entry = entry.unwrap_or_else(|e| panic!("read {}: {e}", base.display())); + let is_root = entry.file_name().to_str().is_some_and(|n| n.starts_with(ROOT_PREFIX)); + if is_root && gone(&entry.path()) { + roots.push(entry.path()); + } + } + roots +} + /// Whether the root at `dir` belongs to a process that is gone. Asked with /// [`GLOBAL`] held, so a root without an owner is one whose making or removal /// was cut short, never one in progress. @@ -311,7 +372,7 @@ fn global_within(tmp: &Path, patience: Duration) -> File { if holder.is_empty() { "an unknown process".to_string() } else { format!("pid {holder}") }; panic!( "{} has been held over {patience:?} by {holder}: every worktree sharing this \ - $TMPDIR is stuck behind it — find and end that process, or wait for it to move on", + base is stuck behind it — find and end that process, or wait for it to move on", path.display() ); } diff --git a/toyos-tmpdir/tests/reclaim.rs b/toyos-tmpdir/tests/reclaim.rs index 8b28c139b2..45786f84f9 100644 --- a/toyos-tmpdir/tests/reclaim.rs +++ b/toyos-tmpdir/tests/reclaim.rs @@ -10,7 +10,7 @@ use std::sync::mpsc::{Receiver, RecvTimeoutError}; use std::sync::{Mutex, MutexGuard, PoisonError}; use std::time::Duration; -use toyos_tmpdir::{TempDir, GLOBAL, OWNER, ROOT_PREFIX}; +use toyos_tmpdir::{TempDir, GLOBAL, OWNER, ROOT_PREFIX, SHORT_BASE}; const HOLD: &str = "TOYOS_TMPDIR_TEST_HOLD"; const HOLDING: &str = "holding "; @@ -28,13 +28,19 @@ fn spawning() -> MutexGuard<'static, ()> { SPAWNING.lock().unwrap_or_else(PoisonError::into_inner) } +/// [`HOLD`]'s value for a holder of a [`TempDir::new`], and of a [`TempDir::short`]. +const IN_TMPDIR: &str = "tmpdir"; +const SHORT: &str = "short"; + /// The holder: this test, run as a child with [`HOLD`] set; a no-op otherwise. #[test] fn holder() { - if std::env::var_os(HOLD).is_none() { - return; - } - let dir = TempDir::new("held"); + let dir = match std::env::var(HOLD) { + Err(std::env::VarError::NotPresent) => return, + Ok(hold) if hold == IN_TMPDIR => TempDir::new("held"), + Ok(hold) if hold == SHORT => TempDir::short("held"), + hold => panic!("{HOLD}={hold:?} is neither {IN_TMPDIR:?} nor {SHORT:?}"), + }; std::fs::write(dir.join("image.img"), b"a guest's disk").unwrap(); println!("{HOLDING}{}", dir.display()); let mut line = String::new(); @@ -54,9 +60,19 @@ impl Holder { /// A holder under `tmp`, returned once its directory exists, and so once its /// sweep of `tmp` is done. fn start(tmp: &Path) -> Holder { + Self::spawn(IN_TMPDIR, tmp) + } + + /// A holder of a [`TempDir::short`], run with `$TMPDIR` at `tmp`, returned + /// once its sweep of [`SHORT_BASE`] is done. + fn short(tmp: &Path) -> Holder { + Self::spawn(SHORT, tmp) + } + + fn spawn(hold: &str, tmp: &Path) -> Holder { let mut child = Command::new(std::env::current_exe().unwrap()) .args(["--exact", "holder", "--nocapture", "--test-threads", "1"]) - .env(HOLD, "1") + .env(HOLD, hold) .env("TMPDIR", tmp) .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -119,30 +135,44 @@ fn entries(tmp: &Path) -> Vec { names } -/// **The whole contract over one `$TMPDIR`**: a killed process's scratch is -/// taken by the next process's first directory, a live process's is left alone -/// however many processes sweep past it, and a process that returns leaves -/// nothing but the lock. +/// **The whole contract, over a `$TMPDIR` and over `/tmp`**: a killed process's +/// scratch is taken by the next process's first directory, a live process's is +/// left alone however many processes sweep past it, and a process that returns +/// leaves nothing but the lock. #[test] fn a_killed_process_is_reclaimed_and_a_live_one_is_never_touched() { let _spawning = spawning(); let tmp = TempDir::new("reclaim"); + reclaimed(|| Holder::start(&tmp), &tmp); + assert_eq!(entries(&tmp), [GLOBAL], "a returned process left something behind"); + // A short directory is under `/tmp` however deep `$TMPDIR` (here `tmp`) is, + // so a socket's path in it fits Darwin's `sun_path`, the shorter of the + // hosts'. + let short_base = std::fs::canonicalize(SHORT_BASE).unwrap(); + reclaimed(|| Holder::short(&tmp), &short_base); +} - let killed = Holder::start(&tmp); +fn reclaimed(start: impl Fn() -> Holder, base: &Path) { + let killed = start(); let killed_root = killed.root(); + assert!( + killed_root.starts_with(base), + "{} is not under {}", + killed_root.display(), + base.display() + ); killed.kill(); assert!(killed_root.exists(), "the premise: SIGKILL leaves the root"); - let live = Holder::start(&tmp); + let live = start(); assert!(!killed_root.exists(), "a killed process's root outlived the next process's sweep"); - let past = Holder::start(&tmp); + let past = start(); assert!(live.dir.join("image.img").exists(), "a sweep took a live process's directory"); let (live_root, past_root) = (live.root(), past.root()); live.finish(); past.finish(); assert!(!live_root.exists() && !past_root.exists(), "a process that returned left its root"); - assert_eq!(entries(&tmp), [GLOBAL], "a returned process left something behind"); } /// A root whose making or removal was cut short, with no owner file, is a gone