run_smoke: cap each script and kill its process group on expiry - #89
Merged
Merged
Conversation
This runner called subprocess.run(capture_output=True) with no timeout at all. When a script leaves a grandchild holding the inherited stdout pipe, the parent blocks on that pipe forever — even though the script's own work finished and the direct child exited. The gate then sits until the 6-hour GitHub Actions ceiling, reporting nothing since the last completed script. That is autolens_workspace_test#196, and this copy had the same shape. Reproduced against this exact file: a two-script suite where the second spawns a grandchild and exits. With BUILD_SCRIPT_TIMEOUT=4 the old runner printed "::group::hangs.py" and then nothing, and was still hung when the harness killed it at 30s — the cap was ignored entirely because there was none. The new one reports "[TIMEOUT (4s)] hangs.py — 4.0s" at 4.0s, exits 1, and leaves zero surviving grandchildren. Adopts the implementation autolens_workspace_test already runs: per-script cap via timeout_for(env) so a profile's BUILD_SCRIPT_TIMEOUT override is resolved parent-side where the kill timer lives, the child in its own session so the whole group is killed, and exit 124 rather than the signal — a timeout means "raise the cap or SLOW-skip it", which -9 would mislabel as an ordinary failure. timeout_for and kill_group are imported from PyAutoHands build_util, each with a local fallback for a checkout without Hands on PYTHONPATH, so the PR gate and the mega-run share one implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTPM1RmMvSuMvJkEntAMv8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Leg A of the per-script timeout backport. Depends on PyAutoHands#257, which adds the
kill_grouphelper this imports — merge that first (the import is guarded, so an older Hands falls back rather than breaking the gate, but the shared implementation only arrives once #257 lands).The defect
This runner called
subprocess.run(capture_output=True)with no timeout at all. When a script leaves a grandchild holding the inherited stdout pipe, the parent blocks on that pipe forever — even though the script's own work finished and the direct child exited. The gate then sits until the 6-hour GitHub Actions ceiling, reporting nothing since the last completed script. That is autolens_workspace_test#196, and this copy had the same shape.Reproduced, against this exact file
A two-script suite whose second script spawns a grandchild and exits, with
BUILD_SCRIPT_TIMEOUT=4:::group::hangs.pyand nothing further; still hung when the harness killed it at 30s. The cap was ignored because there was none.[TIMEOUT (4s)] hangs.py — 4.0s, exit 1, zero surviving grandchildren.What it adopts
The implementation
autolens_workspace_testalready runs:timeout_for(env), so a profile'sBUILD_SCRIPT_TIMEOUToverride is resolved parent-side where the kill timer lives;-9would mislabel as an ordinary failure.timeout_forandkill_groupare imported from PyAutoHandsbuild_util, each with a local fallback for a checkout without Hands onPYTHONPATH, so the PR gate and the mega-run share one implementation. The fallback path was verified against a stubbuild_utilexposingtimeout_forbut nokill_group: still reports TIMEOUT, still zero survivors.The pre-existing docstring wording in this file is left alone — tidying it is not this PR's job.
Generated by Claude Code