From ee3f3afd2c8ebeacb372d6be73e52aa84a7347ca Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:33:59 -0400 Subject: [PATCH 1/3] test: e2e server must die with a SIGKILLed test process Adds the shared server-spawn helper (Drop guard only) and a red test that runs the test binary as a subprocess holding a server, SIGKILLs it, and asserts the server is gone. Fails today: the server survives at PPID 1. Refs #1114 skip-changelog: test harness only --- packages/rust/tests/common/server.rs | 51 +++++++++++ packages/rust/tests/server_reaping_e2e.rs | 103 ++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 packages/rust/tests/common/server.rs create mode 100644 packages/rust/tests/server_reaping_e2e.rs diff --git a/packages/rust/tests/common/server.rs b/packages/rust/tests/common/server.rs new file mode 100644 index 000000000..9d3473c92 --- /dev/null +++ b/packages/rust/tests/common/server.rs @@ -0,0 +1,51 @@ +//! Spawn `dirsql server` as a child that cannot outlive its test. + +use std::ffi::OsStr; +use std::ops::{Deref, DerefMut}; +use std::path::Path; +use std::process::{Child, Command, Stdio}; + +use assert_cmd::prelude::*; + +/// A running `dirsql server`, killed and reaped on drop so a panicking test +/// never leaks a process that holds the inherited stderr pipe open. +pub struct ServerGuard(Child); + +impl Deref for ServerGuard { + type Target = Child; + + fn deref(&self) -> &Child { + &self.0 + } +} + +impl DerefMut for ServerGuard { + fn deref_mut(&mut self) -> &mut Child { + &mut self.0 + } +} + +impl Drop for ServerGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +/// Spawn `dirsql server --port --host localhost` in `cwd` with `extra` +/// args appended. stdout is piped; stderr is inherited so failures surface in +/// test output. +pub fn spawn_server>(cwd: &Path, port: u16, extra: &[S]) -> ServerGuard { + let mut cmd: Command = Command::cargo_bin("dirsql") + .expect("`dirsql` binary must be built by `cargo test` with --features cli"); + cmd.arg("server") + .arg("--port") + .arg(port.to_string()) + .arg("--host") + .arg("localhost") + .args(extra) + .current_dir(cwd) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()); + ServerGuard(cmd.spawn().expect("spawning dirsql failed")) +} diff --git a/packages/rust/tests/server_reaping_e2e.rs b/packages/rust/tests/server_reaping_e2e.rs new file mode 100644 index 000000000..84fbd8dec --- /dev/null +++ b/packages/rust/tests/server_reaping_e2e.rs @@ -0,0 +1,103 @@ +//! The shared server helper must not leak `dirsql server` children when the +//! test process dies without unwinding (OOM kill, cargo-mutants timeout, +//! Ctrl-C). Nothing is mocked: a real test binary holding a real server is +//! SIGKILLed. +//! +//! `server_dies_when_test_process_is_sigkilled` runs this same test binary as +//! a subprocess filtered to the ignored `hold_a_server` helper, which starts a +//! server through the shared helper, waits for its startup banner, prints the +//! server's pid, and sleeps. The driver reads that pid, SIGKILLs the helper, +//! and asserts the server is gone. + +#![cfg(all(feature = "cli", target_os = "linux"))] + +#[path = "common/server.rs"] +mod server; + +use std::io::{BufRead, BufReader}; +use std::net::TcpListener; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +use tempfile::TempDir; + +const PID_PREFIX: &str = "server-pid="; + +#[test] +#[ignore = "subprocess helper for server_dies_when_test_process_is_sigkilled"] +fn hold_a_server() { + let root = TempDir::new().unwrap(); + let port = TcpListener::bind("localhost:0") + .unwrap() + .local_addr() + .unwrap() + .port(); + let mut server = server::spawn_server::<&str>(root.path(), port, &[]); + let mut banner = String::new(); + BufReader::new(server.stdout.take().unwrap()) + .read_line(&mut banner) + .unwrap(); + assert!( + banner.contains(&format!("localhost:{port}")), + "unexpected banner: {banner:?}" + ); + println!("{PID_PREFIX}{}", server.id()); + std::thread::sleep(Duration::from_secs(60)); + drop(server); +} + +fn alive(pid: libc::pid_t) -> bool { + #[expect(unsafe_code, reason = "no safe std API probes another process")] + unsafe { + libc::kill(pid, 0) == 0 + } +} + +fn sigkill(pid: libc::pid_t) { + #[expect( + unsafe_code, + reason = "no safe std API sends a signal to another process" + )] + unsafe { + libc::kill(pid, libc::SIGKILL); + } +} + +#[test] +fn server_dies_when_test_process_is_sigkilled() { + let mut helper = Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "hold_a_server", "--ignored", "--nocapture"]) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect("spawning the helper test failed"); + let mut helper_stdout = BufReader::new(helper.stdout.take().unwrap()); + let server_pid: libc::pid_t = loop { + let mut line = String::new(); + assert_ne!( + helper_stdout.read_line(&mut line).unwrap(), + 0, + "helper exited before printing the server pid" + ); + if let Some(pid) = line.trim_end().strip_prefix(PID_PREFIX) { + break pid.parse().unwrap(); + } + }; + assert!( + alive(server_pid), + "server {server_pid} must be running before its parent dies" + ); + + sigkill(i32::try_from(helper.id()).unwrap()); + helper.wait().unwrap(); + drop(helper_stdout); + + let deadline = Instant::now() + Duration::from_secs(5); + while alive(server_pid) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(50)); + } + if alive(server_pid) { + sigkill(server_pid); + panic!("dirsql server {server_pid} outlived its SIGKILLed test process"); + } +} From fe4be59dc1fdc2116ba8f7523e55c04ffbfc016b Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:36:56 -0400 Subject: [PATCH 2/3] test: reap e2e dirsql servers when the test process dies The shared spawn helper sets PR_SET_PDEATHSIG=SIGKILL on the child in pre_exec (Linux), so a server dies with a test process that is SIGKILLed and never unwinds. cli_e2e, cli_repeatable_config_e2e and on_file_e2e now spawn through it and hold the Drop guard, which also covers panics. Fixes #1114 skip-changelog: test harness only --- packages/rust/tests/cli_e2e.rs | 102 +++++------------- .../rust/tests/cli_repeatable_config_e2e.rs | 34 +++--- packages/rust/tests/common/server.rs | 20 ++++ packages/rust/tests/on_file_e2e.rs | 54 ++++------ 4 files changed, 82 insertions(+), 128 deletions(-) diff --git a/packages/rust/tests/cli_e2e.rs b/packages/rust/tests/cli_e2e.rs index 3b59ca088..a48ef0a3b 100644 --- a/packages/rust/tests/cli_e2e.rs +++ b/packages/rust/tests/cli_e2e.rs @@ -13,15 +13,20 @@ #![cfg(feature = "cli")] +#[path = "common/server.rs"] +mod server; + +use std::ffi::OsStr; use std::fs; use std::io::{BufRead, BufReader}; use std::net::TcpListener; -use std::process::{Child, Command as StdCommand, Stdio}; +use std::process::Child; use std::time::{Duration, Instant}; use assert_cmd::prelude::*; use reqwest::{StatusCode, blocking::Client}; use serde_json::{Value, json}; +use server::ServerGuard; use tempfile::TempDir; /// Write a two-post blog fixture into a fresh tempdir and return it. @@ -80,41 +85,19 @@ fn free_port() -> u16 { .port() } -/// Spawn `dirsql` as a subprocess bound to `--port ` in `dir`. -/// The child inherits stderr so failures surface in test output. -fn spawn_dirsql(dir: &std::path::Path, port: u16) -> Child { - let mut cmd: StdCommand = std::process::Command::cargo_bin("dirsql") - .expect("`dirsql` binary must be built by `cargo test` with --features cli"); - // #662: the HTTP server moved behind the `server` subcommand; `--host` / - // `--port` are now server-local flags. - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost") - .current_dir(dir) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - cmd.spawn().expect("spawning dirsql failed") +/// Spawn `dirsql server` bound to `--port ` in `dir`. +fn spawn_dirsql(dir: &std::path::Path, port: u16) -> ServerGuard { + spawn_dirsql_with_args::<&str>(dir, port, &[]) } /// Spawn `dirsql server` bound to `--port ` in `dir` with `extra` args -/// appended (e.g. `--persist`). Mirrors [`spawn_dirsql`] otherwise. -fn spawn_dirsql_with_args(dir: &std::path::Path, port: u16, extra: &[&str]) -> Child { - let mut cmd: StdCommand = std::process::Command::cargo_bin("dirsql") - .expect("`dirsql` binary must be built by `cargo test` with --features cli"); - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost"); - for a in extra { - cmd.arg(a); - } - cmd.current_dir(dir) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - cmd.spawn().expect("spawning dirsql failed") +/// appended (e.g. `--persist`). +fn spawn_dirsql_with_args>( + dir: &std::path::Path, + port: u16, + extra: &[S], +) -> ServerGuard { + server::spawn_server(dir, port, extra) } /// Block until the server answers `GET /query` (or times out). @@ -135,7 +118,7 @@ fn wait_until_ready(port: u16, timeout: Duration) { panic!("dirsql server did not become ready on port {port} within {timeout:?}"); } -fn kill_and_wait(mut child: Child) { +fn kill_and_wait(mut child: ServerGuard) { // Prefer polite shutdown; fall back to kill if the child hangs. #[cfg(unix)] { @@ -1382,19 +1365,9 @@ on-file = "printf '[{}]'" .unwrap(); let port = free_port(); - let mut cmd: StdCommand = - std::process::Command::cargo_bin("dirsql").expect("binary must exist"); - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost") - .arg("--config") - .arg(elsewhere.path().join(".dirsql.toml")) - .current_dir(cwd.path()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - let child = cmd.spawn().expect("spawn"); + let config = elsewhere.path().join(".dirsql.toml"); + let child = + spawn_dirsql_with_args(cwd.path(), port, &["--config".as_ref(), config.as_os_str()]); wait_until_ready(port, Duration::from_secs(10)); let resp = Client::new() @@ -1421,19 +1394,12 @@ fn explicit_config_flag_loads_the_named_config() { let elsewhere = TempDir::new().unwrap(); let port = free_port(); - let mut cmd: StdCommand = - std::process::Command::cargo_bin("dirsql").expect("binary must exist"); - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost") - .arg("--config") - .arg(fixture.path().join(".dirsql.toml")) - .current_dir(elsewhere.path()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - let child = cmd.spawn().expect("spawn"); + let config = fixture.path().join(".dirsql.toml"); + let child = spawn_dirsql_with_args( + elsewhere.path(), + port, + &["--config".as_ref(), config.as_os_str()], + ); wait_until_ready(port, Duration::from_secs(10)); let resp = Client::new() @@ -1453,19 +1419,9 @@ fn short_config_flag_loads_the_named_config() { let elsewhere = TempDir::new().unwrap(); let port = free_port(); - let mut cmd: StdCommand = - std::process::Command::cargo_bin("dirsql").expect("binary must exist"); - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost") - .arg("-c") - .arg(fixture.path().join(".dirsql.toml")) - .current_dir(elsewhere.path()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - let child = cmd.spawn().expect("spawn"); + let config = fixture.path().join(".dirsql.toml"); + let child = + spawn_dirsql_with_args(elsewhere.path(), port, &["-c".as_ref(), config.as_os_str()]); wait_until_ready(port, Duration::from_secs(10)); let resp = Client::new() diff --git a/packages/rust/tests/cli_repeatable_config_e2e.rs b/packages/rust/tests/cli_repeatable_config_e2e.rs index f0b117283..0a50618c9 100644 --- a/packages/rust/tests/cli_repeatable_config_e2e.rs +++ b/packages/rust/tests/cli_repeatable_config_e2e.rs @@ -8,10 +8,13 @@ //! Gated behind `--features cli` like the sibling `cli_e2e.rs`. #![cfg(feature = "cli")] +#[path = "common/server.rs"] +mod server; + use std::fs; use std::net::TcpListener; use std::path::Path; -use std::process::{Child, Command as StdCommand, Stdio}; +use std::process::{Child, Command as StdCommand}; use std::time::{Duration, Instant}; use assert_cmd::prelude::*; @@ -83,22 +86,16 @@ fn server_serves_tables_from_two_config_flags() { let cfg_b_path = write_table_config(cfg_b.path(), "beta", "b.json"); let port = free_port(); - let mut cmd: StdCommand = std::process::Command::cargo_bin("dirsql") - .expect("`dirsql` binary must be built by `cargo test` with --features cli"); - // #662: server flags live under the `server` subcommand now. - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost") - .arg("-c") - .arg(&cfg_a_path) - .arg("-c") - .arg(&cfg_b_path) - .current_dir(data.path()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - let mut child = cmd.spawn().expect("spawning dirsql failed"); + let mut child = server::spawn_server( + data.path(), + port, + &[ + "-c".as_ref(), + cfg_a_path.as_os_str(), + "-c".as_ref(), + cfg_b_path.as_os_str(), + ], + ); wait_until_ready_or_exit(&mut child, port, Duration::from_secs(10)); @@ -118,8 +115,7 @@ fn server_serves_tables_from_two_config_flags() { assert_eq!(body, vec![json!({"n": 1})], "table {table} must be indexed"); } - let _ = child.kill(); - let _ = child.wait(); + drop(child); } #[test] diff --git a/packages/rust/tests/common/server.rs b/packages/rust/tests/common/server.rs index 9d3473c92..baa8ce1cc 100644 --- a/packages/rust/tests/common/server.rs +++ b/packages/rust/tests/common/server.rs @@ -1,4 +1,8 @@ //! Spawn `dirsql server` as a child that cannot outlive its test. +//! +//! The guard reaps the server when the test unwinds. A SIGKILLed test process +//! never unwinds, so on Linux the child also asks the kernel to SIGKILL it +//! when its parent thread dies (`PR_SET_PDEATHSIG`). use std::ffi::OsStr; use std::ops::{Deref, DerefMut}; @@ -47,5 +51,21 @@ pub fn spawn_server>(cwd: &Path, port: u16, extra: &[S]) -> Serv .current_dir(cwd) .stdout(Stdio::piped()) .stderr(Stdio::inherit()); + #[cfg(target_os = "linux")] + { + use std::os::unix::process::CommandExt; + #[expect( + unsafe_code, + reason = "pre_exec runs between fork and exec; prctl is async-signal-safe" + )] + unsafe { + cmd.pre_exec(|| { + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + } ServerGuard(cmd.spawn().expect("spawning dirsql failed")) } diff --git a/packages/rust/tests/on_file_e2e.rs b/packages/rust/tests/on_file_e2e.rs index 3d54efd62..09c43b3cd 100644 --- a/packages/rust/tests/on_file_e2e.rs +++ b/packages/rust/tests/on_file_e2e.rs @@ -10,14 +10,16 @@ #![cfg(all(feature = "cli", unix))] +#[path = "common/server.rs"] +mod server; + use std::fs; use std::net::TcpListener; -use std::process::{Child, Command as StdCommand, Stdio}; use std::time::{Duration, Instant}; -use assert_cmd::prelude::*; use reqwest::{StatusCode, blocking::Client}; use serde_json::{Value, json}; +use server::ServerGuard; use tempfile::TempDir; fn free_port() -> u16 { @@ -28,7 +30,7 @@ fn free_port() -> u16 { .port() } -fn spawn_dirsql(dir: &std::path::Path, port: u16) -> Child { +fn spawn_dirsql(dir: &std::path::Path, port: u16) -> ServerGuard { // Bare `dirsql` no longer auto-loads a cwd `.dirsql.toml` (#602); these // fixtures place their config at the index root, so pass it explicitly. let config = dir.join(".dirsql.toml"); @@ -37,22 +39,15 @@ fn spawn_dirsql(dir: &std::path::Path, port: u16) -> Child { /// Spawn `dirsql` from working directory `cwd`, optionally pointing `--config` /// at a config file elsewhere (so the config dir can differ from the cwd). -fn spawn_dirsql_with(cwd: &std::path::Path, config: Option<&std::path::Path>, port: u16) -> Child { - let mut cmd: StdCommand = std::process::Command::cargo_bin("dirsql") - .expect("`dirsql` binary must be built with --features cli"); - // #662: the HTTP server is behind the `server` subcommand now. - cmd.arg("server") - .arg("--port") - .arg(port.to_string()) - .arg("--host") - .arg("localhost"); - if let Some(config) = config { - cmd.arg("--config").arg(config); +fn spawn_dirsql_with( + cwd: &std::path::Path, + config: Option<&std::path::Path>, + port: u16, +) -> ServerGuard { + match config { + Some(config) => server::spawn_server(cwd, port, &["--config".as_ref(), config.as_os_str()]), + None => server::spawn_server::<&str>(cwd, port, &[]), } - cmd.current_dir(cwd) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()); - cmd.spawn().expect("spawning dirsql failed") } fn wait_until_ready(port: u16, timeout: Duration) { @@ -71,21 +66,8 @@ fn wait_until_ready(port: u16, timeout: Duration) { panic!("dirsql server did not become ready on port {port} within {timeout:?}"); } -fn kill_and_wait(mut child: Child) { - let _ = child.kill(); - let _ = child.wait(); -} - -/// Kills the spawned server on drop, so a panicking test never leaks a process -/// that keeps the inherited stderr pipe open (which would stall the CI step -/// waiting for EOF). -struct ServerGuard(Child); - -impl Drop for ServerGuard { - fn drop(&mut self) { - let _ = self.0.kill(); - let _ = self.0.wait(); - } +fn kill_and_wait(child: ServerGuard) { + drop(child); } #[test] @@ -162,7 +144,7 @@ on-file = "sh abscheck.sh {path}" .unwrap(); let port = free_port(); - let _server = ServerGuard(spawn_dirsql(root.path(), port)); + let _server = spawn_dirsql(root.path(), port); wait_until_ready(port, Duration::from_secs(10)); let resp = Client::new() @@ -209,11 +191,11 @@ on-file = "sh abscheck.sh {path}" .unwrap(); let port = free_port(); - let _server = ServerGuard(spawn_dirsql_with( + let _server = spawn_dirsql_with( project.path(), Some(&configdir.path().join(".dirsql.toml")), port, - )); + ); wait_until_ready(port, Duration::from_secs(10)); let resp = Client::new() From d6571bbd8c136311316bceb3448ff37f3b0088af Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:35:29 -0400 Subject: [PATCH 3/3] chore: retrigger CI on a fresh head sha The stale release-ci check runs left on the previous sha carry version numbers willfire no longer predicts, so CI Gate reports them as unpredicted checks and cannot confirm the set is complete.