Skip to content

fix: keep test shards out of node's exit deadlock - #382

Merged
MohamedH1998 merged 6 commits into
mainfrom
ci/report-stuck-tests
Nov 11, 2026
Merged

MohamedH1998 merged 6 commits into
mainfrom
ci/report-stuck-tests

Conversation

@MohamedH1998

@MohamedH1998 MohamedH1998 commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

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 calls process.exit() after each file's tests. On node 24 that can deadlock (nodejs/node#64274, fix open in nodejs/node#66171):

  • the main thread, inside process.exit(), joins V8's worker threads;
  • a worker, partway through a concurrent Maglev compile, waits for a GC that only the main thread can run.

Native stacks from a hung Test (4/4) on this PR, taken from cli-migrate.test.ts after all 26 of its tests had passed:

Thread 1 "MainThread":
  uv_thread_join
  node::WorkerThreadsTaskRunner::Shutdown()
  node::NodePlatform::Shutdown()
  node::DefaultProcessExitHandlerInternal(...)
  node::Environment::Exit(...)
Thread 4 "V8Worker" (and thread 3, the same):
  v8::internal::CollectionBarrier::AwaitCollectionBackground(...)
  v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(...)
  ...
  v8::internal::maglev::MaglevCompilationJob::ExecuteJobImpl(...)

cli-migrate is 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.

Before After
Test files end process.exit() after their tests (--test-force-exit) On their own, as pnpm test already does locally
A file that never exits Shard cancelled at 20 min; the log ends mid-shard Shard fails at 10 min with the report below
A normal shard (slowest green one in the last 15 runs: 290 s) Passes Passes; the watchdog exits within 5 s of the tests

The report, as it printed for the hang above:

Processes holding the runner's pipes:
  3308 (fd 0, 1, 2): test file test/cli-migrate.test.ts
test/cli-migrate.test.ts (pid 3308): node report
  No report: the process didn't answer SIGUSR2, so its main thread is blocked.
test/cli-migrate.test.ts (pid 3308): threads
  3308 MainThread: waits in futex_do_wait …
test/cli-migrate.test.ts (pid 3308): native stacks
  (gdb backtrace of every thread, as above)
Error: Tests still running after 10 minutes; stopping them. The report above names what they wait on.

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

File Change
.github/workflows/ci.yml Drops --test-force-exit. Runs the tests in the background beside the watchdog; wait returns the tests' exit status
scripts/report-stuck-tests.mjs New. Polls the runner every 5 s. Past 10 minutes it reports, then stops the runner
packages/nimbus-docs/test/api-collection-dev-restart.test.ts Removes its temp root at exit, after Astro's last write

How to review

  1. .github/workflows/ci.yml: the shard step.
  2. test/api-collection-dev-restart.test.ts: the cleanup.
  3. 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: /proc wait states, then sudo gdb (installed if missing);
    • the bottom block reports, then always stops the runner and removes the reports it asked for.

Checks

  • Caught the real hang twice on this PR's own CI, before the fix (runs on cdb28389 and e998c455). 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-exit uncovered one test, api-collection-dev-restart, whose cleanup raced Astro's last content-store write. Force-exit had hidden it, though plain pnpm test would fail on it too. It now removes its root at exit, like dev-prerender-refresh already 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:

    • a half-written report could make parsing throw before the runner was stopped;
    • one fd closing mid-scan dropped every fd of that process;
    • cleanup removed node reports it didn't ask for.

    Each is fixed and was rerun against the synthetic hangs.

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.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://ci-report-stuck-tests.nimbus-docs.com (commit a3548a5)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://4fedbdea.nimbus-docs.com a3548a5 2026-11-10T18:35:21.396Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://88b02863.nimbus-docs.com e998c45 2026-11-10T17:18:47.069Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://9ed0a9d7.nimbus-docs.com cdb2838 2026-11-10T17:03:51.214Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://23440eba.nimbus-docs.com 515439b 2026-11-10T16:28:57.156Z Visit the dashboard ↗

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.
@MohamedH1998 MohamedH1998 changed the title chore: report what a stuck test shard waits on fix: keep test shards out of node's exit deadlock Nov 10, 2026
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.
@MohamedH1998
MohamedH1998 marked this pull request as ready for review November 11, 2026 11:29
@MohamedH1998
MohamedH1998 merged commit 68f0fc6 into main Nov 11, 2026
111 of 113 checks passed
@MohamedH1998
MohamedH1998 deleted the ci/report-stuck-tests branch November 11, 2026 11:30

This branch was successfully deployed

1 active deployment
cloudflare — a3548a5f Deployed Nov 11, 2026 by MohamedH1998 via Delete closed PR Preview #115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant