Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 91e4a7c

Browse filesBrowse files
joyeecheungtargos
authored andcommitted
loader: use default loader as cascaded loader in the in loader worker
Use the default loader as the cascaded loader in the loader worker. Otherwise we spawn loader workers in the loader workers indefinitely. PR-URL: #47620 Fixes: #47566 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 9101630 commit 91e4a7c
Copy full SHA for 91e4a7c

File tree

Expand file treeCollapse file tree

8 files changed

+106
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+106
-10
lines changed
Open diff view settings
Collapse file

‎lib/internal/main/worker_thread.js‎

Copy file name to clipboardExpand all lines: lib/internal/main/worker_thread.js
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ port.on('message', (message) => {
133133
if (manifestSrc) {
134134
require('internal/process/policy').setup(manifestSrc, manifestURL);
135135
}
136-
setupUserModules();
136+
const isLoaderWorker =
137+
doEval === 'internal' &&
138+
filename === require('internal/modules/esm/utils').loaderWorkerId;
139+
setupUserModules(isLoaderWorker);
137140

138141
if (!hasStdin)
139142
process.stdin.push(null);
Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/hooks.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const {
5151
} = require('internal/modules/esm/resolve');
5252
const {
5353
getDefaultConditions,
54+
loaderWorkerId,
5455
} = require('internal/modules/esm/utils');
5556
const { deserializeError } = require('internal/error_serdes');
5657
const {
@@ -493,7 +494,7 @@ class HooksProxy {
493494
const lock = new SharedArrayBuffer(SHARED_MEMORY_BYTE_LENGTH);
494495
this.#lock = new Int32Array(lock);
495496

496-
this.#worker = new InternalWorker('internal/modules/esm/worker', {
497+
this.#worker = new InternalWorker(loaderWorkerId, {
497498
stderr: false,
498499
stdin: false,
499500
stdout: false,
Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,10 @@ let emittedExperimentalWarning = false;
412412
* @returns {DefaultModuleLoader | CustomizedModuleLoader}
413413
*/
414414
function createModuleLoader(useCustomLoadersIfPresent = true) {
415-
if (useCustomLoadersIfPresent) {
415+
if (useCustomLoadersIfPresent &&
416+
// Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader;
417+
// doing so would cause an infinite loop.
418+
!require('internal/modules/esm/utils').isLoaderWorker()) {
416419
const userLoaderPaths = getOptionValue('--experimental-loader');
417420
if (userLoaderPaths.length > 0) {
418421
if (!emittedExperimentalWarning) {
Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/utils.js
+11-1Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,22 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
9191
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
9292
}
9393

94-
function initializeESM() {
94+
// This is configured during pre-execution. Specifically it's set to true for
95+
// the loader worker in internal/main/worker_thread.js.
96+
let _isLoaderWorker = false;
97+
function initializeESM(isLoaderWorker = false) {
98+
_isLoaderWorker = isLoaderWorker;
9599
initializeDefaultConditions();
96100
// Setup per-isolate callbacks that locate data or callbacks that we keep
97101
// track of for different ESM modules.
98102
setInitializeImportMetaObjectCallback(initializeImportMetaObject);
99103
setImportModuleDynamicallyCallback(importModuleDynamicallyCallback);
100104
}
101105

106+
function isLoaderWorker() {
107+
return _isLoaderWorker;
108+
}
109+
102110
async function initializeHooks() {
103111
const customLoaderPaths = getOptionValue('--experimental-loader');
104112

@@ -165,4 +173,6 @@ module.exports = {
165173
initializeHooks,
166174
getDefaultConditions,
167175
getConditionsSet,
176+
loaderWorkerId: 'internal/modules/esm/worker',
177+
isLoaderWorker,
168178
};
Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/worker.js
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const { receiveMessageOnPort } = require('internal/worker/io');
2929
const {
3030
WORKER_TO_MAIN_THREAD_NOTIFICATION,
3131
} = require('internal/modules/esm/shared_constants');
32-
const { initializeESM, initializeHooks } = require('internal/modules/esm/utils');
32+
const { initializeHooks } = require('internal/modules/esm/utils');
3333

3434

3535
function transferArrayBuffer(hasError, source) {
@@ -80,7 +80,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {
8080

8181

8282
try {
83-
initializeESM();
8483
const initResult = await initializeHooks();
8584
hooks = initResult.hooks;
8685
preloadScripts = initResult.preloadScripts;
Collapse file

‎lib/internal/process/pre_execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/pre_execution.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ function prepareExecution(options) {
119119
}
120120
}
121121

122-
function setupUserModules() {
122+
function setupUserModules(isLoaderWorker = false) {
123123
initializeCJSLoader();
124-
initializeESMLoader();
124+
initializeESMLoader(isLoaderWorker);
125125
const CJSLoader = require('internal/modules/cjs/loader');
126126
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
127127
loadPreloadModules();
@@ -600,9 +600,9 @@ function initializeCJSLoader() {
600600
initializeCJS();
601601
}
602602

603-
function initializeESMLoader() {
603+
function initializeESMLoader(isLoaderWorker) {
604604
const { initializeESM } = require('internal/modules/esm/utils');
605-
initializeESM();
605+
initializeESM(isLoaderWorker);
606606

607607
// Patch the vm module when --experimental-vm-modules is on.
608608
// Please update the comments in vm.js when this block changes.
Collapse file
+77Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { spawnPromisified } from '../common/index.mjs';
2+
import * as fixtures from '../common/fixtures.mjs';
3+
import assert from 'node:assert';
4+
import { execPath } from 'node:process';
5+
import { describe, it } from 'node:test';
6+
7+
describe('Worker threads do not spawn infinitely', { concurrency: true }, () => {
8+
it('should not trigger an infinite loop when using a loader exports no recognized hooks', async () => {
9+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
10+
'--no-warnings',
11+
'--experimental-loader',
12+
fixtures.fileURL('empty.js'),
13+
'--eval',
14+
'setTimeout(() => console.log("hello"),99)',
15+
]);
16+
17+
assert.strictEqual(stderr, '');
18+
assert.match(stdout, /^hello\r?\n$/);
19+
assert.strictEqual(code, 0);
20+
assert.strictEqual(signal, null);
21+
});
22+
23+
it('should support a CommonJS entry point and a loader that imports a CommonJS module', async () => {
24+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
25+
'--no-warnings',
26+
'--experimental-loader',
27+
fixtures.fileURL('es-module-loaders/loader-with-dep.mjs'),
28+
fixtures.path('print-delayed.js'),
29+
]);
30+
31+
assert.strictEqual(stderr, '');
32+
assert.match(stdout, /^delayed\r?\n$/);
33+
assert.strictEqual(code, 0);
34+
assert.strictEqual(signal, null);
35+
});
36+
37+
it('should support --require and --import along with using a loader written in CJS and CJS entry point', async () => {
38+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
39+
'--no-warnings',
40+
'--eval',
41+
'setTimeout(() => console.log("D"),99)',
42+
'--import',
43+
fixtures.fileURL('printC.js'),
44+
'--experimental-loader',
45+
fixtures.fileURL('printB.js'),
46+
'--require',
47+
fixtures.path('printA.js'),
48+
]);
49+
50+
assert.strictEqual(stderr, '');
51+
// The worker code should always run before the --import, but the console.log might arrive late.
52+
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
53+
assert.strictEqual(code, 0);
54+
assert.strictEqual(signal, null);
55+
});
56+
57+
it('should support --require and --import along with using a loader written in ESM and ESM entry point', async () => {
58+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
59+
'--no-warnings',
60+
'--require',
61+
fixtures.path('printA.js'),
62+
'--experimental-loader',
63+
'data:text/javascript,console.log("B")',
64+
'--import',
65+
fixtures.fileURL('printC.js'),
66+
'--input-type=module',
67+
'--eval',
68+
'setTimeout(() => console.log("D"),99)',
69+
]);
70+
71+
assert.strictEqual(stderr, '');
72+
// The worker code should always run before the --import, but the console.log might arrive late.
73+
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
74+
assert.strictEqual(code, 0);
75+
assert.strictEqual(signal, null);
76+
});
77+
});
Collapse file

‎test/fixtures/print-delayed.js‎

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
setTimeout(() => {
2+
console.log('delayed');
3+
}, 100);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.