fix: reap timed-out execution-check process groups

clawbench/environment.py

@@ -5,11 +5,14 @@
 import asyncio
 import json
 import logging
+import os
 import re
+import sys
 from pathlib import Path
 from typing import Any
 
 from clawbench.client import GatewayClient
+from clawbench.environment_files import _execution_subprocess_kwargs, _reap_timed_out_process
 from clawbench.paths import resolve_workspace_path
 from clawbench.render import render_argv_template, render_shell_template, render_template, render_value
 from clawbench.schemas import (
@@ -128,8 +131,6 @@ async def run_execution_check(
             reason=str(exc),
         )
     rendered_env = render_value(spec.env, runtime_values)
-    import os
-    import sys
 
     full_env = {
         **os.environ,
@@ -145,13 +146,15 @@ async def run_execution_check(
     full_env["PYTHONPATH"] = ":".join(python_path_parts)
 
     try:
+        spawn_kwargs = _execution_subprocess_kwargs()
         if spec.shell:
             process = await asyncio.create_subprocess_shell(
                 rendered_command,
                 cwd=str(rendered_cwd),
                 env=full_env,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
+                **spawn_kwargs,
             )
         else:
             process = await asyncio.create_subprocess_exec(
@@ -160,14 +163,14 @@ async def run_execution_check(
                 env=full_env,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
+                **spawn_kwargs,
             )
         stdout_bytes, stderr_bytes = await asyncio.wait_for(
             process.communicate(),
             timeout=spec.timeout_seconds,
         )
     except asyncio.TimeoutError:
-        process.kill()
-        await process.communicate()
+        await _reap_timed_out_process(process, spec.timeout_seconds)
         return ExecutionCheckResult(
             name=spec.name,
             command=rendered_command,

clawbench/environment_files.py

@@ -19,6 +19,8 @@
 import logging
 import os
 import re
+import signal
+import subprocess
 import sys
 from pathlib import Path
 from typing import Any
@@ -92,6 +94,54 @@ def verify_file_state(
 # ---------------------------------------------------------------------------
 
 
+def _execution_subprocess_kwargs() -> dict[str, Any]:
+    if sys.platform == "win32":
+        return {}
+    return {"start_new_session": True}
+
+
+def _kill_execution_pgroup(process: asyncio.subprocess.Process) -> None:
+    """Signal the process group so shell-spawned children do not keep pipes open."""
+    if process.pid is None:
+        return
+    if sys.platform == "win32":
+        subprocess.run(
+            ["taskkill", "/F", "/T", "/PID", str(process.pid)],
+            check=False,
+            capture_output=True,
+        )
+        try:
+            process.kill()
+        except ProcessLookupError:
+            pass
+        return
+    try:
+        os.killpg(os.getpgid(process.pid), signal.SIGKILL)
+        return
+    except (ProcessLookupError, PermissionError, OSError):
+        pass
+    try:
+        process.kill()
+    except ProcessLookupError:
+        pass
+
+
+async def _reap_timed_out_process(
+    process: asyncio.subprocess.Process, timeout_seconds: float
+) -> None:
+    _kill_execution_pgroup(process)
+    try:
+        await asyncio.wait_for(
+            process.communicate(),
+            timeout=max(1.0, float(timeout_seconds)),
+        )
+    except (asyncio.TimeoutError, ProcessLookupError, OSError):
+        try:
+            process.kill()
+        except ProcessLookupError:
+            pass
+
+
 async def run_execution_check(
     spec: ExecutionCheck,
     *,
@@ -135,13 +185,15 @@ async def run_execution_check(
     full_env["PYTHONPATH"] = ":".join(python_path_parts)
 
     try:
+        spawn_kwargs = _execution_subprocess_kwargs()
         if spec.shell:
             process = await asyncio.create_subprocess_shell(
                 rendered_command,
                 cwd=str(rendered_cwd),
                 env=full_env,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
+                **spawn_kwargs,
             )
         else:
             process = await asyncio.create_subprocess_exec(
@@ -150,14 +202,14 @@ async def run_execution_check(
                 env=full_env,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
+                **spawn_kwargs,
             )
         stdout_bytes, stderr_bytes = await asyncio.wait_for(
             process.communicate(),
             timeout=spec.timeout_seconds,
         )
     except asyncio.TimeoutError:
-        process.kill()
-        await process.communicate()
+        await _reap_timed_out_process(process, spec.timeout_seconds)
         return ExecutionCheckResult(
             name=spec.name,
             command=rendered_command,

tests/test_execution_shell_rendering.py

@@ -1,3 +1,7 @@
+import asyncio
+import os
+import signal
+import subprocess
 import sys
 from pathlib import Path
 
@@ -131,3 +135,74 @@ async def test_shell_execution_check_preserves_single_quoted_placeholders(
     )
 
     assert result.passed is True
+
+
+def _write_pipe_holder(path: Path) -> None:
+    path.write_text(
+        "import subprocess\n"
+        "import sys\n"
+        "import time\n"
+        "from pathlib import Path\n"
+        "\n"
+        "child = subprocess.Popen(\n"
+        "    [sys.executable, '-c', 'import time; time.sleep(120)']\n"
+        ")\n"
+        "Path(sys.argv[1]).write_text(str(child.pid), encoding='utf-8')\n"
+        "time.sleep(120)\n",
+        encoding="utf-8",
+    )
+
+
+def _force_kill_pid(pid: int) -> None:
+    if sys.platform == "win32":
+        subprocess.run(
+            ["taskkill", "/F", "/T", "/PID", str(pid)],
+            check=False,
+            capture_output=True,
+        )
+        return
+    try:
+        os.kill(pid, signal.SIGKILL)
+    except OSError:
+        pass
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("run_execution_check", RUNNERS)
+async def test_execution_check_timeout_reaps_shell_child_process_group(
+    tmp_path: Path,
+    run_execution_check,
+):
+    _write_pipe_holder(tmp_path / "hold_pipe.py")
+    pid_file = tmp_path / "child.pid"
+    try:
+        try:
+            result = await asyncio.wait_for(
+                run_execution_check(
+                    ExecutionCheck(
+                        name="timeout-reap",
+                        command="python hold_pipe.py child.pid",
+                        timeout_seconds=1,
+                    ),
+                    workspace=tmp_path,
+                    runtime_values={},
+                ),
+                timeout=8,
+            )
+        except TimeoutError:
+            pytest.fail(
+                "run_execution_check hung after timeout_seconds; "
+                "child still held stdout/stderr"
+            )
+
+        assert result.passed is False
+        assert result.exit_code == -1
+        assert result.reason == "Timed out after 1s"
+        assert pid_file.exists()
+        child_pid = int(pid_file.read_text(encoding="utf-8").strip())
+        if sys.platform != "win32":
+            with pytest.raises(OSError):
+                os.kill(child_pid, 0)
+    finally:
+        if pid_file.exists():
+            _force_kill_pid(int(pid_file.read_text(encoding="utf-8").strip()))