From 914f6080c77cfe32a54888caa51ca6ea13873ce9 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 21 Dec 2023 22:45:25 -0500 Subject: [PATCH] fix #3558: put the `stop()` api call back --- CHANGELOG.md | 4 ++++ lib/npm/browser.ts | 12 ++++++++++++ lib/npm/node.ts | 23 ++++++++++++++++++++++- lib/shared/types.ts | 13 +++++++++++++ scripts/esbuild.js | 7 +------ scripts/js-api-tests.js | 32 +++++++++++++++++++++++++++++--- 6 files changed, 81 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36aa14b179d..5c912b09f89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ } ``` +* Provide the `stop()` API in node to exit esbuild's child process ([#3558](https://github.com/evanw/esbuild/issues/3558)) + + You can now call `stop()` in esbuild's node API to exit esbuild's child process to reclaim the resources used. It only makes sense to do this for a long-lived node process when you know you will no longer be making any more esbuild API calls. It is not necessary to call this to allow node to exit, and it's advantageous to not call this in between calls to esbuild's API as sharing a single long-lived esbuild child process is more efficient than re-creating a new esbuild child process for every API call. This API call used to exist but was removed in [version 0.9.0](https://github.com/evanw/esbuild/releases/v0.9.0). This release adds it back due to a user request. + ## 0.19.10 * Fix glob imports in TypeScript files ([#3319](https://github.com/evanw/esbuild/issues/3319)) diff --git a/lib/npm/browser.ts b/lib/npm/browser.ts index e2d7100e864..9885119a2cd 100644 --- a/lib/npm/browser.ts +++ b/lib/npm/browser.ts @@ -43,6 +43,10 @@ export const analyzeMetafileSync: typeof types.analyzeMetafileSync = () => { throw new Error(`The "analyzeMetafileSync" API only works in node`) } +export const stop = () => { + if (stopService) stopService() +} + interface Service { build: typeof types.build context: typeof types.context @@ -52,6 +56,7 @@ interface Service { } let initializePromise: Promise | undefined +let stopService: (() => void) | undefined let longLivedService: Service | undefined let ensureServiceIsRunning = (): Service => { @@ -129,6 +134,13 @@ const startRunningService = async (wasmURL: string | URL, wasmModule: WebAssembl // This will throw if WebAssembly module instantiation fails await firstMessagePromise + stopService = () => { + worker.terminate() + initializePromise = undefined + stopService = undefined + longLivedService = undefined + } + longLivedService = { build: (options: types.BuildOptions) => new Promise((resolve, reject) => diff --git a/lib/npm/node.ts b/lib/npm/node.ts index 28e7345963e..56a6ddb7076 100644 --- a/lib/npm/node.ts +++ b/lib/npm/node.ts @@ -220,6 +220,11 @@ export let analyzeMetafileSync: typeof types.analyzeMetafileSync = (metafile, op return result! } +export const stop = () => { + if (stopService) stopService() + if (workerThreadService) workerThreadService.stop() +} + let initializeWasCalled = false export let initialize: typeof types.initialize = options => { @@ -243,6 +248,7 @@ interface Service { let defaultWD = process.cwd() let longLivedService: Service | undefined +let stopService: (() => void) | undefined let ensureServiceIsRunning = (): Service => { if (longLivedService) return longLivedService @@ -278,6 +284,16 @@ let ensureServiceIsRunning = (): Service => { stdout.on('data', readFromStdout) stdout.on('end', afterClose) + stopService = () => { + // Close all resources related to the subprocess. + stdin.destroy() + stdout.destroy() + child.kill() + initializeWasCalled = false + longLivedService = undefined + stopService = undefined + } + let refCount = 0 child.unref() if (stdin.unref) { @@ -395,6 +411,7 @@ interface WorkerThreadService { transformSync(input: string | Uint8Array, options?: types.TransformOptions): types.TransformResult formatMessagesSync: typeof types.formatMessagesSync analyzeMetafileSync: typeof types.analyzeMetafileSync + stop(): void } let workerThreadService: WorkerThreadService | null = null @@ -475,7 +492,7 @@ let startWorkerThreadService = (worker_threads: typeof import('worker_threads')) // Calling unref() on a worker will allow the thread to exit if it's the last // only active handle in the event system. This means node will still exit // when there are no more event handlers from the main thread. So there's no - // need to have a "stop()" function. + // need to call the "stop()" function. worker.unref() return { @@ -492,6 +509,10 @@ let startWorkerThreadService = (worker_threads: typeof import('worker_threads')) analyzeMetafileSync(metafile, options) { return runCallSync('analyzeMetafile', [metafile, options]) }, + stop() { + worker.terminate() + workerThreadService = null + }, } } diff --git a/lib/shared/types.ts b/lib/shared/types.ts index 0c26c6bc7f7..b398f845bc4 100644 --- a/lib/shared/types.ts +++ b/lib/shared/types.ts @@ -661,3 +661,16 @@ export interface InitializeOptions { } export let version: string + +// Call this function to terminate esbuild's child process. The child process +// is not terminated and re-created for each API call because it's more +// efficient to keep it around when there are multiple API calls. +// +// In node this happens automatically before the parent node process exits. So +// you only need to call this if you know you will not make any more esbuild +// API calls and you want to clean up resources. +// +// Unlike node, Deno lacks the necessary APIs to clean up child processes +// automatically. You must manually call stop() in Deno when you're done +// using esbuild or Deno will continue running forever. +export declare function stop(): void; diff --git a/scripts/esbuild.js b/scripts/esbuild.js index ce5faaf59a3..4fb7f9b928f 100644 --- a/scripts/esbuild.js +++ b/scripts/esbuild.js @@ -251,12 +251,7 @@ const buildDenoLib = async (esbuildPath) => { fs.writeFileSync(path.join(denoDir, 'wasm.js'), modWASM) // Generate "deno/mod.d.ts" - const types_ts = fs.readFileSync(path.join(repoDir, 'lib', 'shared', 'types.ts'), 'utf8') + - `\n// Unlike node, Deno lacks the necessary APIs to clean up child processes` + - `\n// automatically. You must manually call stop() in Deno when you're done` + - `\n// using esbuild or Deno will continue running forever.` + - `\nexport function stop(): void;` + - `\n` + const types_ts = fs.readFileSync(path.join(repoDir, 'lib', 'shared', 'types.ts'), 'utf8') fs.writeFileSync(path.join(denoDir, 'mod.d.ts'), types_ts) fs.writeFileSync(path.join(denoDir, 'wasm.d.ts'), types_ts) diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index 0d8f57de6f5..74c11b6a743 100644 --- a/scripts/js-api-tests.js +++ b/scripts/js-api-tests.js @@ -7038,7 +7038,7 @@ let functionScopeCases = [ } } -let syncTests = { +let apiSyncTests = { async defaultExport({ esbuild }) { assert.strictEqual(typeof esbuild.version, 'string') assert.strictEqual(esbuild.version, esbuild.default.version) @@ -7356,6 +7356,26 @@ let childProcessTests = { }, } +let syncTests = { + async startStop({ esbuild }) { + for (let i = 0; i < 3; i++) { + let result1 = await esbuild.transform('1+2') + assert.strictEqual(result1.code, '1 + 2;\n') + + let result2 = esbuild.transformSync('2+3') + assert.strictEqual(result2.code, '2 + 3;\n') + + let result3 = await esbuild.build({ stdin: { contents: '1+2' }, write: false }) + assert.strictEqual(result3.outputFiles[0].text, '1 + 2;\n') + + let result4 = esbuild.buildSync({ stdin: { contents: '2+3' }, write: false }) + assert.strictEqual(result4.outputFiles[0].text, '2 + 3;\n') + + esbuild.stop() + } + }, +} + async function assertSourceMap(jsSourceMap, source) { jsSourceMap = JSON.parse(jsSourceMap) assert.deepStrictEqual(jsSourceMap.version, 3) @@ -7399,11 +7419,11 @@ async function main() { ...Object.entries(transformTests), ...Object.entries(formatTests), ...Object.entries(analyzeTests), - ...Object.entries(syncTests), + ...Object.entries(apiSyncTests), ...Object.entries(childProcessTests), ] - const allTestsPassed = (await Promise.all(tests.map(([name, fn]) => { + let allTestsPassed = (await Promise.all(tests.map(([name, fn]) => { const promise = runTest(name, fn) // Time out each individual test after 3 minutes. This exists to help debug test hangs in CI. @@ -7415,6 +7435,12 @@ async function main() { return promise.finally(() => clearTimeout(timeout)) }))).every(success => success) + for (let [name, fn] of Object.entries(syncTests)) { + if (!await runTest(name, fn)) { + allTestsPassed = false + } + } + if (!allTestsPassed) { console.error(`❌ js api tests failed`) process.exit(1)