fix(gateway): verify process is alive after restart before reporting success - #325
Open
sridhar-3009 wants to merge 1 commit into
Open
fix(gateway): verify process is alive after restart before reporting success#325sridhar-3009 wants to merge 1 commit into
sridhar-3009 wants to merge 1 commit into
Conversation
…success gateway_restart_cmd (and the config-change restart helper) called start_gateway_process and immediately printed "restarted (pid=N)" with no check that the subprocess actually stayed running. If the gateway crashed on startup (e.g. bad config, missing dependency), the command silently reported success while the service stayed down, requiring a manual container restart to recover. Add a short polling loop in start_gateway_process that checks _pid_is_running() up to ~1.5 s after spawn. If the process has already exited by then, raise RuntimeError with the PID and log path so the caller can surface a clear error. Update gateway_restart_cmd and _maybe_restart_gateway to catch this error and report it rather than pretending the restart succeeded. Fixes HKUDS#301
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
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
Closes #301
ohmo gateway restartcalledstart_gateway_process()and immediatelyprinted
"restarted (pid=N)"— no verification that the subprocess wasstill alive. If the gateway crashed on startup (bad config, port
conflict, missing dependency), the command silently reported success
while the service stayed down, requiring a manual container restart.
Root cause in
start_gateway_process:stdout/stderrare redirected togateway.logso no outputappears in the calling terminal.
subprocess.Popenreturns a handle instantly; the PID is returnedimmediately without checking whether the process survived.
Fix: After spawning, poll
_pid_is_running(process.pid)for up to~1.5 s in increasing intervals (
0.1 s → 0.2 s → 0.4 s → 0.8 s).This gives the process enough time to initialise before declaring it
healthy, while still catching immediate crashes. If no poll succeeds,
raise
RuntimeErrorwith the PID and log file path.Update
gateway_restart_cmdand_maybe_restart_gatewayto catch thaterror and surface a clear message (non-zero exit for the CLI command).
Test plan
ohmo gateway restartstill printsrestarted (pid=N)and the service is up.restartnow prints an error pointing to the log file instead of falsely reporting success.gateway start(not modified) still works unchanged.