[Bug/RFC] Forced restart race can leave a replacement daemon alive but untracked #634
brandon-julio-t
started this conversation in
General
Replies: 1 comment
|
I believe this will be fixed by #632 but I'm overhauling the restart logic anyhow |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
A forced daemon restart can race with the old process monitor exit path. The replacement process is successfully spawned and recorded as running, then a late state write from the old monitor clears the replacement PID and changes the daemon back to stopped.
The replacement process remains alive but is no longer tracked by the supervisor. If it later exits, retry logic does not restart it because the supervisor already considers the daemon stopped.
Observed production interleaving
This happened on Pitchfork v2.17.0 during a scheduled restart using
retrigger = "always":The replacement continued working for several hours while untracked. When it later exited with an application error,
retry = truedid not take effect.The relevant invariant appears to be:
Root cause
In v2.17.0, the monitor exit path reads daemon state and later performs an unconditional upsert. The state read and final write are not one atomic ownership check. A replacement can be installed between them.
Current
mainincludes a pre-drain stopping snapshot that narrows one timing window, but the final state transition is still separate from its ownership check.There is a second related lifecycle race: concurrent forced starts can both observe a startable state while one stop is in progress. A late stop completion can then overwrite the newly registered owner unless check, stop, and spawn are serialized for a daemon.
Deterministic reproduction
The minimal state-level reproduction is:
pid=None / stopped.A behavioral reproduction starts restart A, waits until the daemon is explicitly
stopping, then starts restart B. The final tracked PID must be alive and every prior supervisor-recorded PID must be dead.A retry-specific case is also important: readiness failure can be reported before graceful termination completes. A retry must replace its own still-live failed PID, but must not replace a different PID installed by a newer start.
Proposed direction
I have a Draft RFC implementation that:
Draft RFC PR: #635
Related work
I am happy to adapt the proposed implementation or combine it with #620 if that better fits the intended supervisor architecture.
Authorship note: the investigation, reproduction, and proposed implementation were assisted by OpenAI GPT-5.6 Sol with High reasoning effort. The report and changes were reviewed by @brandon-julio-t.
All reactions