Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions scripts/run_tests_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,36 @@ def _run_one_file(
bound a pathologically slow or hung file as a whole.
"""
cmd = [sys.executable, "-m", "pytest", str(file), *pytest_args]


# Isolate each pytest child from the runner's console / process group.
#
# POSIX: start_new_session=True → os.setsid() in the child, placing it
# at the head of its own process group so _kill_tree can SIGKILL the
# group atomically.
#
# Windows: start_new_session only maps to CREATE_NEW_PROCESS_GROUP in
# CPython 3.12+ — on 3.11 it is silently ignored, so every child
# shared the runner's console process group. One os.kill(pid, 0)
# liveness probe anywhere in the run — which on Windows routes through
# GenerateConsoleCtrlEvent (bpo-14484) — then broadcast
# KeyboardInterrupt to every concurrent child AND the runner itself.
# Pass the creationflags explicitly, on every Python version:
# CREATE_NEW_PROCESS_GROUP — child is its own ctrl-event group root,
# so console ctrl events can't fan out across children;
# CREATE_NO_WINDOW — child gets its own invisible console, fully
# insulating it from ctrl-event broadcasts on the runner's console
# (and suppressing per-child conhost window flashes).
# _kill_tree handles the Windows kill path via taskkill /F /T.
if sys.platform == "win32":
isolation_kwargs: Dict[str, object] = {
"creationflags": (
subprocess.CREATE_NEW_PROCESS_GROUP
| subprocess.CREATE_NO_WINDOW
),
}
else:
isolation_kwargs = {"start_new_session": True}

subproc_start = time.monotonic()
# launch the pytest process
proc = subprocess.Popen(
Expand All @@ -262,11 +291,7 @@ def _run_one_file(
text=True,
# skipping writing bytecode because we're running a bunch of parallel python processes on the same code
env={**os.environ, 'PYTHONDONTWRITEBYTECODE': '1'},
# POSIX: place the child at the head of its own process group so
# _kill_tree can SIGKILL the group atomically.
# Windows: this maps to CREATE_NEW_PROCESS_GROUP in CPython 3.12+;
# _kill_tree handles the Windows path via taskkill /F /T.
start_new_session=True,
**isolation_kwargs,
)

# Capture the pgid NOW, before the leader can exit and be reaped. Once
Expand Down
63 changes: 63 additions & 0 deletions tests/test_run_tests_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,66 @@ def test_positional_path_not_treated_as_flag(tmp_path: Path) -> None:
# Discovery found the probe file (2 tests), proving the positional path
# was consumed as a root, not forwarded to pytest as a bad flag.
assert "test_flagprobe.py" in proc.stdout, proc.stdout


def _load_runner_module():
"""Import scripts/run_tests_parallel.py as a module (it's not a package)."""
import importlib.util

runner = Path(__file__).resolve().parent.parent / "scripts" / "run_tests_parallel.py"
spec = importlib.util.spec_from_file_location("_run_tests_parallel_under_test", runner)
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
return mod


def test_children_spawn_in_their_own_process_group(monkeypatch, tmp_path: Path) -> None:
"""Every pytest child must be isolated from the runner's process group.

POSIX: ``start_new_session=True`` (os.setsid) so ``_kill_tree`` can
SIGKILL the group atomically.

Windows: ``start_new_session`` is silently IGNORED before CPython 3.12,
so the runner must pass ``CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW``
creationflags explicitly. Without them every child shares the runner's
console process group, and a single ``os.kill(pid, 0)`` liveness probe
anywhere in the run — which on Windows routes through
GenerateConsoleCtrlEvent (bpo-14484) — broadcasts KeyboardInterrupt to
every concurrent child AND the runner itself (observed: the runner died
at 100% completion with ~200 collateral KeyboardInterrupt failures).
"""
mod = _load_runner_module()
captured: dict = {}

class _FakeProc:
pid = 99999
returncode = 0

def communicate(self, timeout=None):
return ("1 passed", None)

def poll(self):
return 0

def kill(self):
pass

def fake_popen(cmd, **kwargs):
captured.update(kwargs)
return _FakeProc()

monkeypatch.setattr(mod.subprocess, "Popen", fake_popen)
# _kill_tree shells out to taskkill on Windows — neuter it so the fake
# pid can't hit a real process.
monkeypatch.setattr(mod, "_kill_tree", lambda proc, pgid=None: None)

probe = tmp_path / "test_probe.py"
probe.write_text("def test_ok():\n assert True\n")
mod._run_one_file(probe, [], tmp_path, 30.0)

if sys.platform == "win32":
flags = captured.get("creationflags", 0)
assert flags & subprocess.CREATE_NEW_PROCESS_GROUP, captured
assert flags & subprocess.CREATE_NO_WINDOW, captured
else:
assert captured.get("start_new_session") is True, captured
33 changes: 20 additions & 13 deletions tests/tools/test_zombie_process_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
gateway deployments.
"""

import os
import signal
import subprocess
import sys
import threading

import psutil


def _spawn_sleep(seconds: float = 60) -> subprocess.Popen:
Expand All @@ -21,12 +20,15 @@ def _spawn_sleep(seconds: float = 60) -> subprocess.Popen:


def _pid_alive(pid: int) -> bool:
"""Return True if a process with the given PID is still running."""
try:
os.kill(pid, 0)
return True
except (ProcessLookupError, PermissionError):
return False
"""Return True if a process with the given PID is still running.

Never probe with ``os.kill(pid, 0)`` here: on Windows that is NOT a
no-op — it routes through GenerateConsoleCtrlEvent (bpo-14484) and
broadcasts Ctrl+C across the shared console, killing every concurrent
pytest child AND the parallel-runner process. psutil probes via
OpenProcess/GetExitCodeProcess on Windows (no signals involved).
"""
return psutil.pid_exists(pid)


class TestZombieReproduction:
Expand All @@ -36,13 +38,14 @@ def test_orphaned_processes_survive_without_cleanup(self):
"""REPRODUCTION: processes spawned directly survive if no one kills
them — this models the gap that causes zombie accumulation when
the gateway drops agent references without calling close()."""
pids = []
procs = []

try:
for _ in range(3):
proc = _spawn_sleep(60)
pids.append(proc.pid)
procs.append(proc)

pids = [p.pid for p in procs]
for pid in pids:
assert _pid_alive(pid), f"PID {pid} should be alive after spawn"

Expand All @@ -56,10 +59,14 @@ def test_orphaned_processes_survive_without_cleanup(self):
f"expected it to survive (demonstrating the bug)"
)
finally:
for pid in pids:
# Popen.kill() (TerminateProcess on Windows, SIGKILL on POSIX)
# instead of os.kill(pid, signal.SIGKILL): signal.SIGKILL does
# not exist on Windows, so the old cleanup raised AttributeError.
for p in procs:
try:
os.kill(pid, signal.SIGKILL)
except (ProcessLookupError, PermissionError):
p.kill()
p.wait(timeout=5)
except Exception:
pass

def test_explicit_terminate_reaps_processes(self):
Expand Down
Loading