fix(cli): avoid Windows crash after network requests (#12121)

* fix: avoid Windows crash after network requests

* fix: destroy dispatchers before Windows error exits

* test: cover destroyDispatchers and document its shutdown contract

Add a JSDoc to destroyDispatchers() explaining it is for process shutdown,
a unit test verifying it destroys the global and cached dispatchers, and bump
@pnpm/network.fetch to minor for the new public export.

* fix(network.fetch): destroy the active global dispatcher on shutdown

Include getGlobalDispatcher() so destroyDispatchers() also closes the
currently-active dispatcher if it was replaced via setGlobalDispatcher().

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/destroy-windows-dispatchers.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/network.fetch": minor
+"pnpm": patch
+---
+
+Avoided a Node.js crash when pnpm exits after network requests on Windows.

network/fetch/src/dispatcher.ts

@@ -7,7 +7,7 @@ import { PnpmError } from '@pnpm/error'
 import type { TlsConfig } from '@pnpm/types'
 import { LRUCache } from 'lru-cache'
 import { SocksClient } from 'socks'
-import { Agent, type Dispatcher, ProxyAgent, setGlobalDispatcher } from 'undici'
+import { Agent, type Dispatcher, getGlobalDispatcher, ProxyAgent, setGlobalDispatcher } from 'undici'
 
 const DEFAULT_MAX_SOCKETS = 50
 const KEEP_ALIVE_TIMEOUT = 30_000 // 30 seconds
@@ -20,14 +20,16 @@ const KEEP_ALIVE_MAX_TIMEOUT = 600_000 // 10 minutes
 // With HTTP/2, undici multiplexes many streams over 1-2 TCP connections sharing a single
 // congestion window. In benchmarks this was slower than opening ~50 independent HTTP/1.1
 // connections that each get their own congestion window and can saturate bandwidth in parallel.
-setGlobalDispatcher(new Agent({
+const GLOBAL_DISPATCHER = new Agent({
   connections: DEFAULT_MAX_SOCKETS,
   keepAliveTimeout: KEEP_ALIVE_TIMEOUT,
   keepAliveMaxTimeout: KEEP_ALIVE_MAX_TIMEOUT,
   connect: {
     autoSelectFamily: true,
   },
-}).compose(stripSecFetchHeaders))
+}).compose(stripSecFetchHeaders)
+
+setGlobalDispatcher(GLOBAL_DISPATCHER)
 
 // undici's fetch() automatically adds sec-fetch-* headers (e.g. sec-fetch-mode: cors)
 // per the Fetch spec. Some registries like Azure DevOps Artifacts interpret these as
@@ -98,6 +100,24 @@ export function clearDispatcherCache (): void {
   DISPATCHER_CACHE.clear()
 }
 
+/**
+ * Destroy the global dispatcher and every cached dispatcher, closing their open
+ * sockets. Intended for process shutdown only: once called, the module can no
+ * longer perform network requests. This is used to work around a Windows crash
+ * that happens when the process exits while sockets are still open
+ * (https://github.com/nodejs/node/issues/56645).
+ */
+export async function destroyDispatchers (): Promise<void> {
+  // getGlobalDispatcher() is included in case something replaced GLOBAL_DISPATCHER
+  // via setGlobalDispatcher(); the Set removes the duplicate when it is still our own instance.
+  const dispatchers = new Set<Dispatcher>([
+    GLOBAL_DISPATCHER,
+    getGlobalDispatcher(),
+    ...DISPATCHER_CACHE.values(),
+  ])
+  await Promise.allSettled(Array.from(dispatchers, dispatcher => dispatcher.destroy()))
+}
+
 /**
  * Get a dispatcher for the given URI and options.
  * Returns undefined if no special configuration is needed (to use global dispatcher).

network/fetch/src/index.ts

@@ -1,4 +1,4 @@
-export { clearDispatcherCache, getDispatcher } from './dispatcher.js'
+export { clearDispatcherCache, destroyDispatchers, getDispatcher } from './dispatcher.js'
 export { fetch, isRedirect, type RetryTimeoutOptions } from './fetch.js'
 export { createDispatchedFetch, type CreateDispatchedFetchOptions, createFetchFromRegistry, type CreateFetchFromRegistryOptions, type DispatcherOptions, fetchWithDispatcher } from './fetchFromRegistry.js'
 export type { FetchFromRegistry } from '@pnpm/fetching.types'

network/fetch/test/dispatcher.test.ts

@@ -1,9 +1,9 @@
 /// <reference path="../../../__typings__/index.d.ts"/>
 import net from 'node:net'
 
-import { afterEach, describe, expect, test } from '@jest/globals'
-import { clearDispatcherCache, type DispatcherOptions, getDispatcher } from '@pnpm/network.fetch'
-import { Agent, ProxyAgent } from 'undici'
+import { afterEach, describe, expect, jest, test } from '@jest/globals'
+import { clearDispatcherCache, destroyDispatchers, type DispatcherOptions, getDispatcher } from '@pnpm/network.fetch'
+import { Agent, getGlobalDispatcher, ProxyAgent } from 'undici'
 
 afterEach(() => {
   clearDispatcherCache()
@@ -270,3 +270,20 @@ describe('client certificates', () => {
     expect(d1).not.toBe(d2) // different certs → different dispatchers
   })
 })
+
+describe('destroyDispatchers', () => {
+  // The destroy() calls are mocked so the live global dispatcher survives the test run.
+  test('destroys the global dispatcher and every cached dispatcher without throwing', async () => {
+    const globalDestroySpy = jest.spyOn(getGlobalDispatcher(), 'destroy').mockResolvedValue()
+    const cached = getDispatcher('https://registry.npmjs.org/foo', { strictSsl: false })!
+    const cachedDestroySpy = jest.spyOn(cached, 'destroy').mockResolvedValue()
+    try {
+      await expect(destroyDispatchers()).resolves.toBeUndefined()
+      expect(globalDestroySpy).toHaveBeenCalled()
+      expect(cachedDestroySpy).toHaveBeenCalled()
+    } finally {
+      globalDestroySpy.mockRestore()
+      cachedDestroySpy.mockRestore()
+    }
+  })
+})

pnpm-lock.yaml

@@ -7811,6 +7811,9 @@ importers:
       '@pnpm/logger':
         specifier: workspace:*
         version: link:../core/logger
+      '@pnpm/network.fetch':
+        specifier: workspace:*
+        version: link:../network/fetch
       '@pnpm/nopt':
         specifier: 'catalog:'
         version: 0.3.1

pnpm/package.json

@@ -111,6 +111,7 @@
     "@pnpm/lockfile.fs": "workspace:*",
     "@pnpm/lockfile.types": "workspace:*",
     "@pnpm/logger": "workspace:*",
+    "@pnpm/network.fetch": "workspace:*",
     "@pnpm/nopt": "catalog:",
     "@pnpm/patching.commands": "workspace:*",
     "@pnpm/pkg-manifest.commands": "workspace:*",

pnpm/src/errorHandler.ts

@@ -3,6 +3,7 @@ import { promisify } from 'node:util'
 import { logger } from '@pnpm/logger'
 import pidTree from 'pidtree'
 
+import { exit } from './exit.js'
 import { type Global, REPORTER_INITIALIZED } from './main.js'
 
 declare const global: Global
@@ -59,5 +60,5 @@ async function killProcesses (status: number): Promise<void> {
   } catch {
     // ignore error here
   }
-  process.exit(status)
+  await exit(status)
 }

pnpm/src/exit.ts

@@ -0,0 +1,12 @@
+export async function exit (status: number): Promise<never> {
+  if (process.platform === 'win32') {
+    try {
+      // Work around https://github.com/nodejs/node/issues/56645.
+      const { destroyDispatchers } = await import('@pnpm/network.fetch')
+      await destroyDispatchers()
+    } catch {
+      // ignore error here
+    }
+  }
+  process.exit(status)
+}

pnpm/src/switchCliVersion.ts

@@ -12,6 +12,7 @@ import { createStoreController } from '@pnpm/store.connection-manager'
 import spawn from 'cross-spawn'
 import semver from 'semver'
 
+import { exit } from './exit.js'
 import { shouldPersistLockfile } from './shouldPersistLockfile.js'
 
 export async function switchCliVersion (config: Config, context: ConfigContext): Promise<void> {
@@ -129,7 +130,7 @@ export async function switchCliVersion (config: Config, context: ConfigContext):
     return
   }
 
-  process.exit(status ?? 0)
+  await exit(status ?? 0)
 }
 
 class VersionSwitchFail extends PnpmError {

pnpm/test/install/global.ts

@@ -408,9 +408,9 @@ test('global add in strict minimumReleaseAge mode reports the user-facing error'
   })
   const output = `${result.stdout.toString()}\n${result.stderr.toString()}`
 
-  expect(result.status).toBe(1)
   expect(output).toContain('ERR_PNPM_NO_MATURE_MATCHING_VERSION')
   expect(output).not.toContain('ERR_PNPM_RESOLUTION_POLICY_VIOLATIONS_UNHANDLED')
+  expect(result.status).toBe(1)
 })
 
 test('global update in loose minimumReleaseAge mode persists immature picks', () => {

pnpm/test/install/minimumReleaseAge.ts

@@ -182,10 +182,10 @@ describe('lockfile minimumReleaseAge verification', () => {
       [PUBLIC_REGISTRY, 'install', '--frozen-lockfile'],
       omitMinReleaseAgeEnv
     )
-    expect(result.status).toBe(1)
     const output = `${result.stdout.toString()}\n${result.stderr.toString()}`
     expect(output).toContain('ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION')
     expect(output).toMatch(/is-odd@0\.1\.2/)
+    expect(result.status).toBe(1)
   })
 
   test('loose mode auto-adds fresh immature picks to minimumReleaseAgeExclude', () => {

pnpm/tsconfig.json

@@ -125,6 +125,9 @@
     {
       "path": "../lockfile/types"
     },
+    {
+      "path": "../network/fetch"
+    },
     {
       "path": "../patching/commands"
     },