-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
src: clear all linked module caches once instantiated #59117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Review requested:
|
af391e1
to
352631b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #59117 +/- ##
=======================================
Coverage ? 90.05%
=======================================
Files ? 648
Lines ? 191089
Branches ? 37449
=======================================
Hits ? 172091
Misses ? 11600
Partials ? 7398
🚀 New features to boost your workflow:
|
352631b
to
eb17cb7
Compare
30ab1d2
to
afd12e1
Compare
There are two phases in module linking: link, and instantiate. These two operations are required to be separated to allow cyclic dependencies. `v8::Module::InstantiateModule` is only required to be invoked on the root module. The global references created by `ModuleWrap::Link` are only cleared at `ModuleWrap::Instantiate`. So the global references created for depended modules are usually not cleared because `ModuleWrap::Instantiate` is not invoked for each of depended modules, and caused memory leak. The change references the linked modules in an object internal slot. This is not an issue for Node.js ESM support as these modules can not be off-loaded. However, this could be outstanding for `vm.Module`.
afd12e1
to
7886911
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % a suggestion. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, good spot
Landed in e55e0a7 |
Local<Object> module_object = | ||
dependent->resolve_cache_[cache_key].Get(isolate); | ||
if (module_object.IsEmpty() || !module_object->IsObject()) { | ||
auto it = dependent->resolve_cache_.find(cache_key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave this a bit of thought and I think we might be able to just save the hash/look up cost completely if we have an V8 API that just always gives us the index. Put together a prototype: https://chromium-review.googlesource.com/c/v8/v8/+/6804466
There are two phases in module linking: link, and instantiate. These two operations are required to be separated to allow cyclic dependencies. `v8::Module::InstantiateModule` is only required to be invoked on the root module. The global references created by `ModuleWrap::Link` are only cleared at `ModuleWrap::Instantiate`. So the global references created for depended modules are usually not cleared because `ModuleWrap::Instantiate` is not invoked for each of depended modules, and caused memory leak. The change references the linked modules in an object internal slot. This is not an issue for Node.js ESM support as these modules can not be off-loaded. However, this could be outstanding for `vm.Module`. PR-URL: nodejs#59117 Fixes: nodejs#50113 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
There are two phases in module linking: link, and instantiate. These two operations are required to be separated to allow cyclic dependencies. `v8::Module::InstantiateModule` is only required to be invoked on the root module. The global references created by `ModuleWrap::Link` are only cleared at `ModuleWrap::Instantiate`. So the global references created for depended modules are usually not cleared because `ModuleWrap::Instantiate` is not invoked for each of depended modules, and caused memory leak. The change references the linked modules in an object internal slot. This is not an issue for Node.js ESM support as these modules can not be off-loaded. However, this could be outstanding for `vm.Module`. PR-URL: nodejs#59117 Fixes: nodejs#50113 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original commit message: [api] Add index-based module resolution in InstantiateModule() Add new InstantiateModule() overload that allows embedders to identify modules requests by index in the module requests array rather than using specifier and import attributes. When embedders want to fetch all the modules using information from module->GetModuleRequests() before calling InstantiateModule() instead of having to do the fetching inside the InstantiateModule() callback, they could just maintain a simple array of modules indexed by module request positions and look up the fetched the module by index in the new callback. Previously this has to be done by mapping from specifier and import attributes to module objects cached on the embedder side, leading to an overhead to hash the specifier and import attributes for each module request. Refs: nodejs#59117 Bug: 435317398 Change-Id: Ie017d2f3ccc605e0f58aa423504b5fa5fdbcc633 Refs: v8/v8@1db0e6b Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
There are two phases in module linking: link, and instantiate. These two operations are required to be separated to allow cyclic dependencies. `v8::Module::InstantiateModule` is only required to be invoked on the root module. The global references created by `ModuleWrap::Link` are only cleared at `ModuleWrap::Instantiate`. So the global references created for depended modules are usually not cleared because `ModuleWrap::Instantiate` is not invoked for each of depended modules, and caused memory leak. The change references the linked modules in an object internal slot. This is not an issue for Node.js ESM support as these modules can not be off-loaded. However, this could be outstanding for `vm.Module`. PR-URL: #59117 Fixes: #50113 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | minor | `24.5.0` -> `24.6.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>nodejs/node (node)</summary> ### [`v24.6.0`](https://github.com/nodejs/node/releases/tag/v24.6.0): 2025-08-14, Version 24.6.0 (Current), @​RafaelGSS [Compare Source](nodejs/node@v24.5.0...v24.6.0) ##### Notable Changes - \[[`471fe712b3`](nodejs/node@471fe712b3)] - **(SEMVER-MINOR)** **cli**: add NODE\_USE\_SYSTEM\_CA=1 (Joyee Cheung) [#​59276](nodejs/node#59276) - \[[`38aedfbf73`](nodejs/node@38aedfbf73)] - **(SEMVER-MINOR)** **crypto**: support ML-DSA KeyObject, sign, and verify (Filip Skokan) [#​59259](nodejs/node#59259) - \[[`201304537e`](nodejs/node@201304537e)] - **(SEMVER-MINOR)** **zlib**: add dictionary support to zstdCompress and zstdDecompress (lluisemper) [#​59240](nodejs/node#59240) - \[[`e79c93a5d0`](nodejs/node@e79c93a5d0)] - **(SEMVER-MINOR)** **http**: add server.keepAliveTimeoutBuffer option (Haram Jeong) [#​59243](nodejs/node#59243) - \[[`c144d69efc`](nodejs/node@c144d69efc)] - **lib**: docs deprecate \_http\_\* (Sebastian Beltran) [#​59293](nodejs/node#59293) - \[[`aeb4de55a7`](nodejs/node@aeb4de55a7)] - **(SEMVER-MINOR)** **fs**: port SonicBoom module to fs module as Utf8Stream (James M Snell) [#​58897](nodejs/node#58897) ##### Commits - \[[`f7484575ff`](nodejs/node@f7484575ff)] - **assert**: change utils to use index instead of for...of (방진혁) [#​59278](nodejs/node#59278) - \[[`269cd16185`](nodejs/node@269cd16185)] - **benchmark**: remove deprecated \_extend from benchmark (Rafael Gonzaga) [#​59228](nodejs/node#59228) - \[[`848e49c20b`](nodejs/node@848e49c20b)] - **benchmark**: add fs warmup to writefile-promises (Bruno Rodrigues) [#​59215](nodejs/node#59215) - \[[`8c609be1b1`](nodejs/node@8c609be1b1)] - **benchmark**: add calibrate-n script (Rafael Gonzaga) [#​59186](nodejs/node#59186) - \[[`6a3bf772d8`](nodejs/node@6a3bf772d8)] - **build**: fix node\_use\_sqlite for GN builds (Shelley Vohr) [#​59017](nodejs/node#59017) - \[[`471fe712b3`](nodejs/node@471fe712b3)] - **(SEMVER-MINOR)** **cli**: add NODE\_USE\_SYSTEM\_CA=1 (Joyee Cheung) [#​59276](nodejs/node#59276) - \[[`38aedfbf73`](nodejs/node@38aedfbf73)] - **(SEMVER-MINOR)** **crypto**: support ML-DSA KeyObject, sign, and verify (Filip Skokan) [#​59259](nodejs/node#59259) - \[[`a312e706cf`](nodejs/node@a312e706cf)] - **crypto**: prepare webcrypto key import/export for modern algorithms (Filip Skokan) [#​59284](nodejs/node#59284) - \[[`3a7c2c3a47`](nodejs/node@3a7c2c3a47)] - **deps**: update ada to 3.2.7 (Node.js GitHub Bot) [#​59336](nodejs/node#59336) - \[[`8d9ceeaf6a`](nodejs/node@8d9ceeaf6a)] - **deps**: update archs files for openssl-3.5.2 (Node.js GitHub Bot) [#​59371](nodejs/node#59371) - \[[`33b06df354`](nodejs/node@33b06df354)] - **deps**: upgrade openssl sources to openssl-3.5.2 (Node.js GitHub Bot) [#​59371](nodejs/node#59371) - \[[`fa70f1af77`](nodejs/node@fa70f1af77)] - **deps**: support madvise(3C) across ALL illumos revisions (Dan McDonald) [#​58237](nodejs/node#58237) - \[[`f834a6be59`](nodejs/node@f834a6be59)] - **deps**: update undici to 7.13.0 (Node.js GitHub Bot) [#​59338](nodejs/node#59338) - \[[`db2417487e`](nodejs/node@db2417487e)] - **deps**: update sqlite to 3.50.4 (Node.js GitHub Bot) [#​59337](nodejs/node#59337) - \[[`41978adb08`](nodejs/node@41978adb08)] - **deps**: V8: backport [`493cb53`](nodejs/node@493cb53691be) (Chengzhong Wu) [#​59238](nodejs/node#59238) - \[[`05667991ca`](nodejs/node@05667991ca)] - **deps**: V8: backport [`1c3e018`](nodejs/node@1c3e018e7d48) (Renegade334) [#​58818](nodejs/node#58818) - \[[`fd61588bb4`](nodejs/node@fd61588bb4)] - **doc**: rename x509.extKeyUsage to x509.keyUsage (Filip Skokan) [#​59332](nodejs/node#59332) - \[[`a271ae4360`](nodejs/node@a271ae4360)] - **doc**: fix Pbkdf2Params hash attribute heading (Filip Skokan) [#​59395](nodejs/node#59395) - \[[`72cfff165b`](nodejs/node@72cfff165b)] - **doc**: fix missing reference links for server.keepAliveTimeoutBuffer (Lee Jiho) [#​59356](nodejs/node#59356) - \[[`8341916772`](nodejs/node@8341916772)] - **doc**: fix grammar in global dispatcher usage (Eng Zer Jun) [#​59344](nodejs/node#59344) - \[[`e3e489706b`](nodejs/node@e3e489706b)] - **doc**: run license-builder (github-actions\[bot]) [#​59343](nodejs/node#59343) - \[[`46527e8cea`](nodejs/node@46527e8cea)] - **doc**: correct orthography `eg.` → `e.g.` (Jacob Smith) [#​59329](nodejs/node#59329) - \[[`d140c3713e`](nodejs/node@d140c3713e)] - **doc**: clarify the need of compiler compatible with c++20 (Rafael Gonzaga) [#​59297](nodejs/node#59297) - \[[`95e9cabf9d`](nodejs/node@95e9cabf9d)] - **doc**: clarify release candidate stability index (Filip Skokan) [#​59295](nodejs/node#59295) - \[[`a056dd36d2`](nodejs/node@a056dd36d2)] - **doc**: add WDYT to glossary (btea) [#​59280](nodejs/node#59280) - \[[`1e2c52f5c4`](nodejs/node@1e2c52f5c4)] - **doc**: add manpage entry for --use-system-ca (Joyee Cheung) [#​59273](nodejs/node#59273) - \[[`31a46fdeb4`](nodejs/node@31a46fdeb4)] - **doc**: add path.join and path.normalize clarification (Rafael Gonzaga) [#​59262](nodejs/node#59262) - \[[`cff3725ff9`](nodejs/node@cff3725ff9)] - **doc**: fix typo in `test/common/README.md` (Yoo) [#​59180](nodejs/node#59180) - \[[`31a9283591`](nodejs/node@31a9283591)] - **doc**: add note on process memoryUsage (fengmk2) [#​59026](nodejs/node#59026) - \[[`5a98bff6b8`](nodejs/node@5a98bff6b8)] - **doc**: format safely for `doc-kit` (Aviv Keller) [#​59229](nodejs/node#59229) - \[[`95b8b7ea5c`](nodejs/node@95b8b7ea5c)] - **domain**: remove deprecated API call (Alex Yang) [#​59339](nodejs/node#59339) - \[[`2990f178bd`](nodejs/node@2990f178bd)] - **fs**: fix glob TypeError on restricted dirs (Sylphy-0xd3ac) [#​58674](nodejs/node#58674) - \[[`e2fb4caf9c`](nodejs/node@e2fb4caf9c)] - **fs**: correct error message when FileHandle is transferred (Alex Yang) [#​59156](nodejs/node#59156) - \[[`aeb4de55a7`](nodejs/node@aeb4de55a7)] - **(SEMVER-MINOR)** **fs**: port SonicBoom module to fs module as Utf8Stream (James M Snell) [#​58897](nodejs/node#58897) - \[[`e79c93a5d0`](nodejs/node@e79c93a5d0)] - **(SEMVER-MINOR)** **http**: add server.keepAliveTimeoutBuffer option (Haram Jeong) [#​59243](nodejs/node#59243) - \[[`0fb005a53f`](nodejs/node@0fb005a53f)] - **http2**: set Http2Stream#sentHeaders for raw headers (Darshan Sen) [#​59244](nodejs/node#59244) - \[[`e055539604`](nodejs/node@e055539604)] - **lib**: add trace-sigint APIs (theanarkh) [#​59040](nodejs/node#59040) - \[[`d2183d860a`](nodejs/node@d2183d860a)] - **lib**: optimize writable stream buffer clearing (Yoo) [#​59406](nodejs/node#59406) - \[[`47543a7e17`](nodejs/node@47543a7e17)] - **lib**: handle windows reserved device names on UNC (Rafael Gonzaga) [#​59286](nodejs/node#59286) - \[[`c6911f0717`](nodejs/node@c6911f0717)] - **lib**: do not modify prototype deprecated asyncResource (RafaelGSS) [#​59195](nodejs/node#59195) - \[[`3c88b769bb`](nodejs/node@3c88b769bb)] - **lib**: restructure assert to become a class (Miguel Marcondes Filho) [#​58253](nodejs/node#58253) - \[[`e91b54df59`](nodejs/node@e91b54df59)] - **lib**: handle superscript variants on windows device (Rafael Gonzaga) [#​59261](nodejs/node#59261) - \[[`4ee467905d`](nodejs/node@4ee467905d)] - **lib**: use validateString (hotpineapple) [#​59296](nodejs/node#59296) - \[[`c144d69efc`](nodejs/node@c144d69efc)] - **lib**: docs deprecate \_http\_\* (Sebastian Beltran) [#​59293](nodejs/node#59293) - \[[`c89b67e681`](nodejs/node@c89b67e681)] - **lib**: add type names in source mapped stack traces (Chengzhong Wu) [#​58976](nodejs/node#58976) - \[[`5b2363be8d`](nodejs/node@5b2363be8d)] - **lib**: prefer AsyncIteratorPrototype primordial (René) [#​59097](nodejs/node#59097) - \[[`41b4f4d694`](nodejs/node@41b4f4d694)] - **meta**: clarify pr objection process further (James M Snell) [#​59096](nodejs/node#59096) - \[[`0eb5962f1e`](nodejs/node@0eb5962f1e)] - **meta**: add mailmap entry for aditi-1400 (Aditi) [#​59316](nodejs/node#59316) - \[[`a2b72c2304`](nodejs/node@a2b72c2304)] - **meta**: add tsc and build team as codeowners building.md (Rafael Gonzaga) [#​59298](nodejs/node#59298) - \[[`d69f3ee1e0`](nodejs/node@d69f3ee1e0)] - **meta**: add nodejs/path to path files (Rafael Gonzaga) [#​59289](nodejs/node#59289) - \[[`1e37eab865`](nodejs/node@1e37eab865)] - **node-api**: reword "implementation in an alternative VM" as implementable (Chengzhong Wu) [#​59036](nodejs/node#59036) - \[[`64add6302a`](nodejs/node@64add6302a)] - **src**: use simdjson to parse SEA configuration (Joyee Cheung) [#​59323](nodejs/node#59323) - \[[`e9c6636585`](nodejs/node@e9c6636585)] - **src**: mark realm leaf classes final (Anna Henningsen) [#​59355](nodejs/node#59355) - \[[`42ef8147d1`](nodejs/node@42ef8147d1)] - **src**: warn about FastOneByteString invalidation (James M Snell) [#​59275](nodejs/node#59275) - \[[`8686b8037a`](nodejs/node@8686b8037a)] - **src**: remove unused DSAKeyExportJob (Filip Skokan) [#​59291](nodejs/node#59291) - \[[`1e5f632666`](nodejs/node@1e5f632666)] - **src**: use C++20 `contains()` method (iknoom) [#​59304](nodejs/node#59304) - \[[`22d4683cfe`](nodejs/node@22d4683cfe)] - **src**: added CHECK\_NOT\_NULL check for multiple eq\_wrap\_async (F3lixTheCat) [#​59267](nodejs/node#59267) - \[[`6a47ff4943`](nodejs/node@6a47ff4943)] - **src**: clear all linked module caches once instantiated (Chengzhong Wu) [#​59117](nodejs/node#59117) - \[[`33728cb4ca`](nodejs/node@33728cb4ca)] - **src**: add nullptr checks in `StreamPipe::New` (Burkov Egor) [#​57613](nodejs/node#57613) - \[[`4a907bdad1`](nodejs/node@4a907bdad1)] - **src**: add percentage support to --max-old-space-size (Asaf Federman) [#​59082](nodejs/node#59082) - \[[`7c189d4f55`](nodejs/node@7c189d4f55)] - **test**: deflake sequential/test-tls-session-timeout (Joyee Cheung) [#​59423](nodejs/node#59423) - \[[`fb0a6fb57f`](nodejs/node@fb0a6fb57f)] - **test**: exclude mock from coverage (Shima Ryuhei) [#​59348](nodejs/node#59348) - \[[`7e10f95f13`](nodejs/node@7e10f95f13)] - **test**: split test-fs-cp.js (Joyee Cheung) [#​59408](nodejs/node#59408) - \[[`41bcf5f659`](nodejs/node@41bcf5f659)] - **test**: update WPT resources,WebCryptoAPI,webstorage (Filip Skokan) [#​59311](nodejs/node#59311) - \[[`f9f3dc94cb`](nodejs/node@f9f3dc94cb)] - **test**: add known issue test for fs.cpSync dereference bug (James M Snell) [#​58941](nodejs/node#58941) - \[[`244d0c38a8`](nodejs/node@244d0c38a8)] - **test**: deflake stream-readable-to-web test (Ethan Arrowood) [#​58948](nodejs/node#58948) - \[[`564e604a1a`](nodejs/node@564e604a1a)] - **test**: make test-inspector-network-resource sequential (Shima Ryuhei) [#​59104](nodejs/node#59104) - \[[`7ab13b7477`](nodejs/node@7ab13b7477)] - **test**: don't use expose internals in test-http-outgoing-buffer.js (Meghan Denny) [#​59219](nodejs/node#59219) - \[[`319df3859a`](nodejs/node@319df3859a)] - **test,crypto**: skip unsupported ciphers (Shelley Vohr) [#​59388](nodejs/node#59388) - \[[`713c70c32a`](nodejs/node@713c70c32a)] - **test\_runner**: remove unused callee convertion (Alex Yang) [#​59221](nodejs/node#59221) - \[[`e4ca30e115`](nodejs/node@e4ca30e115)] - **tools**: disable nullability-completeness warnings (Michaël Zasso) [#​59392](nodejs/node#59392) - \[[`dab7f6b542`](nodejs/node@dab7f6b542)] - **tools**: check for std::vector\<v8::Local> in lint (Aditi) [#​58497](nodejs/node#58497) - \[[`7b94982eb0`](nodejs/node@7b94982eb0)] - **tools**: allow selecting test subsystems with numbers in their names (Darshan Sen) [#​59242](nodejs/node#59242) - \[[`16bbcd8881`](nodejs/node@16bbcd8881)] - **typings**: improve internal binding types (Nam Yooseong) [#​59351](nodejs/node#59351) - \[[`76bc4d659b`](nodejs/node@76bc4d659b)] - **typings**: improve internal binding types (Michaël Zasso) [#​59176](nodejs/node#59176) - \[[`eecd3272a6`](nodejs/node@eecd3272a6)] - **worker**: add name for worker (theanarkh) [#​59213](nodejs/node#59213) - \[[`84c3513ce2`](nodejs/node@84c3513ce2)] - **worker**: implements nits in Web Locks code (Antoine du Hamel) [#​59270](nodejs/node#59270) - \[[`bd68fbd753`](nodejs/node@bd68fbd753)] - **worker**: add cpuUsage for worker (theanarkh) [#​59177](nodejs/node#59177) - \[[`201304537e`](nodejs/node@201304537e)] - **(SEMVER-MINOR)** **zlib**: add dictionary support to zstdCompress and zstdDecompress (lluisemper) [#​59240](nodejs/node#59240) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS43MS4wIiwidXBkYXRlZEluVmVyIjoiNDEuNzEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Original commit message: [api] Add index-based module resolution in InstantiateModule() Add new InstantiateModule() overload that allows embedders to identify modules requests by index in the module requests array rather than using specifier and import attributes. When embedders want to fetch all the modules using information from module->GetModuleRequests() before calling InstantiateModule() instead of having to do the fetching inside the InstantiateModule() callback, they could just maintain a simple array of modules indexed by module request positions and look up the fetched the module by index in the new callback. Previously this has to be done by mapping from specifier and import attributes to module objects cached on the embedder side, leading to an overhead to hash the specifier and import attributes for each module request. Refs: nodejs#59117 Bug: 435317398 Change-Id: Ie017d2f3ccc605e0f58aa423504b5fa5fdbcc633 Refs: v8/v8@1db0e6b Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
Original commit message: [api] Add index-based module resolution in InstantiateModule() Add new InstantiateModule() overload that allows embedders to identify modules requests by index in the module requests array rather than using specifier and import attributes. When embedders want to fetch all the modules using information from module->GetModuleRequests() before calling InstantiateModule() instead of having to do the fetching inside the InstantiateModule() callback, they could just maintain a simple array of modules indexed by module request positions and look up the fetched the module by index in the new callback. Previously this has to be done by mapping from specifier and import attributes to module objects cached on the embedder side, leading to an overhead to hash the specifier and import attributes for each module request. Refs: nodejs#59117 Bug: 435317398 Change-Id: Ie017d2f3ccc605e0f58aa423504b5fa5fdbcc633 Refs: v8/v8@46d7bc7 Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
This doesn't land cleanly on v22.x-staging so a manual backport will be necessary if this is to be released there. |
There are two phases in module linking: link, and instantiate. These
two operations are required to be separated to allow cyclic
dependencies.
v8::Module::InstantiateModule
is only required to be invoked on theroot module. The global references created by
ModuleWrap::Link
areonly cleared at
ModuleWrap::Instantiate
. So the global referencescreated for depended modules are usually not cleared because
ModuleWrap::Instantiate
is not invoked for each of depended modules,and caused memory leak.
The change references the linked modules in an object internal slot.
This is not an issue for Node.js ESM support as these modules can not be
off-loaded. However, this could be outstanding for
vm.Module
.Fixes: #50113