fix(agent): reap browser/VM task resources on aborted turns - #39897
Closed
tim404x wants to merge 1 commit into
Closed
fix(agent): reap browser/VM task resources on aborted turns#39897tim404x wants to merge 1 commit into
tim404x wants to merge 1 commit into
Conversation
The gateway leaked memory slowly under real use: every conversation turn that aborted with an exception or was interrupted left its per-task resources behind. The agent-browser daemon and its Chromium/Xvfb process tree (plus any sandbox VM) would orphan to init and accumulate. Two root causes, both fixed: 1. run_conversation only cleaned up task resources on its normal return paths. On the exception / interrupt path it returned without reaping. This splits the function into a thin public run_conversation() wrapper and _run_conversation_impl(): the wrapper runs _cleanup_task_resources() in a finally that fires ONLY when the impl did not complete normally, so the success path is unchanged (no double cleanup, no timing change) while aborts are always reaped. Cleanup is idempotent and persistence-aware, so it is safe to run on abort. 2. _terminate_host_pid only sent SIGTERM to the process tree snapshot taken at call time. Processes that ignore SIGTERM (chrome_crashpad_handler, renderers under load) survived, and children reparented to init as their parents exited fell off children(recursive=True) entirely. It now snapshots the whole tree up front, SIGTERMs it (letting a live browser flush its already-persisted cookies), waits, then SIGKILLs anything still alive and reaps zombies. Tests: tests/tools/test_run_conversation_abort_cleanup.py covers the wrapper (normal return does not double-reap; exception and interrupt both reap with the correct task id; a generated id is reused for the reap; a failing reaper does not mask the original turn error). Regression-validated. The existing browser-orphan-reaper suite (18 tests) still passes.
tim404x
force-pushed
the
fix/browser-task-cleanup-leak
branch
from
June 5, 2026 16:35
3f63ae2 to
0fcd380
Compare
Contributor
|
Thanks for the focused resource-cleanup work. This is an automated hermes-sweeper review; the requested behavior is now implemented on current
|
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.
The gateway leaked memory slowly under real use: every conversation turn that aborted with an exception or was interrupted left its per-task resources behind. The agent-browser daemon and its Chromium/Xvfb process tree (plus any sandbox VM) would orphan to init and accumulate.
Two root causes, both fixed:
run_conversation only cleaned up task resources on its normal return paths. On the exception / interrupt path it returned without reaping. This splits the function into a thin public run_conversation() wrapper and _run_conversation_impl(): the wrapper runs _cleanup_task_resources() in a finally that fires ONLY when the impl did not complete normally, so the success path is unchanged (no double cleanup, no timing change) while aborts are always reaped. Cleanup is idempotent and persistence-aware, so it is safe to run on abort.
_terminate_host_pid only sent SIGTERM to the process tree snapshot taken at call time. Processes that ignore SIGTERM (chrome_crashpad_handler, renderers under load) survived, and children reparented to init as their parents exited fell off children(recursive=True) entirely. It now snapshots the whole tree up front, SIGTERMs it (letting a live browser flush its already-persisted cookies), waits, then SIGKILLs anything still alive and reaps zombies.
Tests: tests/tools/test_run_conversation_abort_cleanup.py covers the wrapper (normal return does not double-reap; exception and interrupt both reap with the correct task id; a generated id is reused for the reap; a failing reaper does not mask the original turn error). Regression-validated. The existing browser-orphan-reaper suite (18 tests) still passes.
What does this PR do?
Related Issue
Fixes #
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs