fix: hermes update kills freshly-restarted gateway service - #5409
Closed
kshitijk4poor wants to merge 2 commits into
Closed
fix: hermes update kills freshly-restarted gateway service#5409kshitijk4poor wants to merge 2 commits into
kshitijk4poor wants to merge 2 commits into
Conversation
After restarting a service-managed gateway (systemd/launchd), the stale-process sweep calls find_gateway_pids() which returns ALL gateway PIDs via ps aux — including the one just spawned by the service manager. The sweep kills it, leaving the user with a stopped gateway and a confusing 'Restart manually' message. Fix: add _get_service_pids() to query systemd MainPID and launchd PID for active gateway services, then exclude those PIDs from the sweep. Also add exclude_pids parameter to find_gateway_pids() and kill_gateway_processes() so callers can skip known service-managed PIDs. Adds 9 targeted tests covering: - _get_service_pids() for systemd, launchd, empty, and zero-PID cases - find_gateway_pids() exclude_pids filtering - cmd_update integration: service PID not killed after restart - cmd_update integration: manual PID killed while service PID preserved
…f-import, harden launchd parsing - Loop over user/system scope args instead of duplicating the systemd block - Call get_launchd_label() directly instead of self-importing from hermes_cli.gateway - Validate launchd output by checking parts[2] matches expected label (skip header) - Add race-condition assumption docstring
Contributor
|
Merged via PR #5448. Your commits were cherry-picked onto current main with your authorship preserved in git log. Thanks for the fix, @kshitijk4poor! |
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.
Problem
After every
hermes update, the gateway service restarts and immediately stops:The update path restarts the service via systemd/launchd, then sweeps for stale manual gateway processes using
find_gateway_pids()which scansps auxand returns ALL matching PIDs — including the one just spawned by the service manager. The sweep kills it immediately.Introduced in 84a875c — the commit correctly added profile-scoped stop, but the
find_gateway_pids()sweep in the update path has no service-PID exclusion.Fix
_get_service_pids()helper that queries systemdMainPIDand launchd PID for active gateway servicesexclude_pidsparameter tofind_gateway_pids()andkill_gateway_processes()_get_service_pids()and pass the result tofind_gateway_pids(exclude_pids=...)so freshly-restarted service PIDs are skippedTests
9 new tests:
TestGetServicePids: systemd MainPID, launchd PID, empty, zero-PID exclusionTestFindGatewayPidsExclude: exclude_pids filteringTestServicePidExclusion: integration tests proving service PID survives after launchd/systemd restart, manual PIDs still killedNote:
TestCmdUpdateLaunchdRestart::test_update_with_systemd_still_restarts_via_systemdwas already failing on current main (pre-existing).