Fix Windows terminal Ctrl-C process cleanup (#3171)

Co-authored-by: openhands <openhands@all-hands.dev>

Author: neubig

openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py

@@ -32,6 +32,7 @@
 _SETUP_DELAY_SECONDS = 0.5
 _SETUP_POLL_INTERVAL_SECONDS = 0.05
 _MAX_SETUP_WAIT_SECONDS = 2.0
+_INTERRUPT_GRACE_SECONDS = 0.5
 
 _WINDOWS_SPECIALS: dict[str, str] = {
     "ENTER": "\n",
@@ -172,6 +173,7 @@ def close(self) -> None:
             return
 
         self._stop_reader.set()
+        self._terminate_child_processes()
 
         if self.process is not None:
             try:
@@ -327,6 +329,62 @@ def clear_screen(self) -> None:
         time.sleep(_SCREEN_CLEAR_DELAY_SECONDS)
         self._command_running_event.clear()
 
+    def _terminate_child_processes(self) -> bool:
+        """Terminate descendants of the persistent PowerShell process."""
+        if (
+            platform.system() != "Windows"
+            or self.process is None
+            or self.process.poll() is not None
+        ):
+            return False
+
+        script = f"""
+$root = {self.process.pid}
+$childrenByParent = @{{}}
+Get-CimInstance Win32_Process | ForEach-Object {{
+    $parentId = [int]$_.ParentProcessId
+    if (-not $childrenByParent.ContainsKey($parentId)) {{
+        $childrenByParent[$parentId] = New-Object System.Collections.Generic.List[int]
+    }}
+    $childrenByParent[$parentId].Add([int]$_.ProcessId)
+}}
+$toStop = New-Object System.Collections.Generic.List[int]
+function Add-Descendants([int]$processId) {{
+    if (-not $childrenByParent.ContainsKey($processId)) {{ return }}
+    foreach ($childId in $childrenByParent[$processId]) {{
+        if ($childId -eq $PID) {{ continue }}
+        $toStop.Add($childId)
+        Add-Descendants $childId
+    }}
+}}
+Add-Descendants $root
+for ($i = $toStop.Count - 1; $i -ge 0; $i--) {{
+    Stop-Process -Id $toStop[$i] -Force -ErrorAction SilentlyContinue
+}}
+if ($toStop.Count -gt 0) {{ exit 0 }} else {{ exit 1 }}
+"""
+        startupinfo = None
+        startupinfo_cls = getattr(subprocess, "STARTUPINFO", None)
+        if startupinfo_cls is not None:
+            startupinfo = startupinfo_cls()
+            startupinfo.dwFlags |= getattr(subprocess, "STARTF_USESHOWWINDOW", 0)
+        creationflags = getattr(subprocess, "CREATE_NO_WINDOW", 0)
+
+        try:
+            result = subprocess.run(
+                [self.shell_path, "-NoLogo", "-NoProfile", "-Command", script],
+                stdout=subprocess.DEVNULL,
+                stderr=subprocess.DEVNULL,
+                check=False,
+                timeout=5.0,
+                startupinfo=startupinfo,
+                creationflags=creationflags,
+            )
+            return result.returncode == 0
+        except (subprocess.TimeoutExpired, OSError) as exc:
+            logger.debug("Failed to terminate PowerShell child processes: %s", exc)
+            return False
+
     def interrupt(self) -> bool:
         """Interrupt the active command if the process is still alive."""
         if self.process is None or self.process.poll() is not None:
@@ -341,15 +399,21 @@ def interrupt(self) -> bool:
             except Exception as exc:
                 logger.debug("Failed to send CTRL_BREAK_EVENT: %s", exc)
 
-        if not sent_ctrl_break:
+        if sent_ctrl_break:
+            time.sleep(_INTERRUPT_GRACE_SECONDS)
+
+        terminated_children = self._terminate_child_processes()
+        sent_ctrl_c_input = False
+        if not sent_ctrl_break and not terminated_children:
             try:
                 self._write_to_stdin(_WINDOWS_SPECIALS["C-C"])
+                sent_ctrl_c_input = True
             except RuntimeError as exc:
                 logger.debug("Failed to write Ctrl+C to PowerShell stdin: %s", exc)
                 return False
 
         self._command_running_event.clear()
-        return True
+        return sent_ctrl_break or terminated_children or sent_ctrl_c_input
 
     def is_running(self) -> bool:
         """Return whether a command is still running in the PowerShell session."""

tests/tools/browser_use/test_browser_toolset.py

@@ -31,7 +31,8 @@ def _mock_browser_executor_init():
     def fake_init(self, **_kwargs):
         self.full_output_save_dir = None
         self._initialized = False
-        self._cleanup_initiated = False
+        # Toolset tests never allocate browser resources; keep close() a no-op.
+        self._cleanup_initiated = True
         self._action_timeout_seconds = 30.0
         self._async_executor = MagicMock()
         self._async_executor.close = MagicMock()

tests/tools/terminal/test_windows_ctrl_c.py

@@ -0,0 +1,106 @@
+"""Windows-specific terminal interrupt behavior tests."""
+
+import platform
+import subprocess
+
+import pytest
+
+from openhands.tools.terminal.definition import TerminalAction
+from openhands.tools.terminal.terminal import create_terminal_session
+from openhands.tools.terminal.terminal.terminal_session import TerminalCommandStatus
+
+
+pytestmark = pytest.mark.skipif(
+    platform.system() != "Windows",
+    reason="Windows CTRL_BREAK/PowerShell process behavior only applies on Windows",
+)
+
+
+def _powershell_process_exists(pid: int) -> bool:
+    result = subprocess.run(
+        [
+            "powershell.exe",
+            "-NoLogo",
+            "-NoProfile",
+            "-Command",
+            (
+                f"if (Get-Process -Id {pid} -ErrorAction SilentlyContinue) "
+                "{ exit 0 } else { exit 1 }"
+            ),
+        ],
+        stdout=subprocess.DEVNULL,
+        stderr=subprocess.DEVNULL,
+        check=False,
+    )
+    return result.returncode == 0
+
+
+def _stop_powershell_process(pid: int) -> None:
+    subprocess.run(
+        [
+            "powershell.exe",
+            "-NoLogo",
+            "-NoProfile",
+            "-Command",
+            f"Stop-Process -Id {pid} -Force -ErrorAction SilentlyContinue",
+        ],
+        stdout=subprocess.DEVNULL,
+        stderr=subprocess.DEVNULL,
+        check=False,
+    )
+
+
+@pytest.mark.timeout(20)
+def test_windows_ctrl_c_interrupt_kills_child_process_tree(tmp_path) -> None:
+    """Ctrl-C after a timeout should stop the process that kept the command alive.
+
+    This captures the behavior promised by the timeout prompt. The current
+    PowerShell backend sends CTRL_BREAK to the persistent PowerShell process, but
+    does not ensure child processes launched by the command are terminated.
+    """
+    pid_path = tmp_path / "child.pid"
+    script_path = tmp_path / "wait_on_child.ps1"
+    script_path.write_text(
+        "\n".join(
+            [
+                f"$pidPath = '{pid_path.as_posix()}'",
+                "$child = Start-Process -FilePath powershell.exe "
+                "-ArgumentList '-NoLogo','-NoProfile','-Command',"
+                "'Start-Sleep -Seconds 120' -PassThru",
+                "Set-Content -LiteralPath $pidPath -Value $child.Id",
+                "Wait-Process -Id $child.Id",
+            ]
+        ),
+        encoding="utf-8",
+    )
+
+    session = create_terminal_session(
+        work_dir=str(tmp_path),
+        terminal_type="powershell",
+        no_change_timeout_seconds=1,
+    )
+    child_pid: int | None = None
+    child_was_still_running = False
+    try:
+        session.initialize()
+
+        obs = session.execute(TerminalAction(command=f"& '{script_path.as_posix()}'"))
+
+        assert obs.metadata.exit_code == -1
+        assert session.prev_status == TerminalCommandStatus.NO_CHANGE_TIMEOUT
+        assert pid_path.exists()
+        child_pid = int(pid_path.read_text(encoding="utf-8").strip())
+        assert _powershell_process_exists(child_pid)
+
+        session.execute(TerminalAction(command="C-c", is_input=True, timeout=3))
+
+        child_was_still_running = _powershell_process_exists(child_pid)
+    finally:
+        if child_pid is not None:
+            _stop_powershell_process(child_pid)
+        session.close()
+
+    assert not child_was_still_running, (
+        "Windows Ctrl-C reported through the terminal did not terminate the "
+        "child process that kept the timed-out command alive."
+    )