fix(web_server): reap zombie action subprocesses + preserve exit code - #41709
Closed
teknium1 wants to merge 2 commits into
Closed
fix(web_server): reap zombie action subprocesses + preserve exit code#41709teknium1 wants to merge 2 commits into
teknium1 wants to merge 2 commits into
Conversation
…accumulation get_action_status() calls proc.poll() to check if a dashboard action has finished, but never calls proc.wait() afterward. On POSIX systems the kernel retains the process table entry until a blocking waitpid() is issued, so every completed action remains as a zombie for the lifetime of the web server. After poll() returns a non-None exit code, call proc.wait(timeout=1) to reap the child and remove the handle from _ACTION_PROCS. Fixes #38032
Follow-up on the zombie-reap fix: once the Popen handle is reaped and
dropped from _ACTION_PROCS, migrate the exit code/pid into _ACTION_RESULTS
so subsequent /api/actions/{name}/status polls keep reporting the real
result instead of falling back to None. The dashboard polls repeatedly,
so without this the status flips from 'exited N' to 'unknown' on the
next poll.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-assignment |
1 |
First entries
tests/hermes_cli/test_web_server.py:845: [invalid-assignment] invalid-assignment: Invalid subscript assignment with key of type `Literal["gateway-restart"]` and value of type `FinishedProc` on object of type `dict[str, Popen[Unknown]]`
✅ Fixed issues: none
Unchanged: 5201 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Contributor
Author
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
Dashboard action subprocesses (
hermes update,skills-update,curator-run,dump, etc.) no longer accumulate as zombies. Salvages #38045 (@liuhao1024) onto currentmainwith a small follow-up.Root cause:
_spawn_hermes_action()stored each detached child'sPopenin_ACTION_PROCSandget_action_status()only ever calledproc.poll()— never.wait(). A finished child stayed a zombie in the process table until the dashboard server exited (and the stale handle lingered in_ACTION_PROCS).Changes
hermes_cli/web_server.py: onpoll()detecting exit, reap viaproc.wait(timeout=1)and drop the handle from_ACTION_PROCS(@liuhao1024's fix).hermes_cli/web_server.py: follow-up — migrate the reaped exit code/pid into_ACTION_RESULTSso subsequent status polls keep reporting the real result instead of falling back toNone(the dashboard polls repeatedly).tests/hermes_cli/test_web_server.py: reap-and-remove test (@liuhao1024) + second-poll consistency assertion.Validation
_ACTION_PROCSafter exitexit_code: null, pid: nullTargeted suite:
tests/hermes_cli/test_web_server.py -k "reaped or update_hermes or action_status or get_status"→ 6 passed.E2E: spawned a real detached child, ran the reap logic, confirmed
/proc/<pid>has no zombie entry afterward.Closes #38032. Supersedes #38040 and #38049 (both bundled an unrelated
meet_bot pcm_pumpchange — out of scope for this issue).Infographic