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>
nodejs · May 2, 2023 · 91e4a7c · 91e4a7c
1 parent 9101630
commit 91e4a7c
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 10 deletions.
diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -133,7 +133,10 @@ port.on('message', (message) => {
     if (manifestSrc) {
       require('internal/process/policy').setup(manifestSrc, manifestURL);
     }
-    setupUserModules();
+    const isLoaderWorker =
+      doEval === 'internal' &&
+      filename === require('internal/modules/esm/utils').loaderWorkerId;
+    setupUserModules(isLoaderWorker);
 
     if (!hasStdin)
       process.stdin.push(null);

diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -51,6 +51,7 @@ const {
 } = require('internal/modules/esm/resolve');
 const {
   getDefaultConditions,
+  loaderWorkerId,
 } = require('internal/modules/esm/utils');
 const { deserializeError } = require('internal/error_serdes');
 const {
@@ -493,7 +494,7 @@ class HooksProxy {
     const lock = new SharedArrayBuffer(SHARED_MEMORY_BYTE_LENGTH);
     this.#lock = new Int32Array(lock);
 
-    this.#worker = new InternalWorker('internal/modules/esm/worker', {
+    this.#worker = new InternalWorker(loaderWorkerId, {
       stderr: false,
       stdin: false,
       stdout: false,

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -412,7 +412,10 @@ let emittedExperimentalWarning = false;
  * @returns {DefaultModuleLoader | CustomizedModuleLoader}
  */
 function createModuleLoader(useCustomLoadersIfPresent = true) {
-  if (useCustomLoadersIfPresent) {
+  if (useCustomLoadersIfPresent &&
+      // Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader;
+      // doing so would cause an infinite loop.
+      !require('internal/modules/esm/utils').isLoaderWorker()) {
     const userLoaderPaths = getOptionValue('--experimental-loader');
     if (userLoaderPaths.length > 0) {
       if (!emittedExperimentalWarning) {

diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
@@ -91,14 +91,22 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
   throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
 }
 
-function initializeESM() {
+// This is configured during pre-execution. Specifically it's set to true for
+// the loader worker in internal/main/worker_thread.js.
+let _isLoaderWorker = false;
+function initializeESM(isLoaderWorker = false) {
+  _isLoaderWorker = isLoaderWorker;
   initializeDefaultConditions();
   // Setup per-isolate callbacks that locate data or callbacks that we keep
   // track of for different ESM modules.
   setInitializeImportMetaObjectCallback(initializeImportMetaObject);
   setImportModuleDynamicallyCallback(importModuleDynamicallyCallback);
 }
 
+function isLoaderWorker() {
+  return _isLoaderWorker;
+}
+
 async function initializeHooks() {
   const customLoaderPaths = getOptionValue('--experimental-loader');
 
@@ -165,4 +173,6 @@ module.exports = {
   initializeHooks,
   getDefaultConditions,
   getConditionsSet,
+  loaderWorkerId: 'internal/modules/esm/worker',
+  isLoaderWorker,
 };
diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js
@@ -29,7 +29,7 @@ const { receiveMessageOnPort } = require('internal/worker/io');
 const {
   WORKER_TO_MAIN_THREAD_NOTIFICATION,
 } = require('internal/modules/esm/shared_constants');
-const { initializeESM, initializeHooks } = require('internal/modules/esm/utils');
+const { initializeHooks } = require('internal/modules/esm/utils');
 
 
 function transferArrayBuffer(hasError, source) {
@@ -80,7 +80,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {
 
 
   try {
-    initializeESM();
     const initResult = await initializeHooks();
     hooks = initResult.hooks;
     preloadScripts = initResult.preloadScripts;

diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -119,9 +119,9 @@ function prepareExecution(options) {
   }
 }
 
-function setupUserModules() {
+function setupUserModules(isLoaderWorker = false) {
   initializeCJSLoader();
-  initializeESMLoader();
+  initializeESMLoader(isLoaderWorker);
   const CJSLoader = require('internal/modules/cjs/loader');
   assert(!CJSLoader.hasLoadedAnyUserCJSModule);
   loadPreloadModules();
@@ -600,9 +600,9 @@ function initializeCJSLoader() {
   initializeCJS();
 }
 
-function initializeESMLoader() {
+function initializeESMLoader(isLoaderWorker) {
   const { initializeESM } = require('internal/modules/esm/utils');
-  initializeESM();
+  initializeESM(isLoaderWorker);
 
   // Patch the vm module when --experimental-vm-modules is on.
   // Please update the comments in vm.js when this block changes.

diff --git a/test/es-module/test-loaders-workers-spawned.mjs b/test/es-module/test-loaders-workers-spawned.mjs
@@ -0,0 +1,77 @@
+import { spawnPromisified } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import assert from 'node:assert';
+import { execPath } from 'node:process';
+import { describe, it } from 'node:test';
+
+describe('Worker threads do not spawn infinitely', { concurrency: true }, () => {
+  it('should not trigger an infinite loop when using a loader exports no recognized hooks', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--experimental-loader',
+      fixtures.fileURL('empty.js'),
+      '--eval',
+      'setTimeout(() => console.log("hello"),99)',
+    ]);
+
+    assert.strictEqual(stderr, '');
+    assert.match(stdout, /^hello\r?\n$/);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+  });
+
+  it('should support a CommonJS entry point and a loader that imports a CommonJS module', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--experimental-loader',
+      fixtures.fileURL('es-module-loaders/loader-with-dep.mjs'),
+      fixtures.path('print-delayed.js'),
+    ]);
+
+    assert.strictEqual(stderr, '');
+    assert.match(stdout, /^delayed\r?\n$/);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+  });
+
+  it('should support --require and --import along with using a loader written in CJS and CJS entry point', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--eval',
+      'setTimeout(() => console.log("D"),99)',
+      '--import',
+      fixtures.fileURL('printC.js'),
+      '--experimental-loader',
+      fixtures.fileURL('printB.js'),
+      '--require',
+      fixtures.path('printA.js'),
+    ]);
+
+    assert.strictEqual(stderr, '');
+    // The worker code should always run before the --import, but the console.log might arrive late.
+    assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+  });
+
+  it('should support --require and --import along with using a loader written in ESM and ESM entry point', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--require',
+      fixtures.path('printA.js'),
+      '--experimental-loader',
+      'data:text/javascript,console.log("B")',
+      '--import',
+      fixtures.fileURL('printC.js'),
+      '--input-type=module',
+      '--eval',
+      'setTimeout(() => console.log("D"),99)',
+    ]);
+
+    assert.strictEqual(stderr, '');
+    // The worker code should always run before the --import, but the console.log might arrive late.
+    assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+  });
+});
diff --git a/test/fixtures/print-delayed.js b/test/fixtures/print-delayed.js
@@ -0,0 +1,3 @@
+setTimeout(() => {
+  console.log('delayed');
+}, 100);