Skip to content

Commit 052e095

Browse files
joyeecheungrichardlau
authored andcommitted
vm: use internal versions of compileFunction and Script
Instead of using the public versions of the vm APIs internally, use the internal versions so that we can skip unnecessary argument validation. The public versions would need special care to the generation of host-defined options to hit the isolate compilation cache when imporModuleDynamically isn't used, while internally it's almost always used, so this allows us to handle the host-defined options separately. PR-URL: #50137 Backport-PR-URL: #51004 Refs: #35375 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 9f7899e commit 052e095

File tree

7 files changed

+245
-173
lines changed

7 files changed

+245
-173
lines changed

lib/internal/modules/cjs/loader.js

+40-32
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const {
5353
SafeMap,
5454
SafeWeakMap,
5555
String,
56+
Symbol,
5657
StringPrototypeCharAt,
5758
StringPrototypeCharCodeAt,
5859
StringPrototypeEndsWith,
@@ -85,7 +86,12 @@ const {
8586
setOwnProperty,
8687
getLazy,
8788
} = require('internal/util');
88-
const { internalCompileFunction } = require('internal/vm');
89+
const {
90+
internalCompileFunction,
91+
makeContextifyScript,
92+
runScriptInThisContext,
93+
} = require('internal/vm');
94+
8995
const assert = require('internal/assert');
9096
const fs = require('fs');
9197
const path = require('path');
@@ -1236,7 +1242,6 @@ Module.prototype.require = function(id) {
12361242
let resolvedArgv;
12371243
let hasPausedEntry = false;
12381244
/** @type {import('vm').Script} */
1239-
let Script;
12401245

12411246
/**
12421247
* Wraps the given content in a script and runs it in a new context.
@@ -1245,46 +1250,49 @@ let Script;
12451250
* @param {Module} cjsModuleInstance The CommonJS loader instance
12461251
*/
12471252
function wrapSafe(filename, content, cjsModuleInstance) {
1253+
const hostDefinedOptionId = Symbol(`cjs:${filename}`);
1254+
async function importModuleDynamically(specifier, _, importAttributes) {
1255+
const cascadedLoader = getCascadedLoader();
1256+
return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
1257+
importAttributes);
1258+
}
12481259
if (patched) {
1249-
const wrapper = Module.wrap(content);
1250-
if (Script === undefined) {
1251-
({ Script } = require('vm'));
1252-
}
1253-
const script = new Script(wrapper, {
1254-
filename,
1255-
lineOffset: 0,
1256-
importModuleDynamically: async (specifier, _, importAttributes) => {
1257-
const cascadedLoader = getCascadedLoader();
1258-
return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
1259-
importAttributes);
1260-
},
1261-
});
1260+
const wrapped = Module.wrap(content);
1261+
const script = makeContextifyScript(
1262+
wrapped, // code
1263+
filename, // filename
1264+
0, // lineOffset
1265+
0, // columnOffset
1266+
undefined, // cachedData
1267+
false, // produceCachedData
1268+
undefined, // parsingContext
1269+
hostDefinedOptionId, // hostDefinedOptionId
1270+
importModuleDynamically, // importModuleDynamically
1271+
);
12621272

12631273
// Cache the source map for the module if present.
12641274
if (script.sourceMapURL) {
12651275
maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL);
12661276
}
12671277

1268-
return script.runInThisContext({
1269-
displayErrors: true,
1270-
});
1278+
return runScriptInThisContext(script, true, false);
12711279
}
12721280

1281+
const params = [ 'exports', 'require', 'module', '__filename', '__dirname' ];
12731282
try {
1274-
const result = internalCompileFunction(content, [
1275-
'exports',
1276-
'require',
1277-
'module',
1278-
'__filename',
1279-
'__dirname',
1280-
], {
1281-
filename,
1282-
importModuleDynamically(specifier, _, importAttributes) {
1283-
const cascadedLoader = getCascadedLoader();
1284-
return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
1285-
importAttributes);
1286-
},
1287-
});
1283+
const result = internalCompileFunction(
1284+
content, // code,
1285+
filename, // filename
1286+
0, // lineOffset
1287+
0, // columnOffset,
1288+
undefined, // cachedData
1289+
false, // produceCachedData
1290+
undefined, // parsingContext
1291+
undefined, // contextExtensions
1292+
params, // params
1293+
hostDefinedOptionId, // hostDefinedOptionId
1294+
importModuleDynamically, // importModuleDynamically
1295+
);
12881296

12891297
// Cache the source map for the module if present.
12901298
if (result.sourceMapURL) {

lib/internal/process/execution.js

+24-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
Symbol,
45
RegExpPrototypeExec,
56
globalThis,
67
} = primordials;
@@ -24,7 +25,9 @@ const {
2425
emitAfter,
2526
popAsyncContext,
2627
} = require('internal/async_hooks');
27-
28+
const {
29+
makeContextifyScript, runScriptInThisContext,
30+
} = require('internal/vm');
2831
// shouldAbortOnUncaughtToggle is a typed array for faster
2932
// communication with JS.
3033
const { shouldAbortOnUncaughtToggle } = internalBinding('util');
@@ -52,8 +55,7 @@ function evalModule(source, print) {
5255

5356
function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
5457
const CJSModule = require('internal/modules/cjs/loader').Module;
55-
const { kVmBreakFirstLineSymbol } = require('internal/util');
56-
const { pathToFileURL } = require('url');
58+
const { pathToFileURL } = require('internal/url');
5759

5860
const cwd = tryGetCwd();
5961
const origModule = globalThis.module; // Set e.g. when called from the REPL.
@@ -78,16 +80,25 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
7880
`;
7981
globalThis.__filename = name;
8082
RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs.
81-
const result = module._compile(script, `${name}-wrapper`)(() =>
82-
require('vm').runInThisContext(body, {
83-
filename: name,
84-
displayErrors: true,
85-
[kVmBreakFirstLineSymbol]: !!breakFirstLine,
86-
importModuleDynamically(specifier, _, importAttributes) {
87-
const loader = asyncESM.esmLoader;
88-
return loader.import(specifier, baseUrl, importAttributes);
89-
},
90-
}));
83+
const result = module._compile(script, `${name}-wrapper`)(() => {
84+
const hostDefinedOptionId = Symbol(name);
85+
async function importModuleDynamically(specifier, _, importAttributes) {
86+
const loader = asyncESM.esmLoader;
87+
return loader.import(specifier, baseUrl, importAttributes);
88+
}
89+
const script = makeContextifyScript(
90+
body, // code
91+
name, // filename,
92+
0, // lineOffset
93+
0, // columnOffset,
94+
undefined, // cachedData
95+
false, // produceCachedData
96+
undefined, // parsingContext
97+
hostDefinedOptionId, // hostDefinedOptionId
98+
importModuleDynamically, // importModuleDynamically
99+
);
100+
return runScriptInThisContext(script, true, !!breakFirstLine);
101+
});
91102
if (print) {
92103
const { log } = require('internal/console/global');
93104
log(result);

lib/internal/vm.js

+69-61
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,25 @@
11
'use strict';
22

33
const {
4-
ArrayPrototypeForEach,
4+
ReflectApply,
55
Symbol,
66
} = primordials;
77

88
const {
9+
ContextifyScript,
910
compileFunction,
1011
isContext: _isContext,
1112
} = internalBinding('contextify');
13+
const {
14+
runInContext,
15+
} = ContextifyScript.prototype;
1216
const {
1317
default_host_defined_options,
1418
} = internalBinding('symbols');
1519
const {
16-
validateArray,
17-
validateBoolean,
18-
validateBuffer,
1920
validateFunction,
2021
validateObject,
21-
validateString,
22-
validateStringArray,
23-
validateInt32,
2422
} = require('internal/validators');
25-
const {
26-
ERR_INVALID_ARG_TYPE,
27-
} = require('internal/errors').codes;
2823

2924
function isContext(object) {
3025
validateObject(object, 'object', { __proto__: null, allowArray: true });
@@ -48,49 +43,20 @@ function getHostDefinedOptionId(importModuleDynamically, filename) {
4843
return Symbol(filename);
4944
}
5045

51-
function internalCompileFunction(code, params, options) {
52-
validateString(code, 'code');
53-
if (params !== undefined) {
54-
validateStringArray(params, 'params');
55-
}
56-
const {
57-
filename = '',
58-
columnOffset = 0,
59-
lineOffset = 0,
60-
cachedData = undefined,
61-
produceCachedData = false,
62-
parsingContext = undefined,
63-
contextExtensions = [],
64-
importModuleDynamically,
65-
} = options;
66-
67-
validateString(filename, 'options.filename');
68-
validateInt32(columnOffset, 'options.columnOffset');
69-
validateInt32(lineOffset, 'options.lineOffset');
70-
if (cachedData !== undefined)
71-
validateBuffer(cachedData, 'options.cachedData');
72-
validateBoolean(produceCachedData, 'options.produceCachedData');
73-
if (parsingContext !== undefined) {
74-
if (
75-
typeof parsingContext !== 'object' ||
76-
parsingContext === null ||
77-
!isContext(parsingContext)
78-
) {
79-
throw new ERR_INVALID_ARG_TYPE(
80-
'options.parsingContext',
81-
'Context',
82-
parsingContext,
83-
);
84-
}
85-
}
86-
validateArray(contextExtensions, 'options.contextExtensions');
87-
ArrayPrototypeForEach(contextExtensions, (extension, i) => {
88-
const name = `options.contextExtensions[${i}]`;
89-
validateObject(extension, name, { __proto__: null, nullable: true });
46+
function registerImportModuleDynamically(referrer, importModuleDynamically) {
47+
const { importModuleDynamicallyWrap } = require('internal/vm/module');
48+
const { registerModule } = require('internal/modules/esm/utils');
49+
registerModule(referrer, {
50+
__proto__: null,
51+
importModuleDynamically:
52+
importModuleDynamicallyWrap(importModuleDynamically),
9053
});
54+
}
9155

92-
const hostDefinedOptionId =
93-
getHostDefinedOptionId(importModuleDynamically, filename);
56+
function internalCompileFunction(
57+
code, filename, lineOffset, columnOffset,
58+
cachedData, produceCachedData, parsingContext, contextExtensions,
59+
params, hostDefinedOptionId, importModuleDynamically) {
9460
const result = compileFunction(
9561
code,
9662
filename,
@@ -117,23 +83,65 @@ function internalCompileFunction(code, params, options) {
11783
}
11884

11985
if (importModuleDynamically !== undefined) {
120-
validateFunction(importModuleDynamically,
121-
'options.importModuleDynamically');
122-
const { importModuleDynamicallyWrap } = require('internal/vm/module');
123-
const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
124-
const func = result.function;
125-
const { registerModule } = require('internal/modules/esm/utils');
126-
registerModule(func, {
127-
__proto__: null,
128-
importModuleDynamically: wrapped,
129-
});
86+
registerImportModuleDynamically(result.function, importModuleDynamically);
13087
}
13188

13289
return result;
13390
}
13491

92+
function makeContextifyScript(code,
93+
filename,
94+
lineOffset,
95+
columnOffset,
96+
cachedData,
97+
produceCachedData,
98+
parsingContext,
99+
hostDefinedOptionId,
100+
importModuleDynamically) {
101+
let script;
102+
// Calling `ReThrow()` on a native TryCatch does not generate a new
103+
// abort-on-uncaught-exception check. A dummy try/catch in JS land
104+
// protects against that.
105+
try { // eslint-disable-line no-useless-catch
106+
script = new ContextifyScript(code,
107+
filename,
108+
lineOffset,
109+
columnOffset,
110+
cachedData,
111+
produceCachedData,
112+
parsingContext,
113+
hostDefinedOptionId);
114+
} catch (e) {
115+
throw e; /* node-do-not-add-exception-line */
116+
}
117+
118+
if (importModuleDynamically !== undefined) {
119+
registerImportModuleDynamically(script, importModuleDynamically);
120+
}
121+
return script;
122+
}
123+
124+
// Internal version of vm.Script.prototype.runInThisContext() which skips
125+
// argument validation.
126+
function runScriptInThisContext(script, displayErrors, breakOnFirstLine) {
127+
return ReflectApply(
128+
runInContext,
129+
script,
130+
[
131+
null, // sandbox - use current context
132+
-1, // timeout
133+
displayErrors, // displayErrors
134+
false, // breakOnSigint
135+
breakOnFirstLine, // breakOnFirstLine
136+
],
137+
);
138+
}
139+
135140
module.exports = {
136141
getHostDefinedOptionId,
137142
internalCompileFunction,
138143
isContext,
144+
makeContextifyScript,
145+
registerImportModuleDynamically,
146+
runScriptInThisContext,
139147
};

0 commit comments

Comments
 (0)