module: rework of memory management in vm APIs with the importModuleDynamically option

lib/internal/modules/esm/create_dynamic_module.js

@@ -45,8 +45,9 @@ import.meta.done();
 
   if (imports.length)
     reflect.imports = { __proto__: null };
-  const { setCallbackForWrap } = require('internal/modules/esm/utils');
-  setCallbackForWrap(m, {
+  const { registerModule } = require('internal/modules/esm/utils');
+  registerModule(m, {
+    __proto__: null,
     initializeImportMeta: (meta, wrap) => {
       meta.exports = reflect.exports;
       if (reflect.imports)

lib/internal/modules/esm/loader.js

@@ -180,9 +180,10 @@ class ModuleLoader {
   ) {
     const evalInstance = (url) => {
       const { ModuleWrap } = internalBinding('module_wrap');
-      const { setCallbackForWrap } = require('internal/modules/esm/utils');
+      const { registerModule } = require('internal/modules/esm/utils');
       const module = new ModuleWrap(url, undefined, source, 0, 0);
-      setCallbackForWrap(module, {
+      registerModule(module, {
+        __proto__: null,
         initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
         importModuleDynamically: (specifier, { url }, importAssertions) => {
           return this.import(specifier, url, importAssertions);

lib/internal/modules/esm/translators.js

@@ -116,8 +116,9 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
   maybeCacheSourceMap(url, source);
   debug(`Translating StandardModule ${url}`);
   const module = new ModuleWrap(url, undefined, source, 0, 0);
-  const { setCallbackForWrap } = require('internal/modules/esm/utils');
-  setCallbackForWrap(module, {
+  const { registerModule } = require('internal/modules/esm/utils');
+  registerModule(module, {
+    __proto__: null,
     initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
     importModuleDynamically,
   });

lib/internal/modules/esm/utils.js

@@ -7,6 +7,11 @@ const {
   ObjectFreeze,
 } = primordials;
 
+const {
+  privateSymbols: {
+    host_defined_option_symbol,
+  },
+} = internalBinding('util');
 const {
   ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
   ERR_INVALID_ARG_VALUE,
@@ -21,16 +26,8 @@ const {
   setImportModuleDynamicallyCallback,
   setInitializeImportMetaObjectCallback,
 } = internalBinding('module_wrap');
-const {
-  getModuleFromWrap,
-} = require('internal/vm/module');
 const assert = require('internal/assert');
 
-const callbackMap = new SafeWeakMap();
-function setCallbackForWrap(wrap, data) {
-  callbackMap.set(wrap, data);
-}
-
 let defaultConditions;
 function getDefaultConditions() {
   assert(defaultConditions !== undefined);
@@ -73,21 +70,75 @@ function getConditionsSet(conditions) {
   return getDefaultConditionsSet();
 }
 
-function initializeImportMetaObject(wrap, meta) {
-  if (callbackMap.has(wrap)) {
-    const { initializeImportMeta } = callbackMap.get(wrap);
+/**
+ * @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 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 referrer object alive, which would be
+ * passed into the callbacks.
+ * The reference goes like this:
+ * [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(referrer, registry) {
+  const idSymbol = referrer[host_defined_option_symbol];
+  // To prevent it from being GC'ed.
+  registry.callbackReferrer ??= referrer;
+  moduleRegistries.set(idSymbol, registry);
+}
+
+// The native callback
+function initializeImportMetaObject(symbol, meta) {
+  if (moduleRegistries.has(symbol)) {
+    const { initializeImportMeta, callbackReferrer } = moduleRegistries.get(symbol);
     if (initializeImportMeta !== undefined) {
-      meta = initializeImportMeta(meta, getModuleFromWrap(wrap) || wrap);
+      meta = initializeImportMeta(meta, callbackReferrer);
     }
   }
 }
 
-async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
-  if (callbackMap.has(wrap)) {
-    const { importModuleDynamically } = callbackMap.get(wrap);
+// The native callback
+async function importModuleDynamicallyCallback(symbol, specifier, assertions) {
+  if (moduleRegistries.has(symbol)) {
+    const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(symbol);
     if (importModuleDynamically !== undefined) {
-      return importModuleDynamically(
-        specifier, getModuleFromWrap(wrap) || wrap, assertions);
+      return importModuleDynamically(specifier, callbackReferrer, assertions);
     }
   }
   throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
@@ -149,7 +200,7 @@ async function initializeHooks() {
 }
 
 module.exports = {
-  setCallbackForWrap,
+  registerModule,
   initializeESM,
   initializeHooks,
   getDefaultConditions,

lib/internal/vm.js

@@ -100,9 +100,10 @@ function internalCompileFunction(code, params, options) {
     const { importModuleDynamicallyWrap } = require('internal/vm/module');
     const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
     const func = result.function;
-    const { setCallbackForWrap } = require('internal/modules/esm/utils');
-    setCallbackForWrap(result.cacheKey, {
-      importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
+    const { registerModule } = require('internal/modules/esm/utils');
+    registerModule(func, {
+      __proto__: null,
+      importModuleDynamically: wrapped,
     });
   }
 

lib/internal/vm/module.js

@@ -11,7 +11,6 @@ const {
   ObjectSetPrototypeOf,
   ReflectApply,
   SafePromiseAllReturnVoid,
-  SafeWeakMap,
   Symbol,
   SymbolToStringTag,
   TypeError,
@@ -69,7 +68,6 @@ const STATUS_MAP = {
 
 let globalModuleId = 0;
 const defaultModuleName = 'vm:module';
-const wrapToModuleMap = new SafeWeakMap();
 
 const kWrap = Symbol('kWrap');
 const kContext = Symbol('kContext');
@@ -120,25 +118,30 @@ class Module {
       });
     }
 
+    let registry = { __proto__: null };
     if (sourceText !== undefined) {
       this[kWrap] = new ModuleWrap(identifier, context, sourceText,
                                    options.lineOffset, options.columnOffset,
                                    options.cachedData);
-      const { setCallbackForWrap } = require('internal/modules/esm/utils');
-      setCallbackForWrap(this[kWrap], {
+      registry = {
+        __proto__: null,
         initializeImportMeta: options.initializeImportMeta,
         importModuleDynamically: options.importModuleDynamically ?
           importModuleDynamicallyWrap(options.importModuleDynamically) :
           undefined,
-      });
+      };
     } else {
       assert(syntheticEvaluationSteps);
       this[kWrap] = new ModuleWrap(identifier, context,
                                    syntheticExportNames,
                                    syntheticEvaluationSteps);
     }
 
-    wrapToModuleMap.set(this[kWrap], this);
+    // This will take precedence over the referrer as the object being
+    // passed into the callbacks.
+    registry.callbackReferrer = this;
+    const { registerModule } = require('internal/modules/esm/utils');
+    registerModule(this[kWrap], registry);
 
     this[kContext] = context;
   }
@@ -445,5 +448,4 @@ module.exports = {
   SourceTextModule,
   SyntheticModule,
   importModuleDynamicallyWrap,
-  getModuleFromWrap: (wrap) => wrapToModuleMap.get(wrap),
 };

lib/vm.js

@@ -105,8 +105,9 @@ class Script extends ContextifyScript {
       validateFunction(importModuleDynamically,
                        'options.importModuleDynamically');
       const { importModuleDynamicallyWrap } = require('internal/vm/module');
-      const { setCallbackForWrap } = require('internal/modules/esm/utils');
-      setCallbackForWrap(this, {
+      const { registerModule } = require('internal/modules/esm/utils');
+      registerModule(this, {
+        __proto__: null,
         importModuleDynamically:
           importModuleDynamicallyWrap(importModuleDynamically),
       });

src/env-inl.h

@@ -393,16 +393,6 @@ inline AliasedInt32Array& Environment::stream_base_state() {
   return stream_base_state_;
 }
 
-inline uint32_t Environment::get_next_module_id() {
-  return module_id_counter_++;
-}
-inline uint32_t Environment::get_next_script_id() {
-  return script_id_counter_++;
-}
-inline uint32_t Environment::get_next_function_id() {
-  return function_id_counter_++;
-}
-
 ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
     Environment* env)
     : env_(env) {

src/env.h

@@ -746,14 +746,6 @@ class Environment : public MemoryRetainer {
   builtins::BuiltinLoader* builtin_loader();
 
   std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
-  std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
-  std::unordered_map<uint32_t, contextify::ContextifyScript*>
-      id_to_script_map;
-  std::unordered_map<uint32_t, contextify::CompiledFnEntry*> id_to_function_map;
-
-  inline uint32_t get_next_module_id();
-  inline uint32_t get_next_script_id();
-  inline uint32_t get_next_function_id();
 
   EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; }
 

src/env_properties.h

@@ -22,6 +22,7 @@
   V(contextify_context_private_symbol, "node:contextify:context")              \
   V(decorated_private_symbol, "node:decorated")                                \
   V(transfer_mode_private_symbol, "node:transfer_mode")                        \
+  V(host_defined_option_symbol, "node:host_defined_option_symbol")             \
   V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper")    \
   V(napi_type_tag, "node:napi:type_tag")                                       \
   V(napi_wrapper, "node:napi:wrapper")                                         \
@@ -342,7 +343,6 @@
   V(blocklist_constructor_template, v8::FunctionTemplate)                      \
   V(contextify_global_template, v8::ObjectTemplate)                            \
   V(contextify_wrapper_template, v8::ObjectTemplate)                           \
-  V(compiled_fn_entry_template, v8::ObjectTemplate)                            \
   V(crypto_key_object_handle_constructor, v8::FunctionTemplate)                \
   V(env_proxy_template, v8::ObjectTemplate)                                    \
   V(env_proxy_ctor_template, v8::FunctionTemplate)                             \