ci: verify the hang bound instead of assuming it, and widen the Windows kill - #646
Open
oyvindberg wants to merge 2 commits into
Open
ci: verify the hang bound instead of assuming it, and widen the Windows kill#646oyvindberg wants to merge 2 commits into
oyvindberg wants to merge 2 commits into
Conversation
…ws kill run-bounded.sh is the only thing between a hung test step and a destroyed runner, and nothing exercised it. It has now failed three times — runs 30697246865, 30752489661 and 31268861301 — each reaching the job's ceiling at 55m00s, which tears down the `if: always()` telemetry steps. The result is no artifact and no retrievable log (`BlobNotFound`), so the one hang worth reading is the one that reports nothing. Every fix so far was reasoned and shipped unverified, which is why the same signature came back twice. Two changes. A `bound-check` job hangs deliberately and asserts the bound does what the surrounding steps depend on. The harness shape is the point: a tree of bash sleeps is reaped by the process-group kill everywhere, so such a test would pass while the real thing kept failing. hang-tree.sh spawns a detached NATIVE Windows grandchild, because what actually hung was bleep -> BSP daemon -> forked test JVMs, and those are grandchildren outside any POSIX process group that had inherited the step's stdout. It asserts three things: exit 124, that the step ENDED (elapsed < 120s for a 30s bound — killing the child is necessary but not sufficient, since GitHub waits for the output pipes to close and a survivor holding stdout is exactly how a 20-minute bound became a 55-minute teardown), and that no process outlived the kill. Runs on windows-latest and ubuntu-latest, so a Windows-only regression cannot hide. kill_tree also sweeps by image name on Windows. Every part of the targeted path can fail silently: /proc/<pid>/winpid may not exist for the child shape, and `taskkill //T` only walks the tree it can see — both masked by `|| true`. Killing java.exe and bleep.exe outright is safe here specifically, since it runs on a CI runner at the point we have already decided the tree is unsalvageable and the only later steps read files. Verified locally before pushing, which is the habit that was missing: expiry returns 124, a normal exit passes its own code through (7), and no survivors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new job did its job on first run, in 46 seconds instead of 55 minutes, and split the failure in half for the first time: A hang fails the step, and ends it SUCCESS Nothing survived the kill FAILURE So on Windows the bound fires and the step ends (exit 124, well inside the ceiling) but the detached grandchild outlives the kill. `start /B` lets the intermediate `cmd` exit, so there is no living parent link left for `taskkill //T` to walk, and both that and the winpid translation fail silently behind `|| true`. A descendant walk cannot fix this — there is no ancestor to walk from — so the image-name sweep is load-bearing, not belt-and-braces. Which makes a hardcoded sweep list the remaining bug: it named java.exe and bleep.exe while the harness spawned ping.exe. The list is now BOUND_KILL_IMAGES, set by whoever knows what the bounded command actually starts, and node.exe joins the default because the Scala.js suites fork node and a forked node inherits the step's stdout exactly like a test JVM does. This also explains why production hung for 55 minutes while the test's bound ended in 46 seconds: the harness sends its grandchild's output to NUL, so it could not hold the pipe. Real forked JVMs do inherit stdout, so they survive AND hold it open, and GitHub waits for the pipes rather than for us. The POSIX survivor check was wrong in the other direction. `pgrep -f "sleep 3000"` matches any process whose command line merely contains the pattern — including the checker's own ancestry — and it reported a survivor locally that did not exist. Now matches comm exactly and filters on args, verified both ways: silent after a real bound, still catches a planted sleep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
run-bounded.shis the only thing between a hung test step and a destroyed runner, and nothing exercised it.It has failed three times — runs
30697246865,30752489661,31268861301— each reaching the job's ceiling at 55m00s, which tears down theif: always()telemetry steps. The result is no artifact and no retrievable log (BlobNotFound), so the one hang worth reading is the one that reports nothing. Every fix so far was reasoned and shipped unverified, which is why the same signature came back twice.A job that hangs on purpose
bound-checkruns a deliberate hang and asserts the bound does what the surrounding steps depend on it doing.The harness shape is the point. A tree of bash
sleeps is reaped by the process-group kill on every platform, so that test would pass while the real thing kept failing.hang-tree.shspawns a detached native Windows grandchild, because what actually hung wasbleep→ BSP daemon → forked test JVMs: grandchildren, outside any POSIX process group, holding the step's stdout.Three assertions:
Runs on
windows-latestandubuntu-latestso a Windows-only regression can't hide behind "works locally".A wider kill on Windows
Every part of the targeted path can fail silently —
/proc/<pid>/winpidmay not exist for the child shape, andtaskkill //Tonly walks the tree it can see, both masked by|| true. Now also sweepsjava.exe/bleep.exeby image name. Safe here specifically: it runs on a CI runner at the point we've already decided the tree is unsalvageable, and the only later steps read files.Verified before pushing
Which is the habit that was missing: expiry returns 124, a normal exit passes its own code through (7), no survivors.
Note this does not explain the original hang — that evidence was destroyed. It makes the next one diagnosable, which is the prerequisite.
🤖 Generated with Claude Code