From d181b76374166c94d285fcbf4775cfd602482c94 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 13 Dec 2022 23:51:05 +0100 Subject: [PATCH] bootstrap: make CJS loader snapshotable This patch makes the top-level access to runtime states in the CJS loader lazy, and move the side-effects into a initializeCJS() function that gets called during pre-execution. As a result the CJS loader can be included into the built-in snapshot. PR-URL: https://github.com/nodejs/node/pull/45849 Reviewed-By: Geoffrey Booth Reviewed-By: Chengzhong Wu --- lib/internal/modules/cjs/loader.js | 191 ++++++++++-------- lib/internal/modules/helpers.js | 34 ++-- lib/internal/process/pre_execution.js | 9 +- ...ction.js => inspector-global-function.mjs} | 0 test/parallel/test-bootstrap-modules.js | 23 +-- .../test-inspector-break-when-eval.js | 7 +- 6 files changed, 140 insertions(+), 124 deletions(-) rename test/fixtures/{inspector-global-function.js => inspector-global-function.mjs} (100%) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 466c160506e669..2e0be0e3d8a1f8 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -72,7 +72,8 @@ const cjsParseCache = new SafeWeakMap(); // Set first due to cycle with ESM loader functions. module.exports = { wrapSafe, Module, toRealPath, readPackageScope, cjsParseCache, - get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; } + get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; }, + initializeCJS, }; const { BuiltinModule } = require('internal/bootstrap/loaders'); @@ -86,8 +87,8 @@ const { kEmptyObject, filterOwnProperties, setOwnProperty, + getLazy, } = require('internal/util'); -const { Script } = require('vm'); const { internalCompileFunction } = require('internal/vm'); const assert = require('internal/assert'); const fs = require('fs'); @@ -97,21 +98,24 @@ const { sep } = path; const { internalModuleStat } = internalBinding('fs'); const { safeGetenv } = internalBinding('credentials'); const { - cjsConditions, + getCjsConditions, + initializeCjsConditions, hasEsmSyntax, loadBuiltinModule, makeRequireFunction, normalizeReferrerURL, stripBOM, } = require('internal/modules/helpers'); -const { getOptionValue } = require('internal/options'); -const preserveSymlinks = getOptionValue('--preserve-symlinks'); -const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); -const shouldReportRequiredModules = process.env.WATCH_REPORT_DEPENDENCIES; -// Do not eagerly grab .manifest, it may be in TDZ -const policy = getOptionValue('--experimental-policy') ? - require('internal/process/policy') : - null; +const packageJsonReader = require('internal/modules/package_json_reader'); +const { getOptionValue, getEmbedderOptions } = require('internal/options'); +const policy = getLazy( + () => (getOptionValue('--experimental-policy') ? require('internal/process/policy') : null) +); +const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES); + +const getCascadedLoader = getLazy( + () => require('internal/process/esm_loader').esmLoader +); // Whether any user-provided CJS modules had been loaded (executed). // Used for internal assertions. @@ -127,7 +131,6 @@ const { setArrowMessage, } = require('internal/errors'); const { validateString } = require('internal/validators'); -const pendingDeprecation = getOptionValue('--pending-deprecation'); const { CHAR_BACKWARD_SLASH, @@ -140,15 +143,7 @@ const { isProxy } = require('internal/util/types'); -const asyncESM = require('internal/process/esm_loader'); -const { enrichCJSError } = require('internal/modules/esm/translators'); const { kEvaluated } = internalBinding('module_wrap'); -const { - encodedSepRegEx, - packageExportsResolve, - packageImportsResolve -} = require('internal/modules/esm/resolve'); - const isWindows = process.platform === 'win32'; const relativeResolveCache = ObjectCreate(null); @@ -190,13 +185,13 @@ function updateChildren(parent, child, scan) { } function reportModuleToWatchMode(filename) { - if (shouldReportRequiredModules && process.send) { + if (shouldReportRequiredModules() && process.send) { process.send({ 'watch:require': [filename] }); } } function reportModuleNotFoundToWatchMode(basePath, extensions) { - if (shouldReportRequiredModules && process.send) { + if (shouldReportRequiredModules() && process.send) { process.send({ 'watch:require': ArrayPrototypeMap(extensions, (ext) => path.resolve(`${basePath}${ext}`)) }); } } @@ -213,22 +208,6 @@ function Module(id = '', parent) { this.children = []; } -const builtinModules = []; -for (const { 0: id, 1: mod } of BuiltinModule.map) { - if (mod.canBeRequiredByUsers && - BuiltinModule.canBeRequiredWithoutScheme(id)) { - ArrayPrototypePush(builtinModules, id); - } -} - -const allBuiltins = new SafeSet( - ArrayPrototypeFlatMap(builtinModules, (bm) => [bm, `node:${bm}`]) -); -BuiltinModule.getSchemeOnlyModuleNames().forEach((builtin) => allBuiltins.add(`node:${builtin}`)); - -ObjectFreeze(builtinModules); -Module.builtinModules = builtinModules; - Module._cache = ObjectCreate(null); Module._pathCache = ObjectCreate(null); Module._extensions = ObjectCreate(null); @@ -297,26 +276,59 @@ function setModuleParent(value) { moduleParentCache.set(this, value); } -ObjectDefineProperty(Module.prototype, 'parent', { - __proto__: null, - get: pendingDeprecation ? deprecate( - getModuleParent, - 'module.parent is deprecated due to accuracy issues. Please use ' + - 'require.main to find program entry point instead.', - 'DEP0144' - ) : getModuleParent, - set: pendingDeprecation ? deprecate( - setModuleParent, - 'module.parent is deprecated due to accuracy issues. Please use ' + - 'require.main to find program entry point instead.', - 'DEP0144' - ) : setModuleParent, -}); - let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); -Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077'); + +const builtinModules = []; +// This function is called during pre-execution, before any user code is run. +function initializeCJS() { + const pendingDeprecation = getOptionValue('--pending-deprecation'); + ObjectDefineProperty(Module.prototype, 'parent', { + __proto__: null, + get: pendingDeprecation ? deprecate( + getModuleParent, + 'module.parent is deprecated due to accuracy issues. Please use ' + + 'require.main to find program entry point instead.', + 'DEP0144' + ) : getModuleParent, + set: pendingDeprecation ? deprecate( + setModuleParent, + 'module.parent is deprecated due to accuracy issues. Please use ' + + 'require.main to find program entry point instead.', + 'DEP0144' + ) : setModuleParent, + }); + Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077'); + + for (const { 0: id, 1: mod } of BuiltinModule.map) { + if (mod.canBeRequiredByUsers && + BuiltinModule.canBeRequiredWithoutScheme(id)) { + ArrayPrototypePush(builtinModules, id); + } + } + + const allBuiltins = new SafeSet( + ArrayPrototypeFlatMap(builtinModules, (bm) => [bm, `node:${bm}`]) + ); + BuiltinModule.getSchemeOnlyModuleNames().forEach((builtin) => allBuiltins.add(`node:${builtin}`)); + ObjectFreeze(builtinModules); + Module.builtinModules = builtinModules; + + Module.isBuiltin = function isBuiltin(moduleName) { + return allBuiltins.has(moduleName); + }; + + initializeCjsConditions(); + + if (!getEmbedderOptions().noGlobalSearchPaths) { + Module._initPaths(); + } + + // TODO(joyeecheung): deprecate this in favor of a proper hook? + Module.runMain = + require('internal/modules/run_main').executeUserEntryPoint; +} // Given a module name, and a list of paths to test, returns the first // matching file in the following precedence. @@ -337,7 +349,6 @@ function readPackage(requestPath) { const existing = packageJsonCache.get(jsonPath); if (existing !== undefined) return existing; - const packageJsonReader = require('internal/modules/package_json_reader'); const result = packageJsonReader.read(jsonPath); const json = result.containsKeys === false ? '{}' : result.string; if (json === undefined) { @@ -440,7 +451,7 @@ const realpathCache = new SafeMap(); function tryFile(requestPath, isMain) { const rc = _stat(requestPath); if (rc !== 0) return; - if (preserveSymlinks && !isMain) { + if (getOptionValue('--preserve-symlinks') && !isMain) { return path.resolve(requestPath); } return toRealPath(requestPath); @@ -511,9 +522,10 @@ function trySelf(parentPath, request) { } try { + const { packageExportsResolve } = require('internal/modules/esm/resolve'); return finalizeEsmResolution(packageExportsResolve( pathToFileURL(pkgPath + '/package.json'), expansion, pkg, - pathToFileURL(parentPath), cjsConditions), parentPath, pkgPath); + pathToFileURL(parentPath), getCjsConditions()), parentPath, pkgPath); } catch (e) { if (e.code === 'ERR_MODULE_NOT_FOUND') throw createEsmNotFoundErr(request, pkgPath + '/package.json'); @@ -535,9 +547,10 @@ function resolveExports(nmPath, request) { const pkg = _readPackage(pkgPath); if (pkg?.exports != null) { try { + const { packageExportsResolve } = require('internal/modules/esm/resolve'); return finalizeEsmResolution(packageExportsResolve( pathToFileURL(pkgPath + '/package.json'), '.' + expansion, pkg, null, - cjsConditions), null, pkgPath); + getCjsConditions()), null, pkgPath); } catch (e) { if (e.code === 'ERR_MODULE_NOT_FOUND') throw createEsmNotFoundErr(request, pkgPath + '/package.json'); @@ -616,19 +629,19 @@ Module._findPath = function(request, paths, isMain) { if (!trailingSlash) { if (rc === 0) { // File. if (!isMain) { - if (preserveSymlinks) { + if (getOptionValue('--preserve-symlinks')) { filename = path.resolve(basePath); } else { filename = toRealPath(basePath); } - } else if (preserveSymlinksMain) { - // For the main module, we use the preserveSymlinksMain flag instead + } else if (getOptionValue('--preserve-symlinks-main')) { + // For the main module, we use the --preserve-symlinks-main flag instead // mainly for backward compatibility, as the preserveSymlinks flag // historically has not applied to the main module. Most likely this // was intended to keep .bin/ binaries working, as following those // symlinks is usually required for the imports in the corresponding // files to resolve; that said, in some use cases following symlinks - // causes bigger problems which is why the preserveSymlinksMain option + // causes bigger problems which is why the --preserve-symlinks-main option // is needed. filename = path.resolve(basePath); } else { @@ -999,9 +1012,10 @@ Module._resolveFilename = function(request, parent, isMain, options) { const pkg = readPackageScope(parentPath) || {}; if (pkg.data?.imports != null) { try { + const { packageImportsResolve } = require('internal/modules/esm/resolve'); return finalizeEsmResolution( packageImportsResolve(request, pathToFileURL(parentPath), - cjsConditions), parentPath, + getCjsConditions()), parentPath, pkg.path); } catch (e) { if (e.code === 'ERR_MODULE_NOT_FOUND') @@ -1043,6 +1057,7 @@ Module._resolveFilename = function(request, parent, isMain, options) { }; function finalizeEsmResolution(resolved, parentPath, pkgPath) { + const { encodedSepRegEx } = require('internal/modules/esm/resolve'); if (RegExpPrototypeExec(encodedSepRegEx, resolved) !== null) throw new ERR_INVALID_MODULE_SPECIFIER( resolved, 'must not include encoded "/" or "\\" characters', parentPath); @@ -1081,14 +1096,14 @@ Module.prototype.load = function(filename) { Module._extensions[extension](this, filename); this.loaded = true; - const esmLoader = asyncESM.esmLoader; + const cascadedLoader = getCascadedLoader(); // Create module entry at load time to snapshot exports correctly const exports = this.exports; // Preemptively cache if ((module?.module === undefined || module.module.getStatus() < kEvaluated) && - !esmLoader.cjsCache.has(this)) - esmLoader.cjsCache.set(this, exports); + !cascadedLoader.cjsCache.has(this)) + cascadedLoader.cjsCache.set(this, exports); }; @@ -1113,17 +1128,20 @@ Module.prototype.require = function(id) { // (needed for setting breakpoint when called with --inspect-brk) let resolvedArgv; let hasPausedEntry = false; - +let Script; function wrapSafe(filename, content, cjsModuleInstance) { if (patched) { const wrapper = Module.wrap(content); + if (Script === undefined) { + ({ Script } = require('vm')); + } const script = new Script(wrapper, { filename, lineOffset: 0, importModuleDynamically: async (specifier, _, importAssertions) => { - const loader = asyncESM.esmLoader; - return loader.import(specifier, normalizeReferrerURL(filename), - importAssertions); + const cascadedLoader = getCascadedLoader(); + return cascadedLoader.import(specifier, normalizeReferrerURL(filename), + importAssertions); }, }); @@ -1147,9 +1165,9 @@ function wrapSafe(filename, content, cjsModuleInstance) { ], { filename, importModuleDynamically(specifier, _, importAssertions) { - const loader = asyncESM.esmLoader; - return loader.import(specifier, normalizeReferrerURL(filename), - importAssertions); + const cascadedLoader = getCascadedLoader(); + return cascadedLoader.import(specifier, normalizeReferrerURL(filename), + importAssertions); }, }); @@ -1160,8 +1178,10 @@ function wrapSafe(filename, content, cjsModuleInstance) { return result.function; } catch (err) { - if (process.mainModule === cjsModuleInstance) + if (process.mainModule === cjsModuleInstance) { + const { enrichCJSError } = require('internal/modules/esm/translators'); enrichCJSError(err, content); + } throw err; } } @@ -1173,10 +1193,11 @@ function wrapSafe(filename, content, cjsModuleInstance) { Module.prototype._compile = function(content, filename) { let moduleURL; let redirects; - if (policy?.manifest) { + const manifest = policy()?.manifest; + if (manifest) { moduleURL = pathToFileURL(filename); - redirects = policy.manifest.getDependencyMapper(moduleURL); - policy.manifest.assertIntegrity(moduleURL, content); + redirects = manifest.getDependencyMapper(moduleURL); + manifest.assertIntegrity(moduleURL, content); } const compiledWrapper = wrapSafe(filename, content, this); @@ -1277,9 +1298,10 @@ Module._extensions['.js'] = function(module, filename) { Module._extensions['.json'] = function(module, filename) { const content = fs.readFileSync(filename, 'utf8'); - if (policy?.manifest) { + const manifest = policy()?.manifest; + if (manifest) { const moduleURL = pathToFileURL(filename); - policy.manifest.assertIntegrity(moduleURL, content); + manifest.assertIntegrity(moduleURL, content); } try { @@ -1293,10 +1315,11 @@ Module._extensions['.json'] = function(module, filename) { // Native extension for .node Module._extensions['.node'] = function(module, filename) { - if (policy?.manifest) { + const manifest = policy()?.manifest; + if (manifest) { const content = fs.readFileSync(filename); const moduleURL = pathToFileURL(filename); - policy.manifest.assertIntegrity(moduleURL, content); + manifest.assertIntegrity(moduleURL, content); } // Be aware this doesn't use `content` return process.dlopen(module, path.toNamespacedPath(filename)); @@ -1405,9 +1428,5 @@ Module.syncBuiltinESMExports = function syncBuiltinESMExports() { } }; -Module.isBuiltin = function isBuiltin(moduleName) { - return allBuiltins.has(moduleName); -}; - // Backwards compatibility Module.Module = Module; diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index fea3fbcf48dc00..49c086279e238d 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -25,22 +25,31 @@ const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); const { getOptionValue } = require('internal/options'); const { setOwnProperty } = require('internal/util'); -const userConditions = getOptionValue('--conditions'); let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); -const noAddons = getOptionValue('--no-addons'); -const addonConditions = noAddons ? [] : ['node-addons']; +let cjsConditions; +function initializeCjsConditions() { + const userConditions = getOptionValue('--conditions'); + const noAddons = getOptionValue('--no-addons'); + const addonConditions = noAddons ? [] : ['node-addons']; + // TODO: Use this set when resolving pkg#exports conditions in loader.js. + cjsConditions = new SafeSet([ + 'require', + 'node', + ...addonConditions, + ...userConditions, + ]); +} -// TODO: Use this set when resolving pkg#exports conditions in loader.js. -const cjsConditions = new SafeSet([ - 'require', - 'node', - ...addonConditions, - ...userConditions, -]); +function getCjsConditions() { + if (cjsConditions === undefined) { + initializeCjsConditions(); + } + return cjsConditions; +} function loadBuiltinModule(filename, request) { const mod = BuiltinModule.map.get(filename); @@ -62,7 +71,7 @@ function makeRequireFunction(mod, redirects) { let require; if (redirects) { const id = mod.filename || mod.id; - const conditions = cjsConditions; + const conditions = getCjsConditions(); const { resolve, reaction } = redirects; require = function require(specifier) { let missing = true; @@ -231,7 +240,8 @@ function hasEsmSyntax(code) { module.exports = { addBuiltinLibsToObject, - cjsConditions, + getCjsConditions, + initializeCjsConditions, hasEsmSyntax, loadBuiltinModule, makeRequireFunction, diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 899a69a6f3bfff..d66a699b14cec6 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -560,13 +560,8 @@ function initializeWASI() { } function initializeCJSLoader() { - const CJSLoader = require('internal/modules/cjs/loader'); - if (!getEmbedderOptions().noGlobalSearchPaths) { - CJSLoader.Module._initPaths(); - } - // TODO(joyeecheung): deprecate this in favor of a proper hook? - CJSLoader.Module.runMain = - require('internal/modules/run_main').executeUserEntryPoint; + const { initializeCJS } = require('internal/modules/cjs/loader'); + initializeCJS(); } function initializeESMLoader() { diff --git a/test/fixtures/inspector-global-function.js b/test/fixtures/inspector-global-function.mjs similarity index 100% rename from test/fixtures/inspector-global-function.js rename to test/fixtures/inspector-global-function.mjs diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 3ecc7f91a64813..10513e796db719 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -53,26 +53,15 @@ const expectedModules = new Set([ 'NativeModule internal/fs/utils', 'NativeModule internal/idna', 'NativeModule internal/linkedlist', - 'NativeModule internal/modules/helpers', 'NativeModule internal/modules/cjs/loader', - 'NativeModule internal/modules/esm/assert', - 'NativeModule internal/modules/esm/formats', - 'NativeModule internal/modules/esm/get_format', - 'NativeModule internal/modules/esm/initialize_import_meta', - 'NativeModule internal/modules/esm/load', - 'NativeModule internal/modules/esm/loader', - 'NativeModule internal/modules/esm/module_map', - 'NativeModule internal/modules/esm/package_config', - 'NativeModule internal/modules/esm/resolve', - 'NativeModule internal/modules/esm/translators', 'NativeModule internal/modules/esm/utils', + 'NativeModule internal/modules/helpers', 'NativeModule internal/modules/package_json_reader', 'NativeModule internal/modules/run_main', 'NativeModule internal/net', 'NativeModule internal/options', 'NativeModule internal/perf/utils', 'NativeModule internal/priority_queue', - 'NativeModule internal/process/esm_loader', 'NativeModule internal/process/execution', 'NativeModule internal/process/per_thread', 'NativeModule internal/process/pre_execution', @@ -100,15 +89,10 @@ const expectedModules = new Set([ 'NativeModule timers', 'NativeModule url', 'NativeModule util', - 'NativeModule vm', ]); if (!common.isMainThread) { [ - 'Internal Binding messaging', - 'Internal Binding performance', - 'Internal Binding symbols', - 'Internal Binding worker', 'NativeModule diagnostics_channel', 'NativeModule internal/abort_controller', 'NativeModule internal/error_serdes', @@ -139,6 +123,11 @@ if (!common.isMainThread) { ].forEach(expectedModules.add.bind(expectedModules)); } +if (common.isWindows) { + // On Windows fs needs SideEffectFreeRegExpPrototypeExec which uses vm. + expectedModules.add('NativeModule vm'); +} + if (common.hasIntl) { expectedModules.add('Internal Binding icu'); } else { diff --git a/test/sequential/test-inspector-break-when-eval.js b/test/sequential/test-inspector-break-when-eval.js index 1e7ab513dadbbb..bd9969e0dcfffd 100644 --- a/test/sequential/test-inspector-break-when-eval.js +++ b/test/sequential/test-inspector-break-when-eval.js @@ -6,7 +6,10 @@ const { NodeInstance } = require('../common/inspector-helper.js'); const fixtures = require('../common/fixtures'); const { pathToFileURL } = require('url'); -const script = fixtures.path('inspector-global-function.js'); +// This needs to be an ES module file to ensure that internal modules are +// loaded before pausing. See +// https://bugs.chromium.org/p/chromium/issues/detail?id=1246905 +const script = fixtures.path('inspector-global-function.mjs'); async function setupDebugger(session) { console.log('[test]', 'Setting up a debugger'); @@ -23,7 +26,7 @@ async function setupDebugger(session) { // NOTE(mmarchini): We wait for the second console.log to ensure we loaded // every internal module before pausing. See - // https://bugs.chromium.org/p/v8/issues/detail?id=10287. + // https://bugs.chromium.org/p/chromium/issues/detail?id=1246905 const waitForReady = session.waitForConsoleOutput('log', 'Ready!'); session.send({ 'method': 'Debugger.resume' }); await waitForReady;