diff --git a/scripts/run-test-wave.py b/scripts/run-test-wave.py index 44a16c23c..b3998f541 100755 --- a/scripts/run-test-wave.py +++ b/scripts/run-test-wave.py @@ -27,6 +27,14 @@ SKIPPED = re.compile(r"(?:^|, )(?P[0-9]+) skipped") SLOW_SUITES = frozenset(("incremental", "store_arch", "daemon_runtime")) POLL_SECONDS = 0.05 +# Floor for how long an external Windows helper (taskkill.exe, powershell.exe) +# may take to answer. This is deliberately NOT --kill-grace: that flag budgets +# how long a doomed process may take to die, while this budgets process spawn +# plus CIM startup on a loaded runner, which routinely exceeds a second. Wiring +# the two together made a small kill grace flake the whole wave -- taskkill +# timing out is reported as "could not prove cleanup", and the descendant probe +# that runs afterwards fails closed on its own timeout. +WINDOWS_HELPER_TIMEOUT_SECONDS = 30 @dataclass @@ -123,6 +131,10 @@ def start_suite( ) +def windows_helper_timeout(kill_grace: int) -> int: + return max(kill_grace, WINDOWS_HELPER_TIMEOUT_SECONDS) + + def windows_descendants(pid: int, timeout: int) -> bool: """True if any live process still claims `pid` as its parent. @@ -167,7 +179,7 @@ def terminate_process_tree(active: ActiveSuite, kill_grace: int) -> None: # how a deliberately-hanging fixture suite reddened a release run. # taskkill /T cannot walk a tree from a dead PID, so prove cleanup # the only way still available -- nothing is parented to it. - if windows_descendants(process.pid, kill_grace): + if windows_descendants(process.pid, windows_helper_timeout(kill_grace)): raise RuntimeError( f"suite {active.name!r} leader exited leaving live descendants" ) @@ -185,7 +197,7 @@ def terminate_process_tree(active: ActiveSuite, kill_grace: int) -> None: stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - timeout=kill_grace, + timeout=windows_helper_timeout(kill_grace), ) except (OSError, subprocess.TimeoutExpired): completed = None diff --git a/tests/test_parallel_harness_contract.sh b/tests/test_parallel_harness_contract.sh index a658dc56a..511fb25a0 100755 --- a/tests/test_parallel_harness_contract.sh +++ b/tests/test_parallel_harness_contract.sh @@ -256,6 +256,7 @@ python3 - "$scheduler" "$fixture" "$(command -v python3)" <<'PY' from __future__ import annotations import ctypes +import importlib.util import os import pathlib import signal @@ -275,6 +276,29 @@ release = barrier / "timeout_exit_race.release" descendant_path = fixture / "descendant.pid" +def scheduler_wait_budget() -> int: + """Seconds to allow the scheduler to finish refusing. + + On POSIX the refusal is signal-driven and lands well inside --kill-grace. + On Windows it costs external helper spawns (taskkill.exe, and powershell.exe + for the descendant probe), which the scheduler deliberately budgets with its + own floor rather than --kill-grace. Read that floor from the scheduler + instead of restating it: hard-coding a budget here silently turns a slow + runner into a harness failure the moment the two numbers drift apart. The + refusal path can spend the floor twice -- once proving descendants, once in + the cleanup re-entry -- so allow both plus interpreter startup. + """ + if os.name != "nt": + return 8 + spec = importlib.util.spec_from_file_location("cbm_run_test_wave", scheduler) + module = importlib.util.module_from_spec(spec) + # Register before exec: @dataclass resolves annotations through + # sys.modules[cls.__module__], which is None for an unregistered module. + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module.WINDOWS_HELPER_TIMEOUT_SECONDS * 2 + 10 + + def process_state(pid: int) -> str: if os.name == "nt": handle = ctypes.windll.kernel32.OpenProcess(0x101000, False, pid) @@ -370,7 +394,7 @@ try: raise SystemExit("FAIL: scheduler did not observe the forced leader exit") time.sleep(0.02) release.write_text("release\n", encoding="utf-8") - stdout, stderr = process.communicate(timeout=8) + stdout, stderr = process.communicate(timeout=scheduler_wait_budget()) if os.name == "nt": # Assert the PROPERTY, not the wording. This used to require the phrase