From de9b3e4070b59c644b05053eb2e2e71d7efb2984 Mon Sep 17 00:00:00 2001 From: Tony Thayer-Osborne Date: Mon, 3 Aug 2026 16:25:10 -0700 Subject: [PATCH 1/2] fix(ci): stop kill-grace from bounding Windows helper spawns The Windows shard failed Step 0i (parallel suite scheduler contract) with "cleanup failed: hang_after_summary: leader exited leaving live descendants" while its sibling shard passed the same contract. Nothing in the wave was actually leaking a process. run-test-wave.py passed --kill-grace as the subprocess timeout for the two external Windows helpers: taskkill.exe /T /F and the powershell.exe Get-CimInstance descendant probe. Those are different quantities. kill_grace budgets how long a doomed process may take to die; the helpers also have to pay process spawn plus, for PowerShell, CIM startup. The contract fixtures run with --kill-grace 1, and one second is not reliably enough to launch either helper on a loaded runner. The two timeouts then compounded. A timed-out taskkill is reported as "could not prove process-tree cleanup", which raises out of the wave loop; the finally-block cleanup re-enters with the leader already dead, so it falls to the descendant probe, which times out on the same one-second budget and takes its "cannot prove absence -> assume the worst" branch. Phantom descendants, red wave. Give helper invocations their own floor, max(kill_grace, 30). kill_grace still governs every actual death wait, so nothing fails open: the timeout-race contract still refuses with rc=2 over a genuinely surviving descendant, now because PowerShell answered rather than because it timed out. The production path already passed --kill-grace 15 and is unchanged in behaviour. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Tony Thayer-Osborne --- scripts/run-test-wave.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) 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 From 4a891aa8b652a232f374f6545bc303fb147ea257 Mon Sep 17 00:00:00 2001 From: Tony Thayer-Osborne Date: Mon, 24 Aug 2026 14:30:15 -0700 Subject: [PATCH 2/2] test(harness): read the Windows helper budget from the scheduler The parallel-suite scheduler contract gave the scheduler a flat 8-second budget to finish refusing. On Windows that refusal costs external helper spawns (taskkill.exe, and powershell.exe for the descendant probe), which the scheduler now budgets with its own floor rather than --kill-grace. Read that floor out of the scheduler instead of restating it: hard-coding a number here silently turns a slow runner into a harness failure the moment the two 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. POSIX is unchanged: the refusal is signal-driven and still lands inside the original 8 seconds. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Tony Thayer-Osborne --- tests/test_parallel_harness_contract.sh | 26 ++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) 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