fix(browser): reap orphaned Chrome processes after daemon exit on Windows - #43577
fix(browser): reap orphaned Chrome processes after daemon exit on Windows#43577bighamx wants to merge 2 commits into
Conversation
…dows On Windows, when an agent-browser daemon exits (gateway restart, crash), its Chromium child processes get reparented to Explorer.exe. The existing reaper (_reap_orphaned_browser_sessions) handles daemons that are still alive via taskkill /T /F, but cannot reach Chrome children whose daemon already exited — the tree-kill target is gone. Add _reap_orphaned_chrome_processes() as a second-pass reaper that: 1. Scans running chrome.exe processes via wmic 2. Matches those with --user-data-dir containing 'agent-browser-chrome-*' 3. Only targets main headless instances (--headless, no --type=) 4. Skips when _active_sessions is non-empty (live sessions own Chrome) 5. Kills via ProcessRegistry._terminate_host_pid (taskkill /T /F) Called from _browser_cleanup_thread_worker on startup, after the existing daemon reaper runs. Fixes NousResearch#32047
teknium1
left a comment
There was a problem hiding this comment.
Thanks for pursuing the dead-daemon Chrome leak; current main still has the underlying gap: tools/browser_tool.py:1720 deletes a socket directory when its daemon PID is dead without cleaning reparented Chromium.
Problems
tools/browser_tool.py:1366in02edbb497a21tree-kills every matching headless Chrome process when this process has no active sessions. The WMIC query requestsParentProcessIdbut never uses it, so the code cannot distinguish an orphan from a live matching Chrome owned by another Hermes process. This conflicts with current main's fail-closed ownership protection intools/browser_tool.py:1543-1626(added by624580e8363f).tools/browser_tool.py:1344parses CSV usingsplit(',', 3).CommandLineis a quoted CSV field and may contain commas; usecsv.readerand test that case.
Suggested changes
- Positively verify reparented/orphan ownership using the returned parent information before calling
_terminate_host_pid, with a test covering a concurrent live session. - Parse the WMIC response as CSV and make parser/selection tests runnable outside Windows through mocks or a pure helper.
Automated hermes-sweeper review.
| from tools.process_registry import ProcessRegistry | ||
| for pid in main_pids: | ||
| try: | ||
| ProcessRegistry._terminate_host_pid(pid) |
There was a problem hiding this comment.
ParentProcessId is requested but never used. With only this process's _active_sessions guard, a second Hermes process can own a live matching Chrome tree while this process has no sessions, and this tree-kill will terminate it. Positively verify the reparented/orphan relationship before killing.
There was a problem hiding this comment.
Fixed in c78a324. Selection now uses the returned ParentProcessId and fails closed: a matching Chrome process is only reaped when its recorded parent is confirmed dead. Added a cross-platform test proving a live parent from a concurrent Hermes session is skipped.
| line = line.strip() | ||
| if not line or line.startswith("Node"): | ||
| continue | ||
| parts = line.split(",", 3) |
There was a problem hiding this comment.
This is WMIC CSV output, but CommandLine is a quoted free-form field and may contain commas. Parse with csv.reader; otherwise a comma shifts parts[3] and silently prevents correct PID selection.
There was a problem hiding this comment.
Fixed in c78a324. WMIC output is now parsed with csv.reader in a pure selection helper, with a cross-platform test covering a quoted CommandLine field containing a comma.
Require a dead recorded parent before selecting an agent-browser Chrome process, and parse WMIC output as real CSV so concurrent sessions and quoted commas remain safe. Co-authored-by: Cursor <cursoragent@cursor.com>
…platform) When the agent-browser daemon dies without shutting Chromium down (OOM kill / crash / SIGKILL — the common failure mode on memory-starved hosts), the browser is cut loose and keeps running: on POSIX it reparents to init, on Windows its PPID is invalidated. Reproduced live on Linux — SIGKILL the daemon after a successful navigate and the full 12-process Chromium tree survives session cleanup indefinitely. Every existing cleanup path goes blind in that state: * _cleanup_single_browser_session kills via the daemon PID's child tree — _terminate_host_pid returns silently when that PID is already dead — then removes the socket dir, destroying the only pid breadcrumb. * _reap_orphaned_browser_sessions only globs daemon socket dirs (agent-browser-h_*/cdp_*/hermes_*), never the browser's agent-browser-chrome-* user-data dir. On a long-running gateway this accumulates orphaned Chromium processes: 7 chrome processes surviving 6 days on a 2 GB Ubuntu VPS (the NousResearch#32047 symptom; also the macOS case in NousResearch#17388, closed not-planned). Relationship to NousResearch#43577: that open PR reaps the same orphans but is gated Windows-only (`if os.name != "nt": return 0`) on the assumption that "POSIX SIGTERM cascades reliably." The live repro above disproves that assumption — the cascade needs a live process to walk the tree, which an abnormally-killed daemon cannot do (POSIX does not signal children when a parent dies). This change detects orphans cross-platform via psutil (already a hard dependency) instead of WMIC, and reaps via the existing ProcessRegistry._terminate_host_pid (taskkill /T /F on Windows, psutil tree-walk on POSIX), so a single reaper needs no per-OS branch. The POSIX path is verified live; the Windows path is by construction only (cross-platform primitives + the existing taskkill path) and not yet verified on Windows. Fix: sweep for main Chromium processes whose cmdline carries an agent-browser-chrome-* --user-data-dir but whose parent is no longer a live agent-browser daemon; tree-kill them and drop the stale profile dir. The sweep runs on every cleanup-thread tick and from the startup/atexit orphan reap — including when no socket dirs remain, which is exactly the orphan's state — so a long session self-heals instead of leaking until restart. Daemon-owned browsers and foreign Chrome installs are untouched (covered by unit tests). Refs: NousResearch#17388, NousResearch#32047, NousResearch#43577
Problem
On Windows, when an agent-browser daemon exits (gateway restart, crash, SIGKILL), its Chromium child processes get reparented to Explorer.exe and live forever. The existing reaper (
_reap_orphaned_browser_sessions) handles daemons that are still alive viataskkill /T /F, but cannot reach Chrome children whose daemon already exited — the tree-kill target is gone.Evidence: Issue #32047 — 200+ orphaned Chrome processes after a few browser tasks. In the reporter's instance, 6 headless Chrome instances (each with crashpad/gpu/network/storage/renderer children = ~48 processes) had ParentProcessId = Explorer.exe, not the original agent-browser daemon.
PR #31231 added
taskkill /T /Ftree-kill for the daemon reaper, which works when the daemon is still alive. But the reparenting race means the daemon can exit before the reaper runs, leaving Chrome orphans.Fix
Add
_reap_orphaned_chrome_processes()as a second-pass reaper that runs after the existing daemon reaper on startup:chrome.exeprocesses viawmic--user-data-dircontainingagent-browser-chrome-*(the temp profile pattern agent-browser creates)--headlessin cmdline, no--type=) — child processes (renderer, GPU, etc.) are killed bytaskkill /T /Fcascade_active_sessionsis non-empty — if this process has live browser sessions, those Chrome instances belong to it and must not be touchedProcessRegistry._terminate_host_pid(sametaskkill /T /Fas the daemon reaper)Why
wmicinstead of scanning temp dirs?The
agent-browser-chrome-*user-data-dirs don't contain PID files — they're created by Playwright internally. We can't match them to socket dirs by UUID (different naming). Scanning running processes viawmicand matching the command-line--user-data-diris the only reliable way to identify which Chrome instances are orphaned.Files Changed
tools/browser_tool.py— added_reap_orphaned_chrome_processes()and call site in_browser_cleanup_thread_workertests/tools/test_browser_orphan_reaper.py— 5 new tests (Windows-only + POSIX no-op)Testing
Related