From 91b0e6404875281cadb60a5ecdbbd03888442e14 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 11 May 2026 14:10:06 +0200 Subject: [PATCH] fix: terminate worker pool on short-circuit returns from pnpm/main (#11571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pnpm` could hang for the lifetime of the worker pool after a command logically finished whenever the code path returned through `main` without `process.exit(...)`. `main.ts` only ran `finishWorkers()` inside the command-handler `finally` block. Short-circuit returns that came earlier — `--version`, `--help`, fatal config errors that surface before a command runs — bypassed it and left the integrity-resolution worker pool active. The CLI entry (`pnpm/src/pnpm.ts`) now runs `finishWorkers()` in its own `finally`, so every exit path tears down the pool. Cleanup rejections are swallowed so the `finally` never masks the real command result. ## Reproducer ```bash echo '{"devEngines":{"packageManager":{"name":"pnpm","version":"11.0.9","onFail":"download"}}}' > package.json time pnpm --version # before (Linux): prints 11.0.9, then hangs for the worker-pool lifetime # after: prints 11.0.9, exits in ~1-3 s ``` `switchCliVersion` resolves the integrity (spawning workers), finds nothing to swap (range/version already matches running pnpm), and returns through main's `--version` short-circuit. The unterminated workers used to keep the event loop alive. Surfaced in pnpm/action-setup#254 — bumping the action's bootstrap to 11.0.9 made every CI job using `devEngines.packageManager` with `onFail: "download"` hang. --- .changeset/version-exit-finishes-workers.md | 7 +++++ pnpm/src/pnpm.ts | 8 +++++ pnpm/test/packageManagerCheck.test.ts | 34 ++++++++++++++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 .changeset/version-exit-finishes-workers.md diff --git a/.changeset/version-exit-finishes-workers.md b/.changeset/version-exit-finishes-workers.md new file mode 100644 index 0000000000..b6c6cabd6e --- /dev/null +++ b/.changeset/version-exit-finishes-workers.md @@ -0,0 +1,7 @@ +--- +pnpm: patch +--- + +Fix `pnpm --version` hanging for the lifetime of the worker pool after the version was printed. `main.ts`'s `--version` short-circuit returned before reaching the command-handler `finally` that calls `finishWorkers()`, so the worker pool that `switchCliVersion` had spawned during integrity resolution stayed alive and held the Node event loop open. The CLI entry now runs `finishWorkers()` from its own `finally`, so every exit path tears the pool down. + +Repro: `pnpm --version` in a workspace whose `devEngines.packageManager` version already matches the running pnpm + `onFail: "download"`. `switchCliVersion` resolves the integrity (spawning workers), finds nothing to swap, returns. The version prints, then the process hangs. diff --git a/pnpm/src/pnpm.ts b/pnpm/src/pnpm.ts index 2ceacb3a35..8c1d875a5c 100644 --- a/pnpm/src/pnpm.ts +++ b/pnpm/src/pnpm.ts @@ -12,12 +12,20 @@ const argv = buildArgv() })() async function runPnpm (): Promise { + const { finishWorkers } = await import('@pnpm/worker') const { errorHandler } = await import('./errorHandler.js') try { const { main } = await import('./main.js') await main(argv) } catch (err: any) { // eslint-disable-line await errorHandler(err) + } finally { + // `pnpm --version` short-circuits main() after switchCliVersion has + // already spawned the worker pool — drain it here so the event loop + // can exit. Swallow rejections so cleanup never masks the real result. + try { + await finishWorkers() + } catch { /* ignore */ } } } diff --git a/pnpm/test/packageManagerCheck.test.ts b/pnpm/test/packageManagerCheck.test.ts index 84d42f4132..ac39c76b19 100644 --- a/pnpm/test/packageManagerCheck.test.ts +++ b/pnpm/test/packageManagerCheck.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@jest/globals' -import { prepare } from '@pnpm/prepare' +import { prepare, prepareEmpty } from '@pnpm/prepare' import { writeYamlFileSync } from 'write-yaml-file' import { execPnpmSync } from './utils/index.js' @@ -176,6 +176,38 @@ test('devEngines.packageManager with a different PM name should fail with onFail expect(stderr.toString()).toContain('This project is configured to use yarn') }) +test('pnpm --version exits promptly when devEngines.packageManager matches the running pnpm', async () => { + // Regression test: main.ts's `--version` short-circuit returned before + // the command-handler `finally` that calls finishWorkers(), and + // switchCliVersion had already spawned workers during integrity + // resolution. The worker pool then kept the Node event loop alive long + // past the version print. + // Read the running pnpm version from a fresh empty dir — the previous + // test's prepare() leaves cwd in a manifest with a failing pm check, and + // checkPackageManager runs before the --version short-circuit. + prepareEmpty() + const versionProcess = execPnpmSync(['--version']) + const pnpmVersion = versionProcess.stdout.toString().trim() + + prepare({ + devEngines: { + packageManager: { + name: 'pnpm', + version: pnpmVersion, + onFail: 'download', + }, + }, + }) + + // 30 s is comfortably above the post-fix exit time (~3 s) and far below + // the pre-fix hang. If the regression returns, spawnSync's timeout kicks + // in and execPnpmSync throws from its `error`/`signal` checks. + const { status, stdout } = execPnpmSync(['--version'], { timeout: 30_000 }) + + expect(status).toBe(0) + expect(stdout.toString().trim()).toBe(pnpmVersion) +}) + test('devEngines.packageManager array selects the pnpm entry', async () => { prepare({ devEngines: {