Skip to content

Commit 6679e6b

Browse files
joyeecheungrichardlau
authored andcommittedMar 25, 2024
lib: add assertion for user ESM execution
Previously we only had an internal assertion to ensure certain code is executed before any user-provided CJS is run. This patch adds another assertion for ESM. Note that this internal state is not updated during source text module execution via vm because to run any code via vm, some user JS code must have already been executed anyway. In addition this patch moves the states into internal/modules/helpers to avoid circular dependencies. Also moves toggling the states to true *right before* user code execution instead of after in case we are half-way in the execution when internals try to check them. PR-URL: #51748 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent 7639259 commit 6679e6b

File tree

4 files changed

+38
-8
lines changed

4 files changed

+38
-8
lines changed
 

‎lib/internal/modules/cjs/loader.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ module.exports = {
7676
initializeCJS,
7777
Module,
7878
wrapSafe,
79-
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
8079
};
8180

8281
const { BuiltinModule } = require('internal/bootstrap/realm');
@@ -113,6 +112,7 @@ const {
113112
initializeCjsConditions,
114113
loadBuiltinModule,
115114
makeRequireFunction,
115+
setHasStartedUserCJSExecution,
116116
stripBOM,
117117
toRealPath,
118118
} = require('internal/modules/helpers');
@@ -127,9 +127,6 @@ const permission = require('internal/process/permission');
127127
const {
128128
vm_dynamic_import_default_internal,
129129
} = internalBinding('symbols');
130-
// Whether any user-provided CJS modules had been loaded (executed).
131-
// Used for internal assertions.
132-
let hasLoadedAnyUserCJSModule = false;
133130

134131
const {
135132
codes: {
@@ -1364,14 +1361,14 @@ Module.prototype._compile = function(content, filename) {
13641361
const thisValue = exports;
13651362
const module = this;
13661363
if (requireDepth === 0) { statCache = new SafeMap(); }
1364+
setHasStartedUserCJSExecution();
13671365
if (inspectorWrapper) {
13681366
result = inspectorWrapper(compiledWrapper, thisValue, exports,
13691367
require, module, filename, dirname);
13701368
} else {
13711369
result = ReflectApply(compiledWrapper, thisValue,
13721370
[exports, require, module, filename, dirname]);
13731371
}
1374-
hasLoadedAnyUserCJSModule = true;
13751372
if (requireDepth === 0) { statCache = null; }
13761373
return result;
13771374
};

‎lib/internal/modules/esm/module_job.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ const {
2727
} = require('internal/source_map/source_map_cache');
2828
const assert = require('internal/assert');
2929
const resolvedPromise = PromiseResolve();
30-
30+
const {
31+
setHasStartedUserESMExecution,
32+
} = require('internal/modules/helpers');
3133
const noop = FunctionPrototype;
3234

3335
let hasPausedEntry = false;
@@ -206,6 +208,7 @@ class ModuleJob {
206208
this.instantiated = PromiseResolve();
207209
const timeout = -1;
208210
const breakOnSigint = false;
211+
setHasStartedUserESMExecution();
209212
this.module.evaluate(timeout, breakOnSigint);
210213
return { __proto__: null, module: this.module };
211214
}
@@ -214,6 +217,7 @@ class ModuleJob {
214217
await this.instantiate();
215218
const timeout = -1;
216219
const breakOnSigint = false;
220+
setHasStartedUserESMExecution();
217221
try {
218222
await this.module.evaluate(timeout, breakOnSigint);
219223
} catch (e) {

‎lib/internal/modules/helpers.js

+25
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,19 @@ function normalizeReferrerURL(referrerName) {
319319
assert.fail('Unreachable code reached by ' + inspect(referrerName));
320320
}
321321

322+
323+
// Whether we have started executing any user-provided CJS code.
324+
// This is set right before we call the wrapped CJS code (not after,
325+
// in case we are half-way in the execution when internals check this).
326+
// Used for internal assertions.
327+
let _hasStartedUserCJSExecution = false;
328+
// Similar to _hasStartedUserCJSExecution but for ESM. This is set
329+
// right before ESM evaluation in the default ESM loader. We do not
330+
// update this during vm SourceTextModule execution because at that point
331+
// some user code must already have been run to execute code via vm
332+
// there is little value checking whether any user JS code is run anyway.
333+
let _hasStartedUserESMExecution = false;
334+
322335
module.exports = {
323336
addBuiltinLibsToObject,
324337
getCjsConditions,
@@ -328,4 +341,16 @@ module.exports = {
328341
normalizeReferrerURL,
329342
stripBOM,
330343
toRealPath,
344+
hasStartedUserCJSExecution() {
345+
return _hasStartedUserCJSExecution;
346+
},
347+
setHasStartedUserCJSExecution() {
348+
_hasStartedUserCJSExecution = true;
349+
},
350+
hasStartedUserESMExecution() {
351+
return _hasStartedUserESMExecution;
352+
},
353+
setHasStartedUserESMExecution() {
354+
_hasStartedUserESMExecution = true;
355+
},
331356
};

‎lib/internal/process/pre_execution.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,12 @@ function setupSymbolDisposePolyfill() {
190190
function setupUserModules(forceDefaultLoader = false) {
191191
initializeCJSLoader();
192192
initializeESMLoader(forceDefaultLoader);
193-
const CJSLoader = require('internal/modules/cjs/loader');
194-
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
193+
const {
194+
hasStartedUserCJSExecution,
195+
hasStartedUserESMExecution,
196+
} = require('internal/modules/helpers');
197+
assert(!hasStartedUserCJSExecution());
198+
assert(!hasStartedUserESMExecution());
195199
// Do not enable preload modules if custom loaders are disabled.
196200
// For example, loader workers are responsible for doing this themselves.
197201
// And preload modules are not supported in ShadowRealm as well.

0 commit comments

Comments
 (0)
Please sign in to comment.