From 4e49af94bed94d85d99dc02a76642ca12b59f299 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 03:38:23 -0500 Subject: [PATCH] fix(computer_use): stop the cua-driver child on exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CuaDriverBackend caches a long-lived cua-driver subprocess for the life of the Hermes process, and stop() was never called from anywhere — the driver outlived the session that spawned it. #69903 stopped the orphan from pegging a core by disabling the cursor overlay, but left the process behind; this is item 3 of #28152 ("Hermes does not keep the driver alive after tool completion"). Register an atexit hook, mirroring browser_tool's atexit.register(_emergency_cleanup_all_sessions). atexit only, no signal handlers, for the prompt_toolkit reason documented there. reset_backend_for_tests now reuses the same teardown instead of repeating it. --- .../computer_use/test_cua_atexit_teardown.py | 75 +++++++++++++++++++ tools/computer_use/tool.py | 40 ++++++++-- 2 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 tests/computer_use/test_cua_atexit_teardown.py diff --git a/tests/computer_use/test_cua_atexit_teardown.py b/tests/computer_use/test_cua_atexit_teardown.py new file mode 100644 index 000000000000..ff02d94fc5ad --- /dev/null +++ b/tests/computer_use/test_cua_atexit_teardown.py @@ -0,0 +1,75 @@ +"""Tests for cua-driver subprocess teardown on interpreter exit. + +``CuaDriverBackend`` spawns a long-lived ``cua-driver`` child process and +caches it for the life of the Hermes process. Nothing ever tore it down, so +the driver outlived the session that started it (#28152 item 3 — "Hermes does +not keep the driver alive after tool completion"). #69903 fixed the *overlay* +redraw loop that made the lingering process burn CPU, but not the lingering +process itself. + +``tools/computer_use/tool.py`` registers an ``atexit`` hook that stops the +cached backend, mirroring ``browser_tool``'s +``atexit.register(_emergency_cleanup_all_sessions)``. + +These assert the behavior contract — the hook is registered, it stops a live +backend, it is a no-op when nothing was ever started, and it never raises out +of ``atexit`` — not a snapshot of the module's source. +""" + +import atexit +from unittest.mock import MagicMock, patch + +from tools.computer_use import tool as cu_tool + + +class TestAtexitTeardown: + def test_shutdown_stops_a_live_backend(self): + """A cached backend is stopped when the interpreter exits.""" + fake = MagicMock() + with patch.object(cu_tool, "_backend", fake): + cu_tool._shutdown_backend_atexit() + fake.stop.assert_called_once() + + def test_shutdown_clears_the_cached_backend(self): + """After teardown the module cache is empty, so a later call re-spawns.""" + fake = MagicMock() + with patch.object(cu_tool, "_backend", fake): + cu_tool._shutdown_backend_atexit() + assert cu_tool._backend is None + + def test_shutdown_is_a_noop_when_never_started(self): + """No backend was ever created => nothing to stop, no error.""" + with patch.object(cu_tool, "_backend", None): + cu_tool._shutdown_backend_atexit() # must not raise + assert cu_tool._backend is None + + def test_shutdown_swallows_backend_errors(self): + """A failing stop() must not raise out of an atexit hook. + + Exceptions escaping atexit print a traceback on every exit and can + mask the real exit status. + """ + fake = MagicMock() + fake.stop.side_effect = RuntimeError("driver already dead") + with patch.object(cu_tool, "_backend", fake): + cu_tool._shutdown_backend_atexit() # must not raise + assert cu_tool._backend is None + + def test_hook_is_registered_with_atexit(self): + """Importing the tool module registers the teardown hook. + + Verified by unregistering and re-registering: atexit.unregister only + removes a function that was actually registered, so a successful + round-trip proves the import-time registration happened. + """ + atexit.unregister(cu_tool._shutdown_backend_atexit) + try: + with patch.object(cu_tool, "_backend", MagicMock()) as fake: + # Re-register and fire the full atexit chain the way the + # interpreter would, then confirm our hook ran. + atexit.register(cu_tool._shutdown_backend_atexit) + atexit._run_exitfuncs() + fake.stop.assert_called_once() + finally: + atexit.unregister(cu_tool._shutdown_backend_atexit) + atexit.register(cu_tool._shutdown_backend_atexit) diff --git a/tools/computer_use/tool.py b/tools/computer_use/tool.py index 1cda6a78fe91..58117ea90568 100644 --- a/tools/computer_use/tool.py +++ b/tools/computer_use/tool.py @@ -38,6 +38,7 @@ For captures / actions with `capture_after=True`: from __future__ import annotations +import atexit import base64 import json import logging @@ -177,16 +178,39 @@ def _get_backend() -> ComputerUseBackend: return _backend +def _shutdown_backend_atexit() -> None: + """Stop the cached backend so the cua-driver child doesn't outlive us. + + The backend is cached per-process and holds a long-lived ``cua-driver`` + subprocess, so without this the driver survives the Hermes process that + spawned it (#28152 item 3). #69903 kept the orphan from burning a core by + disabling the cursor overlay; the process itself still lingered. + + Mirrors ``browser_tool``'s ``atexit.register(_emergency_cleanup_all_sessions)`` + — same spawn-and-drive-a-subprocess shape. atexit only, no signal handlers: + a ``SystemExit`` raised from a prompt_toolkit key binding corrupts its + coroutine state and makes the process unkillable. Never raises, since an + exception escaping atexit prints a traceback on every exit. + """ + global _backend + # Drop the lock before stop() — teardown budgets 5s and shouldn't block + # an unrelated caller waiting to spawn. + with _backend_lock: + backend, _backend = _backend, None + if backend is None: + return + try: + backend.stop() + except Exception as e: + logger.debug("cua-driver atexit teardown failed: %s", e) + + +atexit.register(_shutdown_backend_atexit) + + def reset_backend_for_tests() -> None: # pragma: no cover """Test helper — tear down the cached backend and per-session state.""" - global _backend - with _backend_lock: - if _backend is not None: - try: - _backend.stop() - except Exception: - pass - _backend = None + _shutdown_backend_atexit() _AUX_VISION_ROUTE_CACHE.clear() with _approval_lock: _session_auto_approve.clear()