fix(browser): use process-tree termination for daemon cleanup#29343
fix(browser): use process-tree termination for daemon cleanup#29343dskwe wants to merge 1 commit into
Conversation
1de07aa to
2c3ca70
Compare
|
Verified on Linux (Docker): Simulated a daemon with 3 child processes. After os.kill(parent, SIGTERM) (old code): 3 orphaned children confirmed. New code (_terminate_host_pid) terminates children before parent, fixing the leak. All tests in test_browser_orphan_reaper.py pass. Note: CI failures are pre-existing on main branch, not introduced by this PR. |
2c3ca70 to
488b95c
Compare
os.kill(pid, SIGTERM) only signals the parent, leaving Chromium child
processes (renderer, GPU, etc.) orphaned. Reuse the existing
ProcessRegistry._terminate_host_pid() helper which walks the process
tree leaf-up via psutil, terminating children before the parent.
488b95c to
ea8fbd9
Compare
|
Merged via PR #31231 (#31231). Your commit was cherry-picked onto current main with your authorship preserved in the git log (commit 571a0dc). The salvage PR also extended the fix to Windows: the existing |
Problem
Browser daemon cleanup was leaving orphaned Chromium processes (renderer, GPU, etc.) because we were just calling
os.kill(pid, SIGTERM)on the parent. This leaves all the child processes running as orphans, consuming resources over time.Solution
Switch to
ProcessRegistry._terminate_host_pid()which walks the process tree withpsutiland terminates children before the parent.Changes
tools/browser_tool.py: Replaceos.kill()calls withProcessRegistry._terminate_host_pid()in_reap_orphaned_browser_sessions()and_cleanup_single_browser_session()tests/tools/test_browser_orphan_reaper.py: Update all 7 related test mocksHow to test
Or manually verify that browser sessions are cleaned up completely by checking for orphaned Chromium processes after cleanup.
Reuses existing
ProcessRegistry._terminate_host_pid()utility which is already well-tested.