fix(dashboard): recycle console executor when all workers are permanently stuck - #59240
Open
sprmn24 wants to merge 1 commit into
Open
fix(dashboard): recycle console executor when all workers are permanently stuck#59240sprmn24 wants to merge 1 commit into
sprmn24 wants to merge 1 commit into
Conversation
…ntly stuck The console command thread pool (_CONSOLE_EXECUTOR_MAX_WORKERS=4) could become permanently wedged: asyncio.wait_for abandons the await on timeout but cannot kill the Python thread, so a truly stuck worker is gone from the pool forever. After 4 such hangs, every subsequent console command for every dashboard session queues behind threads that will never return, hanging the entire dashboard console until process restart. Fix: - Add _new_console_executor() factory (moves atexit registration there) - Track _console_executor_stuck_count under _console_executor_stuck_lock - Add _note_console_submission_outcome(stuck=True/False): increments on timeout, decrements if the thread eventually returns (merely slow vs truly stuck), and atomically swaps in a fresh pool when all workers are wedged via shutdown(wait=False, cancel_futures=True) on the stale one - Switch run_command from loop.run_in_executor to executor.submit + asyncio.wrap_future so a done-callback fires even after wait_for gives up, letting slow-but-not-stuck submissions decrement the counter
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing a real dashboard-console availability failure: current main still submits console commands to a fixed four-worker pool and only times out the await (hermes_cli/web_server.py:15402-15413).
Problems
- The done callback at
hermes_cli/web_server.py:13308decrements the global count for every non-cancelled future, not just a future that previously timed out. A normal successful command can therefore erase the count for a different permanently blocked worker. _console_executor_stuck_counthas no executor-generation ownership (hermes_cli/web_server.py:13017). A late completion from a retired executor can decrement timeout state accumulated by the replacement executor.- The diff adds no timeout/replacement regression test; existing console WS tests cover normal execution and cancellation only (
tests/hermes_cli/test_web_server_console_ws.py:73-134).
Suggested changes
- Associate timeout bookkeeping with each future and executor generation; only a timed-out future's completion may clear its own mark.
- Add deterministic tests for four blocked workers, replacement, and late completion from the old pool.
Automated hermes-sweeper review.
| confirmed=confirmed, | ||
| profile=profile, | ||
| ) | ||
| cf_future.add_done_callback( |
Contributor
There was a problem hiding this comment.
This callback runs for every non-cancelled future, including commands that finished before the timeout. If one worker has timed out, a later normal command decrements its global count and can prevent the pool from recycling after all four workers are actually stuck. Track timeout state on this specific future (and its executor generation) before clearing it.
19 tasks
Open
1 task
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.
What does this PR do?
Fixes a dashboard console denial-of-service: after 4 stuck console command threads, every subsequent command for every session hangs until process restart.
Type of Change
Changes Made
The console command thread pool (
_CONSOLE_EXECUTOR_MAX_WORKERS=4) could become permanently wedged.asyncio.wait_forabandons the await on timeout but cannot kill the Python thread, so a truly stuck worker is gone from the pool forever. After 4 such hangs from any session, every subsequentrun_in_executorsubmission queues behind threads that will never return — hanging the entire dashboard console until process restart.Fix:
_new_console_executor()factory (moves atexit registration there)_console_executor_stuck_countunder_console_executor_stuck_lock_note_console_submission_outcome(stuck=True/False): increments on timeout, decrements if the thread eventually returns (merely slow vs truly stuck), and atomically swaps in a fresh pool when all workers are wedgedrun_commandfromloop.run_in_executortoexecutor.submit+asyncio.wrap_futureso a done-callback fires even afterwait_forgives up, letting slow-but-not-stuck submissions decrement the counterHow to Test
Send 4 console commands that each hang longer than 60s. Before: the 5th command from any session hangs forever. After: the pool is automatically replaced and the 5th command executes normally.
Checklist