fix(daemon): verify PID identity so a recycled PID can't wedge startup - #551
Merged
Conversation
`check_stale_pid` gated the singleton guard on `is_process_alive(pid)`, a bare `OpenProcess`/`kill(0)` existence probe. On Windows that is unsound: PIDs are recycled aggressively, and a terminated daemon can linger as a zombie while a handle to it stays open. A stale PID file (left when a foreground `uffsd` is Ctrl-C'd without cleanup) then resolves to a live but unrelated process, so the guard concludes "another daemon is running" and refuses to start — forever, because the stale-file cleanup only runs on the dead-PID branch. Every `uffsd`/`uffs --daemon start` then fails with "Another daemon instance is already running" / "did not become ready in time" until the user manually deletes daemon.pid. Root cause: the liveness check confirmed *a* process existed, never that it was uffsd. The client already got this right (`is_pid_alive` and the connect-time identity check both confirm the image is uffsd); the daemon had not caught up. Fix: layer an identity check over the liveness probe. `is_daemon_process_alive` resolves the live process's image path (QueryFullProcessImageNameW on Windows, proc_pidpath/`/proc` on unix) and only honors the singleton guard when the image is a uffsd binary. A recycled/foreign PID is treated as a stale file and reclaimed; an unreadable image errs toward "running" so two daemons never race to bind the pipe. The decision is split into a pure, exhaustively unit-tested helper (`pid_entry_is_running_daemon`) plus a name predicate (`is_uffsd_image`); `check_stale_pid_at` is extracted so the whole flow is testable against a tempfile. Also make `uffs --status` honest: instead of always labelling an unreachable-but-present PID file "stale", report a live daemon that is not answering on IPC (still loading, or wedged) distinctly, so a wedged daemon no longer masquerades as "not running". Tests: recycled-PID reclaim, dead/zero/unparseable PID reclaim, missing file, full identity-decision branch matrix, and daemon-name matching (incl. a Windows-only backslash-path case). Validated with lint-prod, lint-prod-windows, lint-tests, lint-tests-windows, rustdoc, and fmt.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The daemon singleton guard gated on a bare liveness probe (
is_process_alive(pid)—OpenProcess/kill(0)). On Windows that is unsound: PIDs are recycled aggressively and a terminated daemon can linger as a zombie. A staledaemon.pid(left when a foregrounduffsdis Ctrl-C'd without cleanup) then resolves to a live but unrelated process, so the guard concludes "another daemon is running" and refuses to start — forever, because the stale-file cleanup only ran on the dead-PID branch. Everyuffsd/uffs --daemon startthen failed with "Another daemon instance is already running" / "did not become ready in time" until the user manually deleteddaemon.pid.Root cause: the liveness check confirmed a process existed, never that it was
uffsd. The client already got this right; the daemon had not caught up.What changed
is_daemon_process_aliveresolves the live process's image path (QueryFullProcessImageNameWon Windows,proc_pidpath//procon unix) and only honors the singleton guard when the image is auffsdbinary. A recycled/foreign PID is treated as a stale file and reclaimed; an unreadable image errs toward "running" so two daemons never race to bind the pipe.pid_entry_is_running_daemon) plus a name predicate (is_uffsd_image);check_stale_pid_atis extracted so the whole flow runs against a tempfile.uffs --status. An unreachable-but-present PID file is no longer always labelled "stale" — a live daemon that is not answering on IPC (still loading, or wedged) is reported distinctly.Testing
Recycled-PID reclaim, dead/zero/unparseable PID reclaim, missing file, full identity-decision branch matrix, and daemon-name matching (incl. a Windows-only backslash-path case). Validated locally with
lint-prod,lint-prod-windows,lint-tests,lint-tests-windows,rustdoc, andfmt.