diff --git a/bcc_core.py b/bcc_core.py index fc2f535..5fa75b9 100644 --- a/bcc_core.py +++ b/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): diff --git a/tests/test_core.py b/tests/test_core.py index d3f3f67..7ae2b82 100644 --- a/tests/test_core.py +++ b/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