Fix using JS synchronous API from from non-main threads (#1411)

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Fix using JS synchronous API from from non-main threads ([#1406](https://github.com/evanw/esbuild/issues/1406))
+
+    This release fixes an issue with the new implementation of the synchronous JS API calls (`transformSync` and `buildSync`) when they are used from a thread other than the main thread. The problem happened because esbuild's new implementation uses node's `worker_threads` library internally and non-main threads were incorrectly assumed to be esbuild's internal thread instead of potentially another unrelated thread. Now esbuild's synchronous JS APIs should work correctly when called from non-main threads.
+
 ## 0.12.12
 
 * Fix `file` loader import paths when subdirectories are present ([#1044](https://github.com/evanw/esbuild/issues/1044))

lib/npm/node.ts

@@ -24,6 +24,11 @@ if (process.env.ESBUILD_WORKER_THREADS !== '0') {
   }
 }
 
+// This should only be true if this is our internal worker thread. We want this
+// library to be usable from other people's worker threads, so we should not be
+// checking for "isMainThread".
+let isInternalWorkerThread = worker_threads?.workerData?.esbuildVersion === ESBUILD_VERSION;
+
 let esbuildCommandAndArgs = (): [string, string[]] => {
   // This feature was added to give external code a way to modify the binary
   // path without modifying the code itself. Do not remove this because
@@ -149,7 +154,7 @@ export let formatMessages: typeof types.formatMessages = (messages, options) =>
 
 export let buildSync: typeof types.buildSync = (options: types.BuildOptions): any => {
   // Try using a long-lived worker thread to avoid repeated start-up overhead
-  if (worker_threads) {
+  if (worker_threads && !isInternalWorkerThread) {
     if (!workerThreadService) workerThreadService = startWorkerThreadService(worker_threads);
     return workerThreadService.buildSync(options);
   }
@@ -169,7 +174,7 @@ export let buildSync: typeof types.buildSync = (options: types.BuildOptions): an
 
 export let transformSync: typeof types.transformSync = (input, options) => {
   // Try using a long-lived worker thread to avoid repeated start-up overhead
-  if (worker_threads) {
+  if (worker_threads && !isInternalWorkerThread) {
     if (!workerThreadService) workerThreadService = startWorkerThreadService(worker_threads);
     return workerThreadService.transformSync(input, options);
   }
@@ -189,7 +194,7 @@ export let transformSync: typeof types.transformSync = (input, options) => {
 
 export let formatMessagesSync: typeof types.formatMessagesSync = (messages, options) => {
   // Try using a long-lived worker thread to avoid repeated start-up overhead
-  if (worker_threads) {
+  if (worker_threads && !isInternalWorkerThread) {
     if (!workerThreadService) workerThreadService = startWorkerThreadService(worker_threads);
     return workerThreadService.formatMessagesSync(messages, options);
   }
@@ -367,7 +372,7 @@ let workerThreadService: WorkerThreadService | null = null;
 let startWorkerThreadService = (worker_threads: typeof import('worker_threads')): WorkerThreadService => {
   let { port1: mainPort, port2: workerPort } = new worker_threads.MessageChannel();
   let worker = new worker_threads.Worker(__filename, {
-    workerData: { workerPort, defaultWD },
+    workerData: { workerPort, defaultWD, esbuildVersion: ESBUILD_VERSION },
     transferList: [workerPort],
 
     // From node's documentation: https://nodejs.org/api/worker_threads.html
@@ -520,6 +525,6 @@ let startSyncServiceWorker = () => {
 };
 
 // If we're in the worker thread, start the worker code
-if (worker_threads && !worker_threads.isMainThread) {
+if (isInternalWorkerThread) {
   startSyncServiceWorker();
 }

scripts/plugin-tests.js

@@ -2481,7 +2481,7 @@ async function main() {
   // Time out these tests after 5 minutes. This exists to help debug test hangs in CI.
   let minutes = 5
   let timeout = setTimeout(() => {
-    console.error(`❌ js api tests timed out after ${minutes} minutes, exiting...`)
+    console.error(`❌ plugin tests timed out after ${minutes} minutes, exiting...`)
     process.exit(1)
   }, minutes * 60 * 1000)
 

scripts/register-test.js

@@ -32,32 +32,84 @@ fs.writeFileSync(register, `
   };
 `)
 
+let tests = {
+  async fromMainThread() {
+    let result = await new Promise((resolve, reject) => child_process.execFile('node', ['-r', register, entry], (err, stdout) => {
+      if (err) reject(err)
+      else resolve(stdout)
+    }))
+    assert.strictEqual(result, `in entry.ts\nin other.ts\n`)
+  },
+
+  async fromChildThread({ testDir }) {
+    const startThread = path.join(testDir, 'startThread.js')
+    fs.writeFileSync(startThread, `
+      const worker_threads = require('worker_threads')
+      if (worker_threads.isMainThread) {
+        console.log('in startThread.js')
+        const worker = new worker_threads.Worker(__filename)
+        worker.postMessage(null)
+        worker.on('message', logs => {
+          for (const log of logs) console.log(log)
+          worker.terminate()
+        })
+      } else {
+        worker_threads.parentPort.on('message', () => {
+          console.log('in worker')
+          let logs = []
+          console.log = x => logs.push(x)
+          require('../entry.ts')
+          worker_threads.parentPort.postMessage(logs)
+        })
+      }
+    `)
+
+    let result = await new Promise((resolve, reject) => child_process.execFile('node', ['-r', register, startThread], (err, stdout) => {
+      if (err) reject(err)
+      else resolve(stdout)
+    }))
+    assert.strictEqual(result, `in startThread.js\nin worker\nin entry.ts\nin other.ts\n`)
+  },
+}
+
 async function main() {
-  let result
-  let promise = new Promise((resolve, reject) => child_process.execFile('node', ['-r', register, entry], (err, stdout) => {
-    if (err) {
-      reject(err)
-    } else {
-      result = stdout
-      resolve()
+  // Time out these tests after 5 minutes. This exists to help debug test hangs in CI.
+  let minutes = 5
+  let timeout = setTimeout(() => {
+    console.error(`❌ register tests timed out after ${minutes} minutes, exiting...`)
+    process.exit(1)
+  }, minutes * 60 * 1000)
+
+  const runTest = async ([name, fn]) => {
+    let testDir = path.join(rootTestDir, name)
+    try {
+      fs.mkdirSync(testDir)
+      await fn({ esbuild, testDir })
+      removeRecursiveSync(testDir)
+      return true
+    } catch (e) {
+      console.error(`❌ ${name}: ${e && e.message || e}`)
+      return false
     }
-  }))
-  let timeout
-  let wait = new Promise((_, reject) => {
-    timeout = setTimeout(() => reject(new Error('This test timed out')), 60 * 1000)
-  })
-  await Promise.race([promise, wait])
-  clearTimeout(timeout)
-  assert.strictEqual(result, `in entry.ts\nin other.ts\n`)
-}
+  }
 
-main().then(
-  () => {
-    console.log(`✅ register test passed`)
-    removeRecursiveSync(rootTestDir)
-  },
-  e => {
-    console.error(`❌ register test failed: ${e && e.message || e}`)
+  // Run all tests in serial
+  let allTestsPassed = true
+  for (let test of Object.entries(tests)) {
+    if (!await runTest(test)) {
+      allTestsPassed = false
+    }
+  }
+
+  if (!allTestsPassed) {
+    console.error(`❌ register tests failed`)
     process.exit(1)
-  },
-)
+  } else {
+    console.log(`✅ register tests passed`)
+    removeRecursiveSync(rootTestDir)
+  }
+
+  clearTimeout(timeout);
+}
+
+main().catch(e => setTimeout(() => { throw e }))