Skip to content

Commit

Permalink
module: initialize module_wrap.callbackMap during pre-execution
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
joyeecheung authored and targos committed Apr 27, 2019
1 parent dd709fc commit 4dfe54a
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 14 deletions.
3 changes: 0 additions & 3 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const { Object } = primordials;
const { Object, SafeWeakMap } = primordials;

const { getOptionValue } = require('internal/options');
const { Buffer } = require('buffer');
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/create_dynamic_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const {
} = primordials;

const { NativeModule } = require('internal/bootstrap/loaders');
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
const {
stripShebang,
stripBOM
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/process/esm_loader.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
'use strict';

const {
callbackMap,
} = internalBinding('module_wrap');
const {
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
} = require('internal/errors').codes;
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/vm/source_text_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)) {
Expand Down

0 comments on commit 4dfe54a

Please sign in to comment.