From 020ccc30422c3b7b5a998fe3e3fb8b5e5fbeaee9 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 21 May 2023 21:28:27 +0200 Subject: [PATCH] Do not force quit Rollup or close stdout (#5004) * Do not force quit Rollup or close stdout reverts #4969, #4983 * Avoid memory leaks via beforeExit listeners --- cli/cli.ts | 6 ++--- rollup.config.ts | 11 ++++++++- src/utils/hookActions.ts | 50 ++++++++++++++++++++++------------------ 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/cli/cli.ts b/cli/cli.ts index 5643fd7c3ff..d06f13c84c5 100644 --- a/cli/cli.ts +++ b/cli/cli.ts @@ -21,8 +21,6 @@ if (command.help || (process.argv.length <= 2 && process.stdin.isTTY)) { } catch { // do nothing } - run(command).then(() => { - process.stdout.on('finish', () => process.exit(0)); - process.stdout.end(); - }); + + run(command); } diff --git a/rollup.config.ts b/rollup.config.ts index 9c3d9c71e08..8d6974376c2 100644 --- a/rollup.config.ts +++ b/rollup.config.ts @@ -137,7 +137,16 @@ export default async function ( terser({ module: true, output: { comments: 'some' } }), collectLicensesBrowser(), writeLicenseBrowser(), - cleanBeforeWrite('browser/dist') + cleanBeforeWrite('browser/dist'), + { + closeBundle() { + // On CI, macOS runs sometimes do not close properly. This is a hack + // to fix this until the problem is understood. + console.log('Force quit.'); + setTimeout(() => process.exit(0)); + }, + name: 'force-close' + } ], strictDeprecations: true, treeshake diff --git a/src/utils/hookActions.ts b/src/utils/hookActions.ts index ef07992eb9d..c2143332a00 100644 --- a/src/utils/hookActions.ts +++ b/src/utils/hookActions.ts @@ -1,4 +1,3 @@ -import { EventEmitter } from 'node:events'; import process from 'node:process'; import type { HookAction, PluginDriver } from './PluginDriver'; @@ -25,33 +24,40 @@ function formatAction([pluginName, hookName, parameters]: HookAction): string { return action; } -// We do not directly listen on process to avoid max listeners warnings for -// complicated build processes -const beforeExitEvent = 'beforeExit'; -// eslint-disable-next-line unicorn/prefer-event-target -const beforeExitEmitter = new EventEmitter(); -beforeExitEmitter.setMaxListeners(0); -process.on(beforeExitEvent, () => beforeExitEmitter.emit(beforeExitEvent)); +let handleBeforeExit: null | (() => void) = null; +const rejectByPluginDriver = new Map void>(); export async function catchUnfinishedHookActions( pluginDriver: PluginDriver, callback: () => Promise ): Promise { - let handleEmptyEventLoop: () => void; const emptyEventLoopPromise = new Promise((_, reject) => { - handleEmptyEventLoop = () => { - const unfulfilledActions = pluginDriver.getUnfulfilledHookActions(); - reject( - new Error( - `Unexpected early exit. This happens when Promises returned by plugins cannot resolve. Unfinished hook action(s) on exit:\n` + - [...unfulfilledActions].map(formatAction).join('\n') - ) - ); - }; - beforeExitEmitter.once(beforeExitEvent, handleEmptyEventLoop); + rejectByPluginDriver.set(pluginDriver, reject); + if (!handleBeforeExit) { + // We only ever create a single event listener to avoid max listener and + // other issues + handleBeforeExit = () => { + for (const [pluginDriver, reject] of rejectByPluginDriver) { + const unfulfilledActions = pluginDriver.getUnfulfilledHookActions(); + reject( + new Error( + `Unexpected early exit. This happens when Promises returned by plugins cannot resolve. Unfinished hook action(s) on exit:\n` + + [...unfulfilledActions].map(formatAction).join('\n') + ) + ); + } + }; + process.once('beforeExit', handleBeforeExit); + } }); - const result = await Promise.race([callback(), emptyEventLoopPromise]); - beforeExitEmitter.off(beforeExitEvent, handleEmptyEventLoop!); - return result; + try { + return await Promise.race([callback(), emptyEventLoopPromise]); + } finally { + rejectByPluginDriver.delete(pluginDriver); + if (rejectByPluginDriver.size === 0) { + process.off('beforeExit', handleBeforeExit!); + handleBeforeExit = null; + } + } }