Skip to content

Commit 6679e6b

Browse files
joyeecheungrichardlau
authored andcommitted
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 <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
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‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ module.exports ={
7676
initializeCJS,
7777
Module,
7878
wrapSafe,
79-
gethasLoadedAnyUserCJSModule(){returnhasLoadedAnyUserCJSModule;},
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-
lethasLoadedAnyUserCJSModule=false;
133130

134131
const{
135132
codes: {
@@ -1364,14 +1361,14 @@ Module.prototype._compile = function(content, filename){
13641361
constthisValue=exports;
13651362
constmodule=this;
13661363
if(requireDepth===0){statCache=newSafeMap();}
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
returnresult;
13771374
};

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ const{
2727
}=require('internal/source_map/source_map_cache');
2828
constassert=require('internal/assert');
2929
constresolvedPromise=PromiseResolve();
30-
30+
const{
31+
setHasStartedUserESMExecution,
32+
}=require('internal/modules/helpers');
3133
constnoop=FunctionPrototype;
3234

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

‎lib/internal/modules/helpers.js‎

Lines changed: 25 additions & 0 deletions
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‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,12 @@ function setupSymbolDisposePolyfill(){
190190
functionsetupUserModules(forceDefaultLoader=false){
191191
initializeCJSLoader();
192192
initializeESMLoader(forceDefaultLoader);
193-
constCJSLoader=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)