From 23d0dca8324e9ae7dbf00f4b3821de566505a56b Mon Sep 17 00:00:00 2001 From: yuexiong Date: Sat, 25 Jul 2026 18:13:37 +0800 Subject: [PATCH] fix(install): reap Windows cua installer process tree --- hermes_cli/tools_config.py | 43 +++++++++++++- tests/hermes_cli/test_install_cua_driver.py | 63 +++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index c0737971062..6139f7e0aca 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -1150,7 +1150,48 @@ def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) - if not is_windows: os.killpg(os.getpgid(proc.pid), _signal.SIGKILL) # windows-footgun: ok — POSIX branch only else: - proc.kill() + # PowerShell may leave download/install helpers alive after its + # direct process is killed. Those descendants inherit stdout + # and can keep both communicate() and install.lock wedged, so + # collect the tree first and kill it leaf-up. + import psutil as _psutil + + try: + parent = _psutil.Process(proc.pid) + descendants = parent.children(recursive=True) + except _psutil.NoSuchProcess: + return + except _psutil.Error as e: + logger.debug( + "could not enumerate cua-driver installer tree for pid %s: %s", + proc.pid, + e, + ) + proc.kill() + return + + for child in reversed(descendants): + try: + child.kill() + except _psutil.NoSuchProcess: + pass + except _psutil.Error as e: + logger.debug( + "could not kill cua-driver installer child pid %s: %s", + child.pid, + e, + ) + try: + parent.kill() + except _psutil.NoSuchProcess: + pass + except _psutil.Error as e: + logger.debug( + "could not kill cua-driver installer parent pid %s: %s", + proc.pid, + e, + ) + proc.kill() except (OSError, ProcessLookupError): proc.kill() diff --git a/tests/hermes_cli/test_install_cua_driver.py b/tests/hermes_cli/test_install_cua_driver.py index 59fceb1b91a..73b95b2daac 100644 --- a/tests/hermes_cli/test_install_cua_driver.py +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -464,6 +464,69 @@ class TestInstallerTimeoutKillsProcessGroup: assert captured.get("start_new_session") is True + def test_windows_timeout_kills_descendants_and_parent(self): + import subprocess + from unittest.mock import MagicMock + from hermes_cli import tools_config + + child = MagicMock() + parent = MagicMock() + parent.children.return_value = [child] + + fake_proc = MagicMock() + fake_proc.pid = 12345 + fake_proc.communicate.side_effect = [ + subprocess.TimeoutExpired(cmd="powershell", timeout=1), + ("", None), + ] + + with patch("platform.system", return_value="Windows"), \ + patch("subprocess.Popen", return_value=fake_proc), \ + patch("psutil.Process", return_value=parent), \ + patch.object(tools_config, "_clear_stale_cua_install_lock"), \ + patch.object(tools_config, "_print_warning"), \ + patch.object(tools_config, "_print_info"): + ok = tools_config._run_cua_driver_installer( + label="Refreshing", verbose=False + ) + + assert ok is False + parent.children.assert_called_once_with(recursive=True) + child.kill.assert_called_once_with() + parent.kill.assert_called_once_with() + fake_proc.kill.assert_not_called() + assert fake_proc.communicate.call_count == 2 + + def test_windows_tree_enumeration_failure_falls_back_to_direct_kill(self): + import psutil + import subprocess + from unittest.mock import MagicMock + from hermes_cli import tools_config + + parent = MagicMock() + parent.children.side_effect = psutil.AccessDenied(pid=12345) + + fake_proc = MagicMock() + fake_proc.pid = 12345 + fake_proc.communicate.side_effect = [ + subprocess.TimeoutExpired(cmd="powershell", timeout=1), + ("", None), + ] + + with patch("platform.system", return_value="Windows"), \ + patch("subprocess.Popen", return_value=fake_proc), \ + patch("psutil.Process", return_value=parent), \ + patch.object(tools_config, "_clear_stale_cua_install_lock"), \ + patch.object(tools_config, "_print_warning"), \ + patch.object(tools_config, "_print_info"): + ok = tools_config._run_cua_driver_installer( + label="Refreshing", verbose=False + ) + + assert ok is False + fake_proc.kill.assert_called_once_with() + assert fake_proc.communicate.call_count == 2 + class TestInstallerNoShell: """The POSIX installer path must not use shell=True or command