fix: share raw metadata across in-flight requests (#13079)

* fix: share raw metadata across in-flight requests

* test: cover the metadata re-serialization fan-out

Add a regression test at the pickPackage/memo-cache boundary: 20 projects
joining one in-flight fetch must mirror the fetched body rather than each
re-serialize it. Against the pre-fix implementation it fails with 19
re-serializations, one per non-initiating caller.

Cover the rejection-path identity guard too, which no test exercised.

Correct the jsonText contract docs, which still described the pre-fix
behaviour, and restore the rationale for dropping the body once the
request settles.

---------

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

.changeset/share-inflight-packuments.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/resolving.npm-resolver": patch
+"pnpm": patch
+---
+
+Fixed an out-of-memory regression when workspace projects concurrently resolve a package with large registry metadata [pnpm/pnpm#13077](https://github.com/pnpm/pnpm/issues/13077).

pnpm11/resolving/npm-resolver/src/fetch.ts

@@ -40,10 +40,10 @@ export interface FetchMetadataResult {
   meta: PackageMeta
   /**
    * The raw registry response body, used only to mirror the response to disk
-   * without re-serializing `meta`. A fresh fetch always sets it, but it
-   * reaches only the caller that initiated the request: the phase-long memo
-   * cache holds a body-less clone (see memoizeFetchMetadata.ts), so cache
-   * hits see `undefined` and the cache never pins the body.
+   * without re-serializing `meta`. A fresh fetch always sets it, and every
+   * caller sharing that in-flight request sees it. Once the request settles
+   * the phase-long memo cache drops the body (see memoizeFetchMetadata.ts),
+   * so later cache hits see `undefined` and the cache never pins the body.
    */
   jsonText: string | undefined
   etag?: string

pnpm11/resolving/npm-resolver/src/memoizeFetchMetadata.ts

@@ -16,14 +16,20 @@ export interface MemoizedFetchMetadata {
  * `clear`, see `clearResolutionCache`), deduplicating concurrent and repeat
  * requests for the same package.
  *
- * Unlike plain memoization, the cache holds a body-less clone of each result:
- * `jsonText` — the raw registry response body, up to tens of MB for a popular
- * package — reaches only the caller that initiated the fetch, which is the
- * caller that writes the disk mirror. A phase-long cache that kept the bodies
- * would pin hundreds of MB on large cold-cache graphs. A cache-hit caller
- * that also writes the mirror falls back to `JSON.stringify(meta)` in
- * `prepareJsonForDisk`, which is equivalent on read: `loadMeta` re-derives
- * `etag` from the headers line.
+ * Unlike plain memoization, the entry is swapped for a body-less clone once
+ * the request settles. `jsonText` — the raw registry response body, up to tens
+ * of MB for a popular package — reaches every caller sharing the in-flight
+ * request, so a package resolved by many workspace projects at once mirrors
+ * that one body to disk instead of each project separately re-serializing
+ * `meta`. Retaining bodies past settlement would pin hundreds of MB on large
+ * cold-cache graphs, so a later cache-hit caller that writes the mirror falls
+ * back to `JSON.stringify(meta)` in `prepareJsonForDisk`, which is equivalent
+ * on read: `loadMeta` re-derives `etag` from the headers line.
+ *
+ * Because that swap lands a turn after the request settles, both settlement
+ * paths write back only while the entry is still their own promise — a `clear`
+ * (or a retry that already replaced the entry) must not be undone by a request
+ * that was in flight when it happened.
  *
  * A rejected fetch is evicted so a transient network failure is retried by
  * the next request instead of being cached for the rest of the phase.
@@ -36,11 +42,21 @@ export function memoizeFetchMetadata (fetch: FetchMetadata): MemoizedFetchMetada
       const cached = cache.get(key)
       if (cached != null) return cached
       const pending = fetch(pkgName, opts)
-      const bodiless = pending.then((result) =>
-        result.notModified ? result : { ...result, jsonText: undefined }
+      cache.set(key, pending)
+      void pending.then(
+        (result) => {
+          if (cache.get(key) !== pending) return
+          cache.set(
+            key,
+            Promise.resolve(
+              result.notModified ? result : { ...result, jsonText: undefined }
+            )
+          )
+        },
+        () => {
+          if (cache.get(key) === pending) cache.delete(key)
+        }
       )
-      bodiless.catch(() => cache.delete(key))
-      cache.set(key, bodiless)
       return pending
     },
     clear: () => {

pnpm11/resolving/npm-resolver/test/memoizeFetchMetadata.test.ts

@@ -37,7 +37,7 @@ test('the initiating caller receives the raw body; cache hits get a body-less cl
   expect(result.jsonText).toBe('{"name":"foo"}')
 })
 
-test('a caller arriving while the fetch is in flight gets the body-less clone', async () => {
+test('callers sharing an in-flight fetch receive the same raw body', async () => {
   let release!: (result: FetchMetadataResult) => void
   const { fetch } = memoizeFetchMetadata(async () => new Promise<FetchMetadataResult>((resolve) => {
     release = resolve
@@ -50,7 +50,7 @@ test('a caller arriving while the fetch is in flight gets the body-less clone',
   const [first, second] = await Promise.all([firstPromise, secondPromise])
   if (first.notModified || second.notModified) throw new Error('expected fresh fetch results')
   expect(first.jsonText).toBe('{"name":"foo"}')
-  expect(second.jsonText).toBeUndefined()
+  expect(second).toBe(first)
 })
 
 test('requests with different options are cached separately', async () => {
@@ -81,6 +81,52 @@ test('a rejected fetch is evicted so the next request retries', async () => {
   expect(calls).toBe(2)
 })
 
+test('clear does not let an in-flight fetch repopulate the cache', async () => {
+  let calls = 0
+  let release!: (result: FetchMetadataResult) => void
+  const { fetch, clear } = memoizeFetchMetadata(async () => {
+    calls++
+    return new Promise<FetchMetadataResult>((resolve) => {
+      release = resolve
+    })
+  })
+
+  const firstPromise = fetch('foo', { registry: REGISTRY })
+  clear()
+  release(fooFetchResult())
+  await firstPromise
+
+  const secondPromise = fetch('foo', { registry: REGISTRY })
+  expect(calls).toBe(2)
+  release(fooFetchResult())
+  await secondPromise
+})
+
+test('a rejected fetch does not evict the request that replaced it', async () => {
+  let calls = 0
+  let release!: (result: FetchMetadataResult) => void
+  const { fetch, clear } = memoizeFetchMetadata(async () => {
+    calls++
+    if (calls === 1) throw new Error('network down')
+    return new Promise<FetchMetadataResult>((resolve) => {
+      release = resolve
+    })
+  })
+
+  const firstPromise = fetch('foo', { registry: REGISTRY })
+  clear()
+  const secondPromise = fetch('foo', { registry: REGISTRY })
+  await expect(firstPromise).rejects.toThrow('network down')
+
+  // The eviction must leave the second request's entry in place, so a third
+  // caller joins it instead of opening a redundant request.
+  const thirdPromise = fetch('foo', { registry: REGISTRY })
+  expect(calls).toBe(2)
+  release(fooFetchResult())
+  const [second, third] = await Promise.all([secondPromise, thirdPromise])
+  expect(third).toBe(second)
+})
+
 test('clear() empties the cache', async () => {
   let calls = 0
   const { fetch, clear } = memoizeFetchMetadata(async () => {

pnpm11/resolving/npm-resolver/test/metaCache.test.ts

@@ -1,11 +1,13 @@
 import { rmSync } from 'node:fs'
 import { readFile } from 'node:fs/promises'
 
-import { expect, test } from '@jest/globals'
+import { expect, jest, test } from '@jest/globals'
 import { ABBREVIATED_META_DIR } from '@pnpm/constants'
 import type { PackageMeta } from '@pnpm/resolving.registry.types'
 import { temporaryDirectory } from 'tempy'
 
+import type { FetchMetadataOptions } from '../src/fetch.js'
+import { memoizeFetchMetadata } from '../src/memoizeFetchMetadata.js'
 import type { RegistryPackageSpec } from '../src/parseBareSpecifier.js'
 import {
   getPkgMetaCacheKey,
@@ -191,6 +193,52 @@ test('the raw response body is written verbatim to the disk mirror', async () =>
   expect(mirror?.slice(mirror.indexOf('\n') + 1)).toBe(rawBody)
 })
 
+test('projects sharing one in-flight fetch mirror the fetched body instead of re-serializing it', async () => {
+  const meta = fooMeta()
+  const rawBody = JSON.stringify(meta)
+  const cacheDir = temporaryDirectory()
+  const projects = 20
+
+  let fetches = 0
+  let joined = 0
+  let allJoined!: () => void
+  const inFlight = new Promise<void>((resolve) => {
+    allJoined = resolve
+  })
+  const memoized = memoizeFetchMetadata(async () => {
+    fetches++
+    // Hold the request open until every project has joined it, so the fan-out
+    // this guards against is reproduced rather than raced for.
+    await inFlight
+    return { meta, jsonText: rawBody, etag: undefined }
+  })
+  const ctx = {
+    fetch: async (pkgName: string, opts: FetchMetadataOptions) => {
+      if (++joined === projects) allJoined()
+      return memoized.fetch(pkgName, opts)
+    },
+    metaCache: createMetaCache(),
+    cacheDir,
+  }
+  const spec: RegistryPackageSpec = { type: 'range', name: 'foo', fetchSpec: '^1.0.0' }
+
+  const stringifySpy = jest.spyOn(JSON, 'stringify')
+  try {
+    const picks = await Promise.all(Array.from({ length: projects }, async () =>
+      pickPackage(ctx, spec, { registry: REGISTRY, dryRun: false, preferredVersionSelectors: undefined })
+    ))
+    expect(picks.every((pick) => pick.pickedPackage?.version === '1.0.0')).toBe(true)
+    expect(fetches).toBe(1)
+    // Re-serializing per project is what exhausted the heap: the body reaches
+    // tens of MB for a popular package, and every project holds its own copy
+    // until the mirror write limiter drains.
+    const serializations = stringifySpy.mock.calls.filter(([value]) => value === meta).length
+    expect(serializations).toBe(0)
+  } finally {
+    stringifySpy.mockRestore()
+  }
+})
+
 test('a disk-promoted cache entry that cannot satisfy the spec falls back to the registry under prefer-offline', async () => {
   const staleMeta = fooMeta()
   const freshMeta = fooMeta()