fix(runtime): unref the game-state watcher so it can't pin the event loop - #7
Merged
TheArchitectit merged 2 commits intoJul 29, 2026
Conversation
…loop An fs_event handle that is active AND referenced holds node's event loop open. ensureGameStateWatcher() opened one per MegaRuntime and only dispose() released it — so any process that never reaches session_shutdown finished all its work and then sat there forever. That is exactly what the extension test files did: a node diagnostic report taken at hang time showed ten referenced fs_event handles, one per runtime the file constructs. The leak was invisible to process.getActiveResourcesInfo(), which does not report fs_event, which is why earlier passes concluded "pending promise, no handle" and blamed PGlite (MEGACOMPACT_PGLITE_DISABLED=true only appeared to fix it because that path never reaches bindRepo -> ensureGameStateWatcher). The watcher is a cache-eviction convenience, never a reason to keep the process alive — unref'd like the perf interval. While pi runs there are always other referenced handles, so it still fires normally. state.test.js, mega-cache-replay.test.js and mega-teamrun.test.js now exit on their own (4s / 6s / 13s; previously unbounded). mega-compact.test.js still hangs on an unrelated PGlite spin.
…ins the loop The comment written alongside the PGlite-close fix asserted that dispose() "only released the fs.watch handle and the perf interval, neither of which was the culprit". The fs.watch handle WAS a culprit: a node diagnostic report (--report-on-signal) taken at hang time lists the game-state watcher as an active AND referenced fs_event, which pins the event loop by itself — independently of PGlite, and on any path that never reaches session_shutdown. Also replaces the hand-wavy "its handles keep node's event loop alive" with the measured behavior: on a teardown that opens both indexes, closing exits in ~3s, while leaving them open either stalls ~13s or never exits at all. Both closes remain correct and necessary. Comment-only; no behavior change.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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.
The bug
ensureGameStateWatcherImpl()opens onefs.watchperMegaRuntime. Anfs_eventhandle that is active and referenced holds node's event loop open,and only
dispose()released it — so any process that never reachessession_shutdownfinishes all its work and then sits there forever.That is exactly what the extension test files do. A node diagnostic report
(
node --report-on-signal --report-signal=SIGUSR2) taken at hang time shows tenreferenced
fs_eventhandles, one per runtime the file constructs:The watcher is a cache-eviction convenience, never a reason to keep the process
alive — so it is now
unref()'d at creation, exactly like the perf samplinginterval. While pi is running there are always other referenced handles, so the
watcher still fires normally;
dispose()still closes it for prompt fd release.Result
Run directly, each file exiting on its own (the file exiting is the assertion):
extensions/mega-runtime/state.test.jsextensions/mega-cache-replay.test.jsextensions/mega-teamrun.test.jsWhy this hid for so long
process.getActiveResourcesInfo()does not reportfs_eventhandles, so athang time it shows only stdio pipes — which reads as "a pending promise, no open
handle" and sent earlier investigation toward PGlite.
MEGACOMPACT_PGLITE_DISABLED=truedid make the files exit, but only because thatpath never reaches
bindRepo()→ensureGameStateWatcher().Relationship to #5 and #6
This is a third, independent holder of the event loop, and it does not
supersede either earlier fix. I re-measured #5's premise to be sure: opening both
indexes and then closing them exits in ~3s, while leaving them open either stalls
~13s or never exits at all — so closing on shutdown remains necessary. #6's
bounded open is untouched by any of this.
The second commit is comment-only. The note added alongside #5 stated that
dispose()"only released the fs.watch handle and the perf interval, neither ofwhich was the culprit". The fs.watch handle was a culprit, so that comment is
corrected in place rather than left to mislead the next reader; the measured
close-vs-leave-open numbers replace the hand-wavy phrasing. The perf interval
genuinely is not a culprit — it is unref'd.
Not fixed here
extensions/mega-compact.test.tsstill hangs, but on an unrelated defect: itwedges inside its final
cleanuptest at ~99% CPU, blocked in PGlite'ssynchronous
execProtocolRawSyncon theINSERT INTO memory_indexfromupsertMemoryEmbedding(). Because the loop is blocked, thePromise.race(…, setTimeout(3000))guard in that test can never fire. Ruled outso far: zero/NaN vectors, duplicate vectors, bulk inserts, close racing an
in-flight upsert, use-after-close, and the data itself (replaying the file's real
tuples into a fresh index runs at 1-2ms each). Worth its own issue.
Also unrelated and pre-existing: the test "S28: non-length stopReasons do not arm
the length-stop flag" fails identically on unmodified
master.