Summary
The unix pid_alive in crates/travsr-plugin-host/src/embed_catalog.rs:485 probes liveness by shelling out to kill -0, and treats a non-zero exit as "not alive". EPERM (the process exists, this uid may not signal it) and ESRCH (no such process) both collapse to false, so a sidecar running under a different uid reads as dead.
This is the exact defect unix_pid_is_alive was written to fix (lib.rs:114-138, #636), which the same crate already exports and which handles EPERM correctly:
match nix::sys::signal::kill(nix::unistd::Pid::from_raw(raw), None) {
Ok(()) => true,
// The process exists; this user just may not signal it.
Err(nix::errno::Errno::EPERM) => true,
Err(_) => false,
}
The Windows arm of pid_alive documents this same class of failure as #500 and fixes it there. The unix arm still has it.
Impact: an orphaned reindex sidecar, reported as a clean shutdown
Both call sites are in terminate_inflight_reindex(), the daemon shutdown path (also reached from stop-embed).
The grace poll (around line 539):
if REINDEX_CHILD_PID.load(Ordering::SeqCst) == 0 || !pid_alive(pid) {
tracing::info!(pid, "embed: reindex drained gracefully after cancel");
...
return;
}
A false pid_alive makes !pid_alive(pid) true on the first iteration, so the daemon logs a successful graceful drain and returns immediately, before waiting any of the grace window. Because it returns there, the kill_pid fallback after the loop (around line 555, itself guarded by a second pid_alive(pid) that would also read false) is never reached.
Net effect when the sidecar runs under a different uid:
- the sidecar is never sent SIGTERM and keeps running after the daemon exits,
- the log positively asserts it "drained gracefully", so the orphan is invisible in the logs,
- the full
CANCEL_GRACE_SECS wait is skipped, so this is not a timing race that a slower host would survive.
Suggested fix
Fold the unix arm onto the existing probe:
#[cfg(unix)]
fn pid_alive(pid: u32) -> bool {
crate::unix_pid_is_alive(pid)
}
That also drops a process spawn per 200 ms poll iteration, and removes the Command/Stdio dependency from this path. unix_pid_is_alive already excludes pid 0 and out-of-range values, so it is strictly more careful than the current version.
Worth a regression test that a pid the current uid cannot signal is reported alive. That is awkward to arrange hermetically without a second uid, so asserting the EPERM-to-true mapping at the unix_pid_is_alive boundary (already covered) plus a test that pid_alive delegates to it may be the practical shape.
Provenance
Raised twice in the #745 review (by @anketpratapsingh as a Major finding, and again in my re-review) and deliberately deferred there: it is pre-existing and sits on a line #745 does not touch, so it was correctly kept out of scope of that PR. Filing it so the deferral is tracked rather than lost. #745 is approved and unaffected.
Verified against origin/master at 5875507, not against the #745 branch, so this stands on its own regardless of what lands there.
Summary
The unix
pid_aliveincrates/travsr-plugin-host/src/embed_catalog.rs:485probes liveness by shelling out tokill -0, and treats a non-zero exit as "not alive".EPERM(the process exists, this uid may not signal it) andESRCH(no such process) both collapse tofalse, so a sidecar running under a different uid reads as dead.This is the exact defect
unix_pid_is_alivewas written to fix (lib.rs:114-138, #636), which the same crate already exports and which handlesEPERMcorrectly:The Windows arm of
pid_alivedocuments this same class of failure as #500 and fixes it there. The unix arm still has it.Impact: an orphaned reindex sidecar, reported as a clean shutdown
Both call sites are in
terminate_inflight_reindex(), the daemon shutdown path (also reached fromstop-embed).The grace poll (around line 539):
A false
pid_alivemakes!pid_alive(pid)true on the first iteration, so the daemon logs a successful graceful drain and returns immediately, before waiting any of the grace window. Because it returns there, thekill_pidfallback after the loop (around line 555, itself guarded by a secondpid_alive(pid)that would also read false) is never reached.Net effect when the sidecar runs under a different uid:
CANCEL_GRACE_SECSwait is skipped, so this is not a timing race that a slower host would survive.Suggested fix
Fold the unix arm onto the existing probe:
That also drops a process spawn per 200 ms poll iteration, and removes the
Command/Stdiodependency from this path.unix_pid_is_alivealready excludes pid 0 and out-of-range values, so it is strictly more careful than the current version.Worth a regression test that a pid the current uid cannot signal is reported alive. That is awkward to arrange hermetically without a second uid, so asserting the
EPERM-to-truemapping at theunix_pid_is_aliveboundary (already covered) plus a test thatpid_alivedelegates to it may be the practical shape.Provenance
Raised twice in the #745 review (by @anketpratapsingh as a Major finding, and again in my re-review) and deliberately deferred there: it is pre-existing and sits on a line #745 does not touch, so it was correctly kept out of scope of that PR. Filing it so the deferral is tracked rather than lost. #745 is approved and unaffected.
Verified against
origin/masterat5875507, not against the #745 branch, so this stands on its own regardless of what lands there.