diff --git a/CHANGELOG.md b/CHANGELOG.md index ff2433f294c..c8983dbfef2 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/lib/npm/node.ts b/lib/npm/node.ts index 2b0237135d0..c538f493e31 100644 --- a/lib/npm/node.ts +++ b/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(); } diff --git a/scripts/plugin-tests.js b/scripts/plugin-tests.js index a3e0de719c2..540e32fd690 100644 --- a/scripts/plugin-tests.js +++ b/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) diff --git a/scripts/register-test.js b/scripts/register-test.js index 7e43325a575..a0d6a5af777 100644 --- a/scripts/register-test.js +++ b/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 }))