Merge pull request 'fix: kill the whole process tree on Windows spawn-test timeout (#13)' (#50) from fix/13-windows-tree-kill into main

bcc_core.py

@@ -1581,6 +1581,23 @@ def server_log_path(name: str) -> Path | None:
 _STDERR_CAP = 4096  # bytes
 
 
+def _kill_process_tree_windows(pid: int) -> None:
+    """
+    Kill `pid` and its whole descendant tree via `taskkill /T /F` (issue #13).
+
+    Popen.kill() only terminates the direct child; runner-style commands
+    (npx → node → server, cmd → real process) leave the actual server alive,
+    leaking a process on every Windows spawn test. taskkill walks the tree.
+    """
+    flags = getattr(subprocess, "CREATE_NO_WINDOW", 0)  # no console flash from the GUI exe
+    with contextlib.suppress(OSError):
+        subprocess.run(
+            ["taskkill", "/PID", str(pid), "/T", "/F"],
+            capture_output=True,
+            creationflags=flags,
+        )
+
+
 def spawn_test(data: dict, timeout: float = 3.0) -> dict:
     """
     Attempt to start a stdio server and observe it for `timeout` seconds.
@@ -1675,6 +1692,10 @@ def _spawn_test_impl(data: dict, timeout: float) -> dict:
     )
     if os.name != "nt":
         popen_kwargs["start_new_session"] = True  # own process group → clean kill
+    else:
+        # The packaged app is windowed (console=False); without this every
+        # spawn test of a console server flashes a console window.
+        popen_kwargs["creationflags"] = getattr(subprocess, "CREATE_NO_WINDOW", 0)
 
     try:
         proc = subprocess.Popen(args_list, **popen_kwargs)
@@ -1696,7 +1717,7 @@ def _spawn_test_impl(data: dict, timeout: float) -> dict:
             if os.name != "nt":
                 os.killpg(os.getpgid(proc.pid), _signal.SIGKILL)
             else:
-                proc.kill()  # best-effort on Windows
+                _kill_process_tree_windows(proc.pid)
         except OSError:
             pass
         with contextlib.suppress(subprocess.TimeoutExpired):

tests/test_core.py

@@ -329,6 +329,55 @@ def test_spawn_test_large_stderr_does_not_deadlock():
     assert len(r["stderr"]) <= c._STDERR_CAP
 
 
+def test_windows_tree_kill_uses_taskkill(monkeypatch):
+    """The Windows kill path must walk the process tree (taskkill /T /F);
+    Popen.kill() alone orphans grandchildren (issue #13)."""
+    calls = []
+
+    def fake_run(cmd, **kwargs):
+        calls.append(cmd)
+        return _FakeRC()
+
+    class _FakeRC:
+        returncode = 0
+
+    monkeypatch.setattr(c.subprocess, "run", fake_run)
+    c._kill_process_tree_windows(1234)
+    assert calls == [["taskkill", "/PID", "1234", "/T", "/F"]]
+
+
+@pytest.mark.skipif(os.name != "nt", reason="Windows-only: real process-tree kill")
+def test_spawn_test_windows_kills_child_process_tree(tmp_path):
+    """A spawn-tested parent that has its own child (npx -> node style) must
+    not leave the child running after the timeout kill (issue #13)."""
+    import subprocess as sp
+    import time as _t
+
+    pid_file = tmp_path / "child.pid"
+    parent_script = (
+        "import subprocess, sys, time\n"
+        "p = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)'])\n"
+        f"open({str(pid_file)!r}, 'w').write(str(p.pid))\n"
+        "time.sleep(60)\n"
+    )
+    r = c.spawn_test({"command": sys.executable, "args": ["-c", parent_script]}, timeout=2.0)
+    assert r["outcome"] == "ok"  # parent was still running at timeout, then tree-killed
+    assert pid_file.is_file(), "parent never spawned its child"
+    child_pid = int(pid_file.read_text())
+
+    deadline = _t.time() + 5.0
+    alive = True
+    while _t.time() < deadline:
+        out = sp.run(
+            ["tasklist", "/FI", f"PID eq {child_pid}"], capture_output=True, text=True
+        ).stdout
+        alive = str(child_pid) in out
+        if not alive:
+            break
+        _t.sleep(0.25)
+    assert not alive, f"child pid {child_pid} survived the spawn-test kill"
+
+
 def test_spawn_test_non_string_env_value():
     # Pasted JSON can carry numeric env values ("env": {"PORT": 8080}) that never
     # round-trip through the editor. Popen rejects non-str env; spawn_test must