From da87f978cd7641c6afae22f355c54a01adf4c1c2 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Mon, 20 Apr 2026 16:24:42 -0400
Subject: [PATCH] fix(proc): revert process_running to cheap kill(0) probe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit 1ed8e6cbd changed process_running(pid) to shell out to `ps` on
every call to distinguish running processes from zombies. That cost
~2 ms per invocation on macOS (fork + exec + wait), and the test
harness calls process_running O(tests × markers) times under session
flock contention. Across a `cargo nextest run -p fabro-cli` that added
up to ~90 s of suite time, and the zombie-aware semantics turned out
to have no production caller on Unix (the server's worker-termination
loop uses process_group_alive; the CLI stop/status paths don't need
zombie detection for a daemon that reparents to init).

Restore the pre-1ed8e6cbd body: process_running is now a straight
kill(pid, 0) via process_exists on Unix, true on non-unix. Delete
unix_process_state (the `ps` helper) and its zombie regression test,
since they describe behavior we're rolling back. process_group_alive
and its tests are unchanged.

Measured on this branch against baseline db953c838:
  reap_nextest p50:   172 ms -> 0.3 ms
  TestContext::new:  316 ms mean -> 15 ms mean

If a future caller genuinely needs zombie-aware semantics, add it back
alongside that caller with a benchmark in context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/crates/fabro-proc/src/signal.rs | 70 ++++-------------------------
 1 file changed, 8 insertions(+), 62 deletions(-)

diff --git a/lib/crates/fabro-proc/src/signal.rs b/lib/crates/fabro-proc/src/signal.rs
index a418b6dca..7325c3f3d 100644
--- a/lib/crates/fabro-proc/src/signal.rs
+++ b/lib/crates/fabro-proc/src/signal.rs
@@ -20,42 +20,15 @@ pub fn process_exists(pid: u32) -> bool {
 
 /// Check whether a process with the given PID is still running.
 ///
-/// On Unix, this treats zombie / defunct processes as not running even though
-/// they still have a visible PID until their parent reaps them. If the
-/// follow-up `ps` probe fails, this falls back to `process_exists(pid)` to
-/// preserve the old conservative behavior.
+/// On Unix, delegates to `process_exists` (a `kill(pid, 0)` probe). Unreaped
+/// zombies count as running here because callers that need to distinguish
+/// zombies from live processes are a narrow minority; giving every caller
+/// the zombie check would require spawning `ps` on every probe and would
+/// dominate test-harness setup time at the scale we run it. If a caller
+/// needs zombie-aware semantics, it should be introduced alongside that
+/// caller with a benchmark in context.
 pub fn process_running(pid: u32) -> bool {
-    #[cfg(unix)]
-    {
-        if !process_exists(pid) {
-            return false;
-        }
-
-        unix_process_state(pid).is_none_or(|state| !matches!(state, 'Z' | 'z'))
-    }
-    #[cfg(not(unix))]
-    {
-        process_exists(pid)
-    }
-}
-
-#[cfg(unix)]
-#[expect(
-    clippy::disallowed_methods,
-    reason = "Unix process-state detection shells out to ps to distinguish running processes from zombies"
-)]
-fn unix_process_state(pid: u32) -> Option<char> {
-    let output = std::process::Command::new("ps")
-        .args(["-ww", "-o", "stat=", "-p", &pid.to_string()])
-        .output()
-        .ok()?;
-    if !output.status.success() {
-        return None;
-    }
-
-    String::from_utf8_lossy(&output.stdout)
-        .chars()
-        .find(|ch| !ch.is_whitespace())
+    process_exists(pid)
 }
 
 /// Check whether any process in the given process group is alive.
@@ -164,33 +137,6 @@ mod tests {
         assert!(process_running(std::process::id()));
     }
 
-    #[cfg(unix)]
-    #[test]
-    #[expect(
-        clippy::disallowed_methods,
-        reason = "process-state test needs to spawn a short-lived child and intentionally leave it unreaped"
-    )]
-    fn process_running_returns_false_for_unreaped_zombie_child() {
-        let mut child = Command::new("sh")
-            .args(["-c", "exit 0"])
-            .spawn()
-            .expect("short-lived child should spawn");
-        let pid = child.id();
-
-        std::thread::sleep(Duration::from_millis(100));
-
-        assert!(
-            process_exists(pid),
-            "unreaped zombie should still have a visible pid"
-        );
-        assert!(
-            !process_running(pid),
-            "unreaped zombie should not count as a running process"
-        );
-
-        let _status = child.wait().expect("child should remain waitable");
-    }
-
     #[cfg(unix)]
     #[test]
     #[expect(