module: initialize module_wrap.callbackMap during pre-execution

joyeecheung · targos · commit 4dfe54a89ac8 · 2019-04-27T11:36:04.000+02:00
Since the bootstrap does not actually use ESM at all, there is no need to create this map so early. This patch moves the initialization of the map to pre-execution, so that the only binding loaded in loaders is native_module. In addition, switch to SafeWeakMap. PR-URL: #27323 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -135,9 +135,6 @@ let internalBinding;
   };
 }
 
-// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
-internalBinding('module_wrap').callbackMap = new WeakMap();
-
 // Think of this as module.exports in this file even though it is not
 // written in CommonJS style.
 const loaderExports = {
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const { Object } = primordials;
+const { Object, SafeWeakMap } = primordials;
 
 const { getOptionValue } = require('internal/options');
 const { Buffer } = require('buffer');
@@ -342,6 +342,9 @@ function initializeCJSLoader() {
 }
 
 function initializeESMLoader() {
+  // Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
+  internalBinding('module_wrap').callbackMap = new SafeWeakMap();
+
   const experimentalModules = getOptionValue('--experimental-modules');
   const experimentalVMModules = getOptionValue('--experimental-vm-modules');
   if (experimentalModules || experimentalVMModules) {
diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js
@@ -2,7 +2,6 @@
 
 const { ArrayPrototype } = primordials;
 
-const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
 const debug = require('internal/util/debuglog').debuglog('esm');
 
 const createDynamicModule = (exports, url = '', evaluate) => {
@@ -21,7 +20,7 @@ import.meta.exports.${name} = {
 
 import.meta.done();
 `;
-
+  const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
   const m = new ModuleWrap(source, `${url}`);
   m.link(() => 0);
   m.instantiate();
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -7,7 +7,6 @@ const {
 } = primordials;
 
 const { NativeModule } = require('internal/bootstrap/loaders');
-const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
 const {
   stripShebang,
   stripBOM
@@ -45,6 +44,7 @@ async function importModuleDynamically(specifier, { url }) {
 translators.set('module', async function moduleStrategy(url) {
   const source = `${await readFileAsync(new URL(url))}`;
   debug(`Translating StandardModule ${url}`);
+  const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
   const module = new ModuleWrap(stripShebang(source), url);
   callbackMap.set(module, {
     initializeImportMeta,
diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
@@ -1,8 +1,5 @@
 'use strict';
 
-const {
-  callbackMap,
-} = internalBinding('module_wrap');
 const {
   ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
 } = require('internal/errors').codes;
@@ -14,6 +11,7 @@ const {
 } = require('internal/vm/source_text_module');
 
 exports.initializeImportMetaObject = function(wrap, meta) {
+  const { callbackMap } = internalBinding('module_wrap');
   if (callbackMap.has(wrap)) {
     const { initializeImportMeta } = callbackMap.get(wrap);
     if (initializeImportMeta !== undefined) {
@@ -23,6 +21,7 @@ exports.initializeImportMetaObject = function(wrap, meta) {
 };
 
 exports.importModuleDynamicallyCallback = async function(wrap, specifier) {
+  const { callbackMap } = internalBinding('module_wrap');
   if (callbackMap.has(wrap)) {
     const { importModuleDynamically } = callbackMap.get(wrap);
     if (importModuleDynamically !== undefined) {
diff --git a/lib/internal/vm/source_text_module.js b/lib/internal/vm/source_text_module.js
@@ -25,16 +25,16 @@ const {
   validateString
 } = require('internal/validators');
 
+const binding = internalBinding('module_wrap');
 const {
   ModuleWrap,
-  callbackMap,
   kUninstantiated,
   kInstantiating,
   kInstantiated,
   kEvaluating,
   kEvaluated,
   kErrored,
-} = internalBinding('module_wrap');
+} = binding;
 
 const STATUS_MAP = {
   [kUninstantiated]: 'uninstantiated',
@@ -116,7 +116,7 @@ class SourceTextModule {
     linkingStatusMap.set(this, 'unlinked');
     wrapToModuleMap.set(wrap, this);
 
-    callbackMap.set(wrap, {
+    binding.callbackMap.set(wrap, {
       initializeImportMeta,
       importModuleDynamically: importModuleDynamically ? async (...args) => {
         const m = await importModuleDynamically(...args);
diff --git a/lib/vm.js b/lib/vm.js
@@ -29,7 +29,6 @@ const {
   isContext: _isContext,
   compileFunction: _compileFunction
 } = internalBinding('contextify');
-const { callbackMap } = internalBinding('module_wrap');
 const {
   ERR_INVALID_ARG_TYPE,
   ERR_VM_MODULE_NOT_MODULE,
@@ -100,6 +99,7 @@ class Script extends ContextifyScript {
       }
       const { wrapMap, linkingStatusMap } =
         require('internal/vm/source_text_module');
+      const { callbackMap } = internalBinding('module_wrap');
       callbackMap.set(this, { importModuleDynamically: async (...args) => {
         const m = await importModuleDynamically(...args);
         if (isModuleNamespaceObject(m)) {

Original file line number	Diff line number	Diff line change
`@@ -135,9 +135,6 @@ let internalBinding;`
`135`	`135`	`};`
`136`	`136`	`}`
`137`	`137`
`138`		`-// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.`
`139`		`-internalBinding('module_wrap').callbackMap = new WeakMap();`
`140`		`-`
`141`	`138`	`// Think of this as module.exports in this file even though it is not`
`142`	`139`	`// written in CommonJS style.`
`143`	`140`	`const loaderExports = {`