mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-19 06:07:59 -04:00
fix: terminate worker pool on short-circuit returns from pnpm/main (#11571)
`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.
This commit is contained in:
7
.changeset/version-exit-finishes-workers.md
Normal file
7
.changeset/version-exit-finishes-workers.md
Normal file
@@ -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.
|
||||
@@ -12,12 +12,20 @@ const argv = buildArgv()
|
||||
})()
|
||||
|
||||
async function runPnpm (): Promise<void> {
|
||||
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 */ }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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: {
|
||||
|
||||
Reference in New Issue
Block a user