From 5e7946fe79193633e9a78aae48548635cbc11023 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Mon, 9 Sep 2019 12:20:16 -0500 Subject: vm: refactor SourceTextModule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removes redundant `instantiate` method - Refactors `link` to match the spec linking steps more accurately - Removes URL validation from SourceTextModule specifiers - DRYs some dynamic import logic Closes: https://github.com/nodejs/node/issues/29030 Co-Authored-By: Michaël Zasso PR-URL: https://github.com/nodejs/node/pull/29776 Reviewed-By: James M Snell Reviewed-By: Michaël Zasso Reviewed-By: Minwoo Jung --- lib/internal/errors.js | 2 - lib/internal/modules/esm/loader.js | 4 +- lib/internal/vm/source_text_module.js | 284 ++++++++++++++++++++-------------- lib/vm.js | 20 +-- 4 files changed, 179 insertions(+), 131 deletions(-) (limited to 'lib') diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 53f448cf2f..ab874cd533 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1211,8 +1211,6 @@ E('ERR_VM_MODULE_DIFFERENT_CONTEXT', 'Linked modules must use the same context', Error); E('ERR_VM_MODULE_LINKING_ERRORED', 'Linking has already failed for the provided module', Error); -E('ERR_VM_MODULE_NOT_LINKED', - 'Module must be linked before it can be instantiated', Error); E('ERR_VM_MODULE_NOT_MODULE', 'Provided module is not an instance of Module', Error); E('ERR_VM_MODULE_STATUS', 'Module status %s', Error); diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 09109d3c71..9800e8a550 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -127,7 +127,7 @@ class Loader { this.moduleMap.set(url, job); const { module, result } = await job.run(); return { - namespace: module.namespace(), + namespace: module.getNamespace(), result }; } @@ -135,7 +135,7 @@ class Loader { async import(specifier, parent) { const job = await this.getModuleJob(specifier, parent); const { module } = await job.run(); - return module.namespace(); + return module.getNamespace(); } hook({ resolve, dynamicInstantiate }) { diff --git a/lib/internal/vm/source_text_module.js b/lib/internal/vm/source_text_module.js index 881c298ce5..61af2e7a29 100644 --- a/lib/internal/vm/source_text_module.js +++ b/lib/internal/vm/source_text_module.js @@ -3,14 +3,12 @@ const { Object, SafePromise } = primordials; const { isModuleNamespaceObject } = require('internal/util/types'); -const { URL } = require('internal/url'); const { isContext } = internalBinding('contextify'); const { ERR_INVALID_ARG_TYPE, ERR_VM_MODULE_ALREADY_LINKED, ERR_VM_MODULE_DIFFERENT_CONTEXT, ERR_VM_MODULE_LINKING_ERRORED, - ERR_VM_MODULE_NOT_LINKED, ERR_VM_MODULE_NOT_MODULE, ERR_VM_MODULE_STATUS, } = require('internal/errors').codes; @@ -22,7 +20,7 @@ const { const { validateInt32, validateUint32, - validateString + validateString, } = require('internal/validators'); const binding = internalBinding('module_wrap'); @@ -37,29 +35,33 @@ const { } = binding; const STATUS_MAP = { - [kUninstantiated]: 'uninstantiated', - [kInstantiating]: 'instantiating', - [kInstantiated]: 'instantiated', + [kUninstantiated]: 'unlinked', + [kInstantiating]: 'linking', + [kInstantiated]: 'linked', [kEvaluating]: 'evaluating', [kEvaluated]: 'evaluated', [kErrored]: 'errored', }; let globalModuleId = 0; +const defaultModuleName = 'vm:module'; const perContextModuleId = new WeakMap(); -const wrapMap = new WeakMap(); -const dependencyCacheMap = new WeakMap(); -const linkingStatusMap = new WeakMap(); -// ModuleWrap -> vm.SourceTextModule const wrapToModuleMap = new WeakMap(); -const defaultModuleName = 'vm:module'; -// TODO(devsnek): figure out AbstractModule class or protocol +const kNoError = Symbol('kNoError'); + class SourceTextModule { - constructor(src, options = {}) { + #wrap; + #identifier; + #context; + #dependencySpecifiers; + #statusOverride; + #error = kNoError; + + constructor(source, options = {}) { emitExperimentalWarning('vm.SourceTextModule'); - validateString(src, 'src'); + validateString(source, 'source'); if (typeof options !== 'object' || options === null) throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); @@ -81,21 +83,6 @@ class SourceTextModule { } } - let { url } = options; - if (url !== undefined) { - validateString(url, 'options.url'); - url = new URL(url).href; - } else if (context === undefined) { - url = `${defaultModuleName}(${globalModuleId++})`; - } else if (perContextModuleId.has(context)) { - const curId = perContextModuleId.get(context); - url = `${defaultModuleName}(${curId})`; - perContextModuleId.set(context, curId + 1); - } else { - url = `${defaultModuleName}(0)`; - perContextModuleId.set(context, 1); - } - validateInt32(lineOffset, 'options.lineOffset'); validateInt32(columnOffset, 'options.columnOffset'); @@ -111,114 +98,167 @@ class SourceTextModule { 'options.importModuleDynamically', 'function', importModuleDynamically); } - const wrap = new ModuleWrap(src, url, context, lineOffset, columnOffset); - wrapMap.set(this, wrap); - linkingStatusMap.set(this, 'unlinked'); - wrapToModuleMap.set(wrap, this); + let { identifier } = options; + if (identifier !== undefined) { + validateString(identifier, 'options.identifier'); + } else if (context === undefined) { + identifier = `${defaultModuleName}(${globalModuleId++})`; + } else if (perContextModuleId.has(context)) { + const curId = perContextModuleId.get(context); + identifier = `${defaultModuleName}(${curId})`; + perContextModuleId.set(context, curId + 1); + } else { + identifier = `${defaultModuleName}(0)`; + perContextModuleId.set(context, 1); + } - binding.callbackMap.set(wrap, { + this.#wrap = new ModuleWrap( + source, identifier, context, + lineOffset, columnOffset, + ); + wrapToModuleMap.set(this.#wrap, this); + this.#identifier = identifier; + this.#context = context; + + binding.callbackMap.set(this.#wrap, { initializeImportMeta, - importModuleDynamically: importModuleDynamically ? async (...args) => { - const m = await importModuleDynamically(...args); - if (isModuleNamespaceObject(m)) { - return m; - } - if (!m || !wrapMap.has(m)) - throw new ERR_VM_MODULE_NOT_MODULE(); - const childLinkingStatus = linkingStatusMap.get(m); - if (childLinkingStatus === 'errored') - throw m.error; - return m.namespace; - } : undefined, + importModuleDynamically: importModuleDynamically ? + importModuleDynamicallyWrap(importModuleDynamically) : + undefined, }); + } - Object.defineProperties(this, { - url: { value: url, enumerable: true }, - context: { value: context, enumerable: true }, - }); + get status() { + try { + this.#error; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (this.#error !== kNoError) { + return 'errored'; + } + if (this.#statusOverride) { + return this.#statusOverride; + } + return STATUS_MAP[this.#wrap.getStatus()]; } - get linkingStatus() { - return linkingStatusMap.get(this); + get identifier() { + try { + return this.#identifier; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } } - get status() { - return STATUS_MAP[wrapMap.get(this).getStatus()]; + get context() { + try { + return this.#context; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } } get namespace() { - const wrap = wrapMap.get(this); - if (wrap.getStatus() < kInstantiated) - throw new ERR_VM_MODULE_STATUS( - 'must not be uninstantiated or instantiating' - ); - return wrap.namespace(); + try { + this.#wrap; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (this.#wrap.getStatus() < kInstantiated) { + throw new ERR_VM_MODULE_STATUS('must not be unlinked or linking'); + } + return this.#wrap.getNamespace(); } get dependencySpecifiers() { - let deps = dependencyCacheMap.get(this); - if (deps !== undefined) - return deps; - - deps = wrapMap.get(this).getStaticDependencySpecifiers(); - Object.freeze(deps); - dependencyCacheMap.set(this, deps); - return deps; + try { + this.#dependencySpecifiers; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (this.#dependencySpecifiers === undefined) { + this.#dependencySpecifiers = this.#wrap.getStaticDependencySpecifiers(); + } + return this.#dependencySpecifiers; } get error() { - const wrap = wrapMap.get(this); - if (wrap.getStatus() !== kErrored) + try { + this.#error; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (this.#error !== kNoError) { + return this.#error; + } + if (this.#wrap.getStatus() !== kErrored) { throw new ERR_VM_MODULE_STATUS('must be errored'); - return wrap.getError(); + } + return this.#wrap.getError(); } async link(linker) { - if (typeof linker !== 'function') + try { + this.#link; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + + if (typeof linker !== 'function') { throw new ERR_INVALID_ARG_TYPE('linker', 'function', linker); - if (linkingStatusMap.get(this) !== 'unlinked') + } + if (this.status !== 'unlinked') { throw new ERR_VM_MODULE_ALREADY_LINKED(); - const wrap = wrapMap.get(this); - if (wrap.getStatus() !== kUninstantiated) - throw new ERR_VM_MODULE_STATUS('must be uninstantiated'); + } - linkingStatusMap.set(this, 'linking'); + await this.#link(linker); - const promises = wrap.link(async (specifier) => { - const m = await linker(specifier, this); - if (!m || !wrapMap.has(m)) + this.#wrap.instantiate(); + } + + #link = async function(linker) { + this.#statusOverride = 'linking'; + + const promises = this.#wrap.link(async (identifier) => { + const module = await linker(identifier, this); + try { + module.#wrap; + } catch { throw new ERR_VM_MODULE_NOT_MODULE(); - if (m.context !== this.context) + } + if (module.context !== this.context) { throw new ERR_VM_MODULE_DIFFERENT_CONTEXT(); - const childLinkingStatus = linkingStatusMap.get(m); - if (childLinkingStatus === 'errored') + } + if (module.status === 'errored') { throw new ERR_VM_MODULE_LINKING_ERRORED(); - if (childLinkingStatus === 'unlinked') - await m.link(linker); - return wrapMap.get(m); + } + if (module.status === 'unlinked') { + await module.#link(linker); + } + return module.#wrap; }); try { - if (promises !== undefined) + if (promises !== undefined) { await SafePromise.all(promises); - linkingStatusMap.set(this, 'linked'); - } catch (err) { - linkingStatusMap.set(this, 'errored'); - throw err; + } + } catch (e) { + this.#error = e; + throw e; + } finally { + this.#statusOverride = undefined; } - } + }; - instantiate() { - const wrap = wrapMap.get(this); - const status = wrap.getStatus(); - if (status === kInstantiating || status === kEvaluating) - throw new ERR_VM_MODULE_STATUS('must not be instantiating or evaluating'); - if (linkingStatusMap.get(this) !== 'linked') - throw new ERR_VM_MODULE_NOT_LINKED(); - wrap.instantiate(); - } async evaluate(options = {}) { + try { + this.#wrap; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (typeof options !== 'object' || options === null) { throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); } @@ -229,24 +269,41 @@ class SourceTextModule { } else { validateUint32(timeout, 'options.timeout', true); } - const { breakOnSigint = false } = options; if (typeof breakOnSigint !== 'boolean') { throw new ERR_INVALID_ARG_TYPE('options.breakOnSigint', 'boolean', breakOnSigint); } - - const wrap = wrapMap.get(this); - const status = wrap.getStatus(); + const status = this.#wrap.getStatus(); if (status !== kInstantiated && status !== kEvaluated && status !== kErrored) { throw new ERR_VM_MODULE_STATUS( - 'must be one of instantiated, evaluated, and errored' + 'must be one of linked, evaluated, or errored' ); } - const result = wrap.evaluate(timeout, breakOnSigint); - return { result, __proto__: null }; + const result = this.#wrap.evaluate(timeout, breakOnSigint); + return { __proto__: null, result }; + } + + static importModuleDynamicallyWrap(importModuleDynamically) { + // Named declaration for function name + const importModuleDynamicallyWrapper = async (...args) => { + const m = await importModuleDynamically(...args); + if (isModuleNamespaceObject(m)) { + return m; + } + try { + m.#wrap; + } catch { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (m.status === 'errored') { + throw m.error; + } + return m.namespace; + }; + return importModuleDynamicallyWrapper; } [customInspectSymbol](depth, options) { @@ -258,16 +315,19 @@ class SourceTextModule { const o = Object.create({ constructor: ctor }); o.status = this.status; - o.linkingStatus = this.linkingStatus; - o.url = this.url; + o.identifier = this.identifier; o.context = this.context; return require('internal/util/inspect').inspect(o, options); } } +// Declared as static to allow access to #wrap +const importModuleDynamicallyWrap = + SourceTextModule.importModuleDynamicallyWrap; +delete SourceTextModule.importModuleDynamicallyWrap; + module.exports = { SourceTextModule, wrapToModuleMap, - wrapMap, - linkingStatusMap, + importModuleDynamicallyWrap, }; diff --git a/lib/vm.js b/lib/vm.js index ec6614e661..90f332c775 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -31,10 +31,8 @@ const { } = internalBinding('contextify'); const { ERR_INVALID_ARG_TYPE, - ERR_VM_MODULE_NOT_MODULE, } = require('internal/errors').codes; const { - isModuleNamespaceObject, isArrayBufferView, } = require('internal/util/types'); const { @@ -100,21 +98,13 @@ class Script extends ContextifyScript { 'function', importModuleDynamically); } - const { wrapMap, linkingStatusMap } = + const { importModuleDynamicallyWrap } = 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)) { - return m; - } - if (!m || !wrapMap.has(m)) - throw new ERR_VM_MODULE_NOT_MODULE(); - const childLinkingStatus = linkingStatusMap.get(m); - if (childLinkingStatus === 'errored') - throw m.error; - return m.namespace; - } }); + callbackMap.set(this, { + importModuleDynamically: + importModuleDynamicallyWrap(importModuleDynamically), + }); } } -- cgit v1.2.3