Repository navigation
[AI-839] Fix daemon PID file identity check on Linux + detached stdio hang - #147
Conversation
… hang
The daemon lifecycle compared Process.StartTime for exact tick-equality to
decide whether the process at a PID is still our daemon. On Linux that value
is recomputed per process from a boot-time estimate (wall clock - uptime), so
the daemon and a later status/stop/doctor invocation disagree by ms-to-seconds.
Every live daemon was therefore misread as a stale PID file: status reported
"stale" and deleted the file, doctor then showed "(no pid file)" while the
flock was HELD, stop no-op'd, and start -d re-spawned only to collide on the
flock ("already running under the name"). macOS uses an absolute sysctl start
timestamp that is identical across processes, so it never reproduced.
Introduce ProcessStartToken: on Linux it reads /proc/<pid>/stat field 22 (the
kernel's boot-relative starttime, byte-identical for every reader and immune to
clock adjustments); on macOS/Windows it keeps Process.StartTime.Ticks. The
daemon and the CLI supervisor both write it, and IsOurDaemon compares it. A
token mismatch stays conclusive (rejects recycled PIDs); the weaker name
fallback applies only when the token can't be read.
Also fix the secondary symptom where `kcap daemon start -d` hangs for an
output-capturing caller: the detached daemon inherited our stdout/stderr and
held the pipe open for its lifetime. Redirect all three streams, prevent
Windows handle inheritance, and close them parent-side (same AI-820 idiom as
the watcher / what's-done spawns).
Verified on real Linux: start -d -> status running -> duplicate start refused
-> stop kills -> status clean -> doctor cleanable; `start -d | cat` returns
promptly instead of hanging. Adds regression tests (the /proc assertion runs on
Linux); full unit suite passes; both AOT publishes are IL-warning clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review by Qodo
1.
|
PR Summary by QodoFix Linux daemon PID identity checks and prevent detached stdio hangs WalkthroughsDescription• Introduce a cross-process stable start token to validate daemon PID files on Linux. • Update daemon CLI lifecycle checks (start/status/stop/doctor) to use the new token. • Redirect and close all stdio streams for detached daemon starts to avoid pipe hangs. Diagramgraph TD
cli["kcap CLI"] --> pid[("PID file")] --> check["IsOurDaemon()"] --> token["ProcessStartToken"]
token --> proc{{"/proc/<pid>/stat"}}
token --> st{{"Process.StartTime"}}
cli --> detach["StartDetached()"] --> stdio["Redirect+close stdio"] --> daemon["kcap-daemon"]
subgraph Legend
direction LR
_p["Process/component"] ~~~ _f[("File")] ~~~ _e{{"OS API"}}
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Compare Process.StartTime with tolerance on Linux
2. Rely solely on the per-name flock (no PID identity check)
Recommendation: The PR’s approach (persist a stable, kernel-derived start token on Linux and compare it authoritatively when readable) is the most reliable way to distinguish a live daemon from PID reuse across separate CLI invocations. The detached stdio fix is also the correct strategy for avoiding pipe hangs when the caller captures output. File ChangesBug fix (3)
Tests (2)
|
…reboots Address qodo review on #147: - Legacy token stranded a running daemon across upgrade (correctness): a pre-AI-839 PID file stored a bare Process.StartTime tick count on line 2. IsOurDaemon treated any readable mismatch as conclusive, so after a CLI upgrade the still-running old daemon was classified stale and its PID file deleted — reintroducing the AI-839 symptom. ProcessStartToken now emits a scheme-prefixed token and exposes a tri-state Matches(): same-scheme mismatch stays conclusive (recycled-PID rejection), but a legacy/foreign-scheme or unreadable token returns null so IsOurDaemon falls back to the image-name check and keeps the old daemon manageable. Verified on Linux: a live daemon with a bare-ticks PID file now reports running and `stop` kills it. - Linux token lacked a boot id (reliability): the token was only boot-relative starttime, so a PID file surviving a reboot could match an unrelated process reusing the PID and tick offset. Token is now lx:<boot_id>:<starttime> using /proc/sys/kernel/random/boot_id; macOS/Windows stay tk:<absolute-ticks>. The third finding (detached daemon Console writes failing on the closed pipe) does not reproduce: a detached early-exit path (bad --max-agents) still exits with its intended code 1 — .NET tolerates the broken-pipe console write — so the exit-code/failure behavior is unchanged. Left as-is. Adds Matches tri-state tests (self/same-scheme-mismatch/legacy/dead-pid) and updates the Linux token assertion. Full unit suite green; no AOT IL warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the qodo review in 00c5212: 1. Legacy token blocks stop (Action required) — Fixed. Tokens are now scheme-prefixed and 3. Linux token lacks boot ID (Optional) — Fixed in the same change. Linux token is now 2. Detached stderr/out may fail (Recommended) — Not reproducible, leaving as-is. I exercised a detached early-exit path ( |
Problem (AI-839)
A live, server-registered daemon could not be managed from the CLI on Linux:
statusreportednot running (stale PID file),doctorshowedHELD … (no pid file),stop --yeswas a no-op, andstart -drefused with "already running under the name" — leaving no CLI path to restart it.kcap daemon start -dalso hung instead of returning.Root cause
The lifecycle decides "is the process at this PID still our daemon?" by comparing
Process.StartTimefor exact tick-equality. On Linux that value is recomputed per process from a boot-time estimate (wall clock − uptime), so the daemon and a laterstatus/stop/doctorinvocation disagree by ms-to-seconds (measured ~0.8 ms in a Linux container). Every live daemon was therefore misread as a stale PID file:status→ "stale" and deletes the pid filedoctor→ then shows(no pid file)while the flock is genuinelyHELDstop→ no-opstart -d→ re-spawns, new daemon collides on the flock → "already running under the name"macOS uses an absolute
sysctlstart timestamp that is identical across processes, so it never reproduced.Secondary:
StartDetachedredirected only stdin, so the detached daemon inherited the caller's stdout/stderr and held the pipe open for life →start -dhung for any output-capturing caller.Fix
ProcessStartToken(Core) — a cross-process-stable start token. Linux reads/proc/<pid>/statfield 22 (kernel boot-relativestarttime, byte-identical for every reader, immune to clock drift); macOS/Windows keepProcess.StartTime.Ticks. Used byDaemonLock.WritePidFile, the CLIWritePidFile/ReadPidFile/IsOurDaemon. A token mismatch stays conclusive (rejects recycled PIDs); the name fallback only applies when the token is unreadable.PreventInheritedStdHandles()(Windows), close them parent-side (same AI-820 idiom as the watcher / what's-done spawns).Verification (real Linux, .NET SDK container)
start -d→statusrunning → duplicatestart -dcorrectly refused →stopkills it →statusclean →doctorSTALE/cleanable.kcap daemon start -d | catreturns in ~1 s (was a hang)./procassertion runs on Linux); full unit suite 1412/1412; both AOT publishes IL-warning clean.🤖 Generated with Claude Code