Skip to content

Keep test:rail-list out of a deadlock at exit, and name what hung in CI - #511

Merged
Kashyab Ambarani (kashyab12) merged 3 commits into
mainfrom
mako/suite-hang-named
Nov 7, 2026
Merged

Kashyab Ambarani (kashyab12) merged 3 commits into
mainfrom
mako/suite-hang-named

Conversation

@kashyab12

@kashyab12 Kashyab Ambarani (kashyab12) commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • test:rail-list hung to the 1200 s suite limit on main at 7ae2137 and on Find a Fin's reach from what's listed, not by reading its tree's journals #503. It's a deadlock inside Node, not a leaked handle or an unresolved promise: both scripts are straight-line code that end in process.exit(0).
  • From the stacks of a hung process, on macos-15:
    • The main thread is in process.exit → DisposePlatform → WorkerThreadsTaskRunner::Shutdown → uv_thread_join, waiting for V8's worker.
    • The worker is in a concurrent Maglev compile, out of room to allocate, in CollectionBarrier::AwaitCollectionBackground, waiting for the main thread to run a collection.
  • Neither ever moves, at 0% CPU. This suite lands in it because it loads a lot of code and exits about a second later, while the JIT is still compiling it.
  • The two scripts now run with --no-maglev. It's on the scripts because NODE_OPTIONS refuses that flag.
  • ci-suites now runs each suite through scripts/lib/run-suite.mjs. A suite is stopped after 5 quiet minutes (the longest silence of any suite on recent main runs is 80 s) as well as at the 20 minute limit. It prints HUNG:, every process in the suite's group and each Node process's stacks on all threads, then ends the whole group.

Test plan

  • Reproduced on macos-15, cold tsx cache: 5 hangs in 64 rounds, stacks captured by the new runner
  • The same loop on this fix, on the same runner: 260 of 260 clean
  • 80 cold rounds on Node 24.22.2 here and 100 warm ones don't hang, so this Mac can't prove it either way; the loop on the runner is the proof
  • test-run-suite.mjs: output and exit code come through; a silent suite is named down to a child and the group ends; a talkative one still stops at its limit; a process left in the background holds nothing
  • tsx --no-maglev reaches the child's execArgv
  • npm run lint and npm run typecheck pass

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test:rail-list hung to CI's 20 minute limit. A cold loop on macos-15 caught
it 5 times in 64 rounds, and the stacks show a deadlock in Node: exit joins
V8's workers, while one, compiling with Maglev, waits on a collection only
the main thread can run. Exiting a second after loading all this code is
what lands in it, so these two scripts run with --no-maglev.

A suite now stops after 5 quiet minutes, about four times the longest
silence of any, saying what was running and every thread's stack, then
ending its whole group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved as a CI workaround at c48f8427a0ce18bf949478d5a45e7b79542c8426.

The cause is supported by the captured stacks, not just a timeout: run 42349900865 shows the exiting main thread joining V8 workers while a Maglev worker waits in CollectionBarrier::AwaitCollectionBackground. I checked the underlying runner logs: 3/53 and 2/11 stopped before the workaround, versus 260/260 clean with it. These are the author's CI experiments, inspected here, not a loop I repeated locally.

Locally, test:rail-list and test-run-suite.mjs passed, and tsx forwards --no-maglev. The rail/archive test statements are unchanged apart from comments. Planted faults dropping a Thread's older Session and archiving only its first target each still fail their assertion under the new flag. A runner mutation returning success for a stopped suite also fails its test. All mutations were restored. The runner preserves nonzero exit codes, names/kills the owned group on silence or elapsed limit, and CI treats its null outcome as failure. Every existing suite remains exactly once, with only test:run-suite added.

Real-host scope: this is not inherently test-only. nodeHostExit calls process.exit for real host quits/restarts/signals/startup failures, and plain-Node mako-host does not disable these compilers. The same class is an open upstream Node shutdown issue; its proposed fix also identifies concurrent Sparkplug, so --no-maglev is a targeted mitigation for the captured test fault, not a universal shutdown fix. Plain-Node hosts need a separate shutdown investigation; no actual Mako host hang was reproduced here.

The Mac Helper deserves a narrower conclusion. I checked the installed runtime (Electron 43.4.0 / Node 24.18.1), whose defaults enable both compilers. But Electron's Node-mode entry skips initialization of Node's own V8 platform, and Node's platform disposal skips shutdown when that platform was not initialized. Thus the captured NodePlatform::Shutdown join is not established for the Mac Helper merely by its enabled Maglev flag. This evidence does not identify the person's running-host freeze as this deadlock. It does support landing this bounded CI fix without claiming the product shutdown question solved.

A suite whose command failed to start ended with no word of it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 09eff9c7df8d4bd526f29f6be3542bf58785c1e2, delta from c48f8427a.

This is the two-file diagnostic addition described: a spawn error now names the command and its error, then returns the same null failure as before. It does not turn an unstarted suite into a pass or change the silence/elapsed limits.

test-run-suite.mjs passed, including the new missing-command case (DID NOT START, command path, ENOENT, null outcome). Removing only the diagnostic makes the new assertion fail with empty output. The control was restored and the worktree is clean. The prior CI-workaround approval and its distinction between plain-Node host shutdown and Electron's Mac Helper remain unchanged.

@kashyab12
Kashyab Ambarani (kashyab12) merged commit 68d68e7 into main Nov 7, 2026
27 checks passed
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