Repository navigation
fix: keep test shards out of node's exit deadlock - #382
Merged
Merged
Conversation
A test shard sometimes runs to the job timeout with no failure: a test file outlives its tests, or a process it started keeps its output open, and the runner waits on it. --test-timeout can't end that wait. After 12 minutes the shard now prints the process tree, every process holding the runner's pipes, and a node report summary for each file still running, then stops.
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://ci-report-stuck-tests.nimbus-docs.com (commit a3548a5)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
With --test-force-exit the runner calls process.exit() after each file's tests. On node 24 that can deadlock: the main thread joins V8's worker threads while a concurrent Maglev compile on one of them waits for a GC only the main thread can run (nodejs/node#64274). cli-migrate, the slowest file, hit it often enough to hang a shard until the job timed out. Files now exit on their own; report-stuck-tests names any that don't.
Astro's content sync writes the data store after server.stop() returns. The after hook deleted the root first, so the late write rejected once the file was no longer force-exited. Remove the root at exit instead, as dev-prerender-refresh does.
This branch was successfully deployed
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.
A
Test (N/4)shard sometimes ran to the 20-minute job timeout and was cancelled, with no failing test in the log. This finds the cause, removes it, and makes any future stuck shard say what it waits on.The cause
The shard step passed
--test-force-exit, so the runner callsprocess.exit()after each file's tests. On node 24 that can deadlock (nodejs/node#64274, fix open in nodejs/node#66171):process.exit(), joins V8's worker threads;Native stacks from a hung
Test (4/4)on this PR, taken fromcli-migrate.test.tsafter all 26 of its tests had passed:cli-migrateis the shard's longest, most JIT-heavy file, so it's the one that hits this. Output prints in file order, which is why each hung log stopped right after its last test.Before → after
APIs, config, packages: unchanged. CI only.
process.exit()after their tests (--test-force-exit)pnpm testalready does locallyThe report, as it printed for the hang above:
For a file that answers, it prints its JavaScript stack and the handles keeping it alive, from the node report. Node reports also hold environment variables, which are never printed, and the reports are removed.
What changed
.github/workflows/ci.yml--test-force-exit. Runs the tests in the background beside the watchdog;waitreturns the tests' exit statusscripts/report-stuck-tests.mjspackages/nimbus-docs/test/api-collection-dev-restart.test.tsHow to review
.github/workflows/ci.yml: the shard step.test/api-collection-dev-restart.test.ts: the cleanup.scripts/report-stuck-tests.mjs:holders()finds what holds the runner's pipes, pairing its socket ends with their peers (ss -x; libuv's stdio pipes are socketpairs);summarise()reads the SIGUSR2 report node already enables for each test file;nativeStacks()handles a file too blocked to answer:/procwait states, thensudo gdb(installed if missing);Checks
Caught the real hang twice on this PR's own CI, before the fix (runs on
cdb28389ande998c455). The second run produced the stacks above.After the fix, 4 full CI runs on
a3548a5f: all 8 shards passed each time, with jobs at 152–310 s including setup.Dropping
--test-force-exituncovered one test,api-collection-dev-restart, whose cleanup raced Astro's last content-store write. Force-exit had hidden it, though plainpnpm testwould fail on it too. It now removes its root at exit, likedev-prerender-refreshalready does.Synthetic hangs in a node 24 container (a file that never exits, a leaked process, a blocked main thread): each named, the step fails at the deadline, no reports left. The native-stack path was tested with gdb absent, so the install branch ran.
Real shard 4/4 with a 60 s deadline: the report names the running test files.
Normal and failing runs: the watchdog exits by itself and the step's status matches the tests'.
Review: 3 Codex adversarial rounds to consensus on the watchdog. Rounds 1–2 found:
Each is fixed and was rerun against the synthetic hangs.