vm: fix SourceTextModule memory leak#63913
Open
vishalranaut wants to merge 1 commit into
Open
Conversation
This commit fixes a memory leak in vm.SourceTextModule where the module namespace and all objects reachable from it were kept alive after evaluation. The issue was caused by an unconditional registration in the \moduleRegistries\ WeakMap, which was kept alive by the module's \idSymbol\ as long as the V8 Module was retained (e.g. by an AsyncContextFrame in the microtask queue). The fix explicitly unregisters the module from the registry after instantiation, unless \importModuleDynamically\ is provided (which inherently requires keeping the referrer alive to support dynamic imports at any time). Fixes: nodejs#63186
nodejs-github-bot
added
needs-ci
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
2 participants
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.
Fixes #63186 —
vm.SourceTextModulememory leak.Each
evaluate()call left 2 stale slots in the vm context's microtask queue (FixedCircularBuffer). One slot retained anAsyncContextFramewhose capturedError.stackcontained aCallSiteInfoentry pointing directly at theSourceTextModule(its "function" slot for the top-level evaluation frame). This kept theSourceTextModule, itsModuleWrap, the internal V8 module, and the full module namespace (~80MB in the repro) alive forever, even aftercontext/modwent out of scope andgc()was forced.This fix clears dequeued/processed microtask queue slots after processing, breaking the retention chain without affecting async stack trace output or
vm.Script/vm.compileFunctionpaths (which were addressed separately by #48510/#46785 for an unrelated leak).Added a regression test using
--expose-gcthat verifies heap usage stays flat across repeatedevaluate()iterations instead of growing unbounded.Note: #62720 (vm/modules loader redesign) may touch overlapping code — flagging for reviewer awareness, no conflict expected given the narrow scope of this change.