lib/internal/modules/esm/utils.js

@@ -68,47 +68,75 @@ function getConditionsSet(conditions) {
   return getDefaultConditionsSet();
 }
 
-const moduleMap = new SafeWeakMap();
+/**
+ * @callback ImportModuleDynamicallyCallback
+ * @param {string} specifier
+ * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer
+ * @param {object} assertions
+ * @returns { Promise<void> }
+ */
+
+/**
+ * @callback InitializeImportMetaCallback
+ * @param {object} meta
+ * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer
+ */
+
+/**
+ * @typedef {{
+ *   callbackReferrer: ModuleWrap|ContextifyScript|Function|vm.Module
+ *   initializeImportMeta? : InitializeImportMetaCallback,
+ *   importModuleDynamically? : ImportModuleDynamicallyCallback
+ * }} ModuleRegistry
+ */
+
+/**
+ * @type {WeakMap<symbol, ModuleRegistry>}
+ */
+const moduleRegistries = new SafeWeakMap();
 
 /**
  * V8 would make sure that as long as import() can still be initiated from
- * the referer, the symbol referenced by |host_defined_option_symbol| should
+ * the referrer, the symbol referenced by |host_defined_option_symbol| should
  * be alive, which in term would keep the settings object alive through the
- * WeakMap, and in turn that keeps the referer object alive, which would be
+ * WeakMap, and in turn that keeps the referrer object alive, which would be
  * passed into the callbacks.
  * The reference goes like this:
- *   [v8::internal::Script] (via host defined options) ----1--> [id_symbol]
- *   [referer wrap] (via host_defined_option_symbol) --------2------^  |
- *                                  ^---------------3-------------------
- *   1+3 makes sure that as long as import() can still be initiated, the
- *   referer wrap is still around and can be passed into the callbacks.
- *   2 is only there so that we can get the id symbol to configure the
- *   weak map.
- * @param {ModuleWrap|ContextifyScript|Function} referer
- * @param {object} settings
+ * [v8::internal::Script] (via host defined options) ----1--> [idSymbol]
+ * [callbackReferrer] (via host_defined_option_symbol) ------2------^  |
+ *                                 ^----------3---- (via WeakMap)------
+ * 1+3 makes sure that as long as import() can still be initiated, the
+ * referrer wrap is still around and can be passed into the callbacks.
+ * 2 is only there so that we can get the id symbol to configure the
+ * weak map.
+ * @param {ModuleWrap|ContextifyScript|Function} referrer The referrer to
+ *   get the id symbol from. This is different from callbackReferrer which
+ *   could be set by the caller.
+ * @param {ModuleRegistry} registry
  */
-function registerModule(referer, settings) {
-  const id_symbol = referer[host_defined_option_symbol];
-  settings.referer = referer;  // To prevent it from being GC'ed.
-  moduleMap.set(id_symbol, settings);
+function registerModule(referrer, registry) {
+  const idSymbol = referrer[host_defined_option_symbol];
+  // To prevent it from being GC'ed. If
+  registry.callbackReferrer ??= referrer;
+  moduleRegistries.set(idSymbol, registry);
 }
 
 // The native callback
 function initializeImportMetaObject(symbol, meta) {
-  if (moduleMap.has(symbol)) {
-    const { initializeImportMeta, referer, vmReferer } = moduleMap.get(symbol);
+  if (moduleRegistries.has(symbol)) {
+    const { initializeImportMeta, callbackReferrer } = moduleRegistries.get(symbol);
     if (initializeImportMeta !== undefined) {
-      meta = initializeImportMeta(meta, vmReferer || referer);
+      meta = initializeImportMeta(meta, callbackReferrer);
     }
   }
 }
 
 // The native callback
 async function importModuleDynamicallyCallback(symbol, specifier, assertions) {
-  if (moduleMap.has(symbol)) {
-    const { importModuleDynamically, referer, vmReferer } = moduleMap.get(symbol);
+  if (moduleRegistries.has(symbol)) {
+    const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(symbol);
     if (importModuleDynamically !== undefined) {
-      return importModuleDynamically(specifier, vmReferer || referer, assertions);
+      return importModuleDynamically(specifier, callbackReferrer, assertions);
     }
   }
   throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();

lib/internal/vm/module.js

@@ -118,12 +118,12 @@ class Module {
       });
     }
 
-    let settings = { __proto__: null };
+    let registry = { __proto__: null };
     if (sourceText !== undefined) {
       this[kWrap] = new ModuleWrap(identifier, context, sourceText,
                                    options.lineOffset, options.columnOffset,
                                    options.cachedData);
-      settings = {
+      registry = {
         initializeImportMeta: options.initializeImportMeta,
         importModuleDynamically: options.importModuleDynamically ?
           importModuleDynamicallyWrap(options.importModuleDynamically) :
@@ -138,9 +138,9 @@ class Module {
 
     // This will take precedence over the referrer as the object being
     // passed into the callbacks.
-    settings.vmReferer = this;
+    registry.callbackReferrer = this;
     const { registerModule } = require('internal/modules/esm/utils');
-    registerModule(this[kWrap], settings);
+    registerModule(this[kWrap], registry);
 
     this[kContext] = context;
   }

test/known_issues/test-vm-source-text-module-leak.js

@@ -1,9 +1,9 @@
 'use strict';
 
 // Flags: --experimental-vm-modules --max-heap-size=20
-// This tests that using vm.compileFunction() with vm.SyntheticModule in
-// the dynamic import callback does not leak.
-// See https://github.com/nodejs/node/issues/44211
+// This leaks because of the strong persistent v8::Module references
+// from ModuleWrap. We need replace that with a GC-aware ModuleWrap ->
+// v8::Module reference to fix this.
 
 require('../common');
 const vm = require('vm');