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_;
 }
 
-inlineuint32_tEnvironment::get_next_module_id(){
-return module_id_counter_++;
-}
-inlineuint32_tEnvironment::get_next_script_id(){
-return script_id_counter_++;
-}
-inlineuint32_tEnvironment::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;
-
-inlineuint32_tget_next_module_id();
-inlineuint32_tget_next_script_id();
-inlineuint32_tget_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) \