test(harness): TOCTOU-safe shared port/spawn helpers across all 33 server-spawning suites

CHANGELOG.md

@@ -6,6 +6,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed — test-harness port-flake sweep (task #18)
+
+33 integration suites that spawn a real `moon` process shared a copy-pasted
+`free_port()` that binds `:0`, reads the port, and drops the listener before
+the server spawns. Two CI-observed failure modes shipped with that pattern:
+a **port TOCTOU** (between probe drop and moon's bind, a concurrent test's
+probe — or an outbound connection's ephemeral source port — takes the port,
+so moon exits with EADDRINUSE) and a **dead-server blind poll** (harnesses
+polled `connect()` for up to 30s without checking child liveness, reporting
+"server never accepted: Connection refused" while the real bind error sat
+unread in the server's stderr log).
+
+- New shared `tests/common/mod.rs`: `reserve_port()` (process-wide dedup set
+  over kernel-chosen probe ports) and `spawn_listening()` (spawns via a
+  caller closure, polls TCP accept **while watching `child.try_wait()`**,
+  and respawns on a fresh port the moment a child dies — external
+  ephemeral-port steals can't be prevented, only recovered from).
+- All 33 suites converted; protocol-level readiness (PING/AUTH) stays with
+  each suite. Kill-9/SIGTERM/restart tests keep their deliberate
+  same-port+same-dir restart legs untouched — only the first spawn of each
+  server lifecycle goes through `spawn_listening`. Expected-startup-failure
+  tests (CLI validation) keep direct spawns on `reserve_port()` ports.
+- Verified: all suites green plus a 10-rep stress running 14 port-hungry
+  suites concurrently (the CI contention pattern that produced the original
+  flakes).
+
 ### Added — R2: multi-shard master PSYNC (task #20, RFC 1B)
 
 A master running `--shards N` now serves full replication to a single-shard

tests/admin_auth_cors_ratelimit.rs

@@ -9,21 +9,12 @@
 
 #![cfg(feature = "console")]
 
-use std::net::TcpListener;
+mod common;
+
 use std::process::{Child, Command, Stdio};
 use std::thread;
 use std::time::{Duration, Instant};
 
-/// Bind a throw-away listener to get a free port, then drop it. There is a
-/// TOCTOU window here but it's good enough for local/CI integration tests.
-fn free_port() -> u16 {
-    TcpListener::bind("127.0.0.1:0")
-        .expect("bind for port probe")
-        .local_addr()
-        .expect("local_addr")
-        .port()
-}
-
 /// RAII wrapper around a moon child process. `Drop` SIGKILLs the child and
 /// waits so the port is released before the next test runs.
 struct Moon {
@@ -53,25 +44,28 @@ fn spawn_moon(extra: &[&str]) -> Option<Moon> {
         );
         return None;
     }
-    let port = free_port();
-    let admin = free_port();
-    let mut args: Vec<String> = vec![
-        "--port".into(),
-        port.to_string(),
-        "--admin-port".into(),
-        admin.to_string(),
-        "--shards".into(),
-        "1".into(),
-    ];
-    for a in extra {
-        args.push((*a).to_string());
-    }
-    let child = Command::new(&bin)
-        .args(&args)
-        .stdout(Stdio::null())
-        .stderr(Stdio::null())
-        .spawn()
-        .ok()?;
+    // The plain client `--port` is never dialed by this suite (only the
+    // admin HTTP port is) — reserve it once, up front, so it doesn't
+    // collide with anything else in-process.
+    let port = common::reserve_port();
+    let extra_owned: Vec<String> = extra.iter().map(|a| (*a).to_string()).collect();
+    let (child, admin) = common::spawn_listening(|admin_port| {
+        let mut args: Vec<String> = vec![
+            "--port".into(),
+            port.to_string(),
+            "--admin-port".into(),
+            admin_port.to_string(),
+            "--shards".into(),
+            "1".into(),
+        ];
+        args.extend(extra_owned.iter().cloned());
+        Command::new(&bin)
+            .args(&args)
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .spawn()
+            .expect("spawn moon")
+    });
     // Poll /healthz until the admin server accepts connections (max 8s).
     let deadline = Instant::now() + Duration::from_secs(8);
     loop {
@@ -203,8 +197,11 @@ fn hard02_wildcard_with_auth_rejected_at_startup() {
     if !bin.exists() {
         return;
     }
-    let port = free_port();
-    let admin = free_port();
+    // Deliberately expects startup to FAIL (wildcard CORS + auth is
+    // rejected before the server ever binds), so `spawn_listening`'s
+    // accept-retry loop doesn't fit here — just reserve two dedup'd ports.
+    let port = common::reserve_port();
+    let admin = common::reserve_port();
     let out = Command::new(&bin)
         .args([
             "--port",

tests/busy_poll_idle.rs

@@ -24,6 +24,8 @@
 #![cfg(all(unix, target_os = "linux", feature = "runtime-monoio"))]
 #![allow(clippy::unwrap_used)]
 
+mod common;
+
 use std::io::{Read, Write};
 use std::net::TcpStream;
 use std::process::{Child, Command};
@@ -33,13 +35,6 @@ fn moon_binary() -> std::path::PathBuf {
     std::path::PathBuf::from(env!("CARGO_BIN_EXE_moon"))
 }
 
-fn free_port() -> u16 {
-    let l = std::net::TcpListener::bind("127.0.0.1:0").expect("bind :0");
-    let p = l.local_addr().expect("local_addr").port();
-    drop(l);
-    p
-}
-
 struct ServerGuard(Child);
 
 impl Drop for ServerGuard {
@@ -49,6 +44,31 @@ impl Drop for ServerGuard {
     }
 }
 
+/// Spawn moon with `--io-busy-poll-us <poll_us>` on `shards` shards and wait
+/// until it ACCEPTS a connection (see `common::spawn_listening`).
+fn spawn_moon(dir: &std::path::Path, shards: u32, poll_us: &str) -> (ServerGuard, u16) {
+    let (child, port) = common::spawn_listening(|port| {
+        Command::new(moon_binary())
+            .args([
+                "--port",
+                &port.to_string(),
+                "--dir",
+                &dir.to_string_lossy(),
+                "--shards",
+                &shards.to_string(),
+                "--appendonly",
+                "no",
+                "--io-busy-poll-us",
+                poll_us,
+            ])
+            .stdout(std::fs::File::create(dir.join("moon.stdout.log")).expect("stdout log"))
+            .stderr(std::fs::File::create(dir.join("moon.stderr.log")).expect("stderr log"))
+            .spawn()
+            .expect("spawn moon")
+    });
+    (ServerGuard(child), port)
+}
+
 fn wait_for_ready(port: u16, deadline: Duration) -> bool {
     let start = Instant::now();
     while start.elapsed() < deadline {
@@ -90,25 +110,7 @@ fn cpu_seconds(pid: u32) -> f64 {
 #[test]
 fn busy_poll_idle_server_does_not_burn_cpu() {
     let dir = tempfile::tempdir().expect("tempdir");
-    let port = free_port();
-    let child = Command::new(moon_binary())
-        .args([
-            "--port",
-            &port.to_string(),
-            "--dir",
-            &dir.path().to_string_lossy(),
-            "--shards",
-            "4",
-            "--appendonly",
-            "no",
-            "--io-busy-poll-us",
-            "200",
-        ])
-        .stdout(std::fs::File::create(dir.path().join("moon.stdout.log")).expect("stdout log"))
-        .stderr(std::fs::File::create(dir.path().join("moon.stderr.log")).expect("stderr log"))
-        .spawn()
-        .expect("spawn moon");
-    let guard = ServerGuard(child);
+    let (guard, port) = spawn_moon(dir.path(), 4, "200");
     let pid = guard.0.id();
 
     assert!(
@@ -138,25 +140,7 @@ fn busy_poll_idle_server_does_not_burn_cpu() {
 #[test]
 fn busy_poll_recovers_after_idle() {
     let dir = tempfile::tempdir().expect("tempdir");
-    let port = free_port();
-    let child = Command::new(moon_binary())
-        .args([
-            "--port",
-            &port.to_string(),
-            "--dir",
-            &dir.path().to_string_lossy(),
-            "--shards",
-            "2",
-            "--appendonly",
-            "no",
-            "--io-busy-poll-us",
-            "40",
-        ])
-        .stdout(std::fs::File::create(dir.path().join("moon.stdout.log")).expect("stdout log"))
-        .stderr(std::fs::File::create(dir.path().join("moon.stderr.log")).expect("stderr log"))
-        .spawn()
-        .expect("spawn moon");
-    let guard = ServerGuard(child);
+    let (guard, port) = spawn_moon(dir.path(), 2, "40");
 
     assert!(
         wait_for_ready(port, Duration::from_secs(15)),

tests/client_tracking_invalidation.rs

@@ -16,16 +16,13 @@
 //! binary is missing (MOON_BIN pin wins, then target/release, then
 //! target/debug).
 
+mod common;
+
 use std::io::{Read, Write};
-use std::net::{TcpListener, TcpStream};
+use std::net::TcpStream;
 use std::process::{Child, Command, Stdio};
 use std::time::{Duration, Instant};
 
-fn free_port() -> u16 {
-    let l = TcpListener::bind("127.0.0.1:0").expect("bind 127.0.0.1:0");
-    l.local_addr().unwrap().port()
-}
-
 fn moon_binary() -> Option<std::path::PathBuf> {
     if let Ok(p) = std::env::var("MOON_BIN") {
         return Some(std::path::PathBuf::from(p));
@@ -55,30 +52,33 @@ impl Drop for Moon {
 
 fn spawn_moon_4shard() -> Option<Moon> {
     let bin = moon_binary()?;
-    let port = free_port();
+    let (child, port) = common::spawn_listening(|port| {
+        let tmp_dir = std::env::temp_dir().join(format!("moon-tracking-{port}"));
+        let _ = std::fs::create_dir_all(&tmp_dir);
+        Command::new(&bin)
+            .args([
+                "--port",
+                &port.to_string(),
+                "--shards",
+                "4",
+                "--admin-port",
+                "0",
+                "--appendonly",
+                "no",
+                // This host hovers near the 5% diskfull line — the guard
+                // would turn every SET into a MOONERR and flake the suite.
+                "--disk-free-min-pct",
+                "0",
+                "--dir",
+                tmp_dir.to_str().unwrap(),
+            ])
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .spawn()
+            .expect("spawn moon")
+    });
+    // Same directory formula the closure used for the winning attempt.
     let tmp_dir = std::env::temp_dir().join(format!("moon-tracking-{port}"));
-    let _ = std::fs::create_dir_all(&tmp_dir);
-    let child = Command::new(&bin)
-        .args([
-            "--port",
-            &port.to_string(),
-            "--shards",
-            "4",
-            "--admin-port",
-            "0",
-            "--appendonly",
-            "no",
-            // This host hovers near the 5% diskfull line — the guard would
-            // turn every SET into a MOONERR and flake the suite.
-            "--disk-free-min-pct",
-            "0",
-            "--dir",
-            tmp_dir.to_str().unwrap(),
-        ])
-        .stdout(Stdio::null())
-        .stderr(Stdio::null())
-        .spawn()
-        .ok()?;
     let moon = Moon {
         child,
         port,

tests/cmd_flush_dbsize_debug_memory.rs

@@ -8,17 +8,12 @@
 //! Run with:
 //!   cargo test --release --test cmd_flush_dbsize_debug_memory
 
-use std::net::TcpListener;
+mod common;
+
 use std::process::{Child, Command, Stdio};
 use std::thread;
 use std::time::{Duration, Instant};
 
-/// Allocate an OS-assigned TCP port, drop the listener, return the number.
-fn free_port() -> u16 {
-    let l = TcpListener::bind("127.0.0.1:0").expect("bind 127.0.0.1:0");
-    l.local_addr().unwrap().port()
-}
-
 fn redis_cli_available() -> bool {
     Command::new("redis-cli")
         .arg("--version")
@@ -63,26 +58,29 @@ fn spawn_moon() -> Option<Moon> {
         );
         return None;
     }
-    let port = free_port();
+    let (child, port) = common::spawn_listening(|port| {
+        let tmp_dir = std::env::temp_dir().join(format!("moon-test-{port}"));
+        let _ = std::fs::create_dir_all(&tmp_dir);
+        Command::new(&bin)
+            .args([
+                "--port",
+                &port.to_string(),
+                "--shards",
+                "1",
+                "--admin-port",
+                "0",
+                "--appendonly",
+                "no",
+                "--persistence-dir",
+                tmp_dir.to_str().unwrap(),
+            ])
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .spawn()
+            .expect("spawn moon")
+    });
+    // Same directory formula the closure used for the winning attempt.
     let tmp_dir = std::env::temp_dir().join(format!("moon-test-{port}"));
-    let _ = std::fs::create_dir_all(&tmp_dir);
-    let child = Command::new(&bin)
-        .args([
-            "--port",
-            &port.to_string(),
-            "--shards",
-            "1",
-            "--admin-port",
-            "0",
-            "--appendonly",
-            "no",
-            "--persistence-dir",
-            tmp_dir.to_str().unwrap(),
-        ])
-        .stdout(Stdio::null())
-        .stderr(Stdio::null())
-        .spawn()
-        .ok()?;
     let moon = Moon {
         child,
         port,