Repository: vercel/next.js
Pull request: fix: memory leak from cloneResponse

repro info here: https://github.com/snyamathi/nextjs-mem-leak

## Background

> The Fetch Standard allows users to skip consuming the response body by relying on garbage collection to release connection resources.

Undici added support for this in https://github.com/nodejs/undici/pull/3199 and later for the body of cloned responses in https://github.com/nodejs/undici/pull/3458 which first landed in [Undici version 6.19.8](https://github.com/nodejs/undici/commits/v6.19.8) in August 2024. In September 2024, this issue https://github.com/vercel/next.js/discussions/69635 was raised for the error, `TypeError: Response.clone: Body has already been consumed`

In NextJS's [dedupe-fetch](https://github.com/vercel/next.js/blame/ab59e37a66f1a70b1172af9149d8aa3ba1509330/packages/next/src/server/lib/dedupe-fetch.ts#L92) the cloned response is returned to userland, while the original response is stored in a react cache

```js
// match was pulled from react cache, a clone is returned to the user
return match.then((response: Response) => response.clone());
```

## Undici Bug

I'm omitting some details in the code snippet below (see full changes at https://github.com/nodejs/undici/pull/3458/files), but the Undici change to `use FinalizationRegistry for cloned response body` seems to have mixed up the response pointer and stream bodies when registering with the finalization registry, resulting in the wrong stream being cancelled.

The original response, (the `match` above) stored in the react cache is cloned, and then **its** stream is registered with the finalization registry when the cloned response `newRequest` is reclaimed.

**I believe this is the true underlying cause of the errors:** `Body has already been consumed`

```js
function cloneBody(instance, body) {
  const [out1, out2] = body.stream.tee();

  // Erroneously registering newRequest + old body with finalization registry
  streamRegistry.register(instance, new WeakRef(out1));

  // Original request + out1 is used in Next's dedupe-request cache
  body.stream = out1;

  // Clone request + out2 is returned to userland
  return {
    stream: out2,
  };
}

// When newRequest is reclaimed, the original request.body is cancelled by mistake!
newRequest.body = cloneBody(newRequest, request.body);

// This is what the registry looks like
streamRegistry = new FinalizationRegistry((weakRef) => {
  const stream = weakRef.deref();
  if (stream && !stream.locked && !isDisturbed(stream) && !isErrored(stream)) {
    stream.cancel("Response object has been garbage collected").catch(noop);
  }
});
```

https://github.com/snyamathi/nextjs-mem-leak/blob/main/index.mjs has a simple reproduction of this issue which shows that when the clone is reclaimed, the original stream is cancelled, leading to issues

## Memory Leak

This lead to https://github.com/vercel/next.js/pull/73274 which fixed the problem by adding a custom `cloneResponse` function.

https://github.com/vercel/next.js/blob/7ef0a2b2767b4159f8db8e1884bac98370528a25/packages/next/src/server/lib/clone-response.ts

However, this in turn has lead to a memory leak because now there is no one to garbage collect the tee'd stream.

https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/tee

> To cancel the stream you then need to cancel both resulting branches. Teeing a stream will generally lock it for the duration, preventing other readers from locking it.

Undici was incorrectly cancelling the stream (leading to a bug), and Next is simply not cancelling the stream (leading to a memory leak). This can be observed with a Docker setup which shows a current version of NextJS, the last version prior to the custom `cloneResponse` function, as well as the effects of the proposed fix here.

<img width="2400" height="1200" alt="plot" src="https://github.com/user-attachments/assets/a1df0afd-cc8a-4a89-8c92-381ee71fff59" />

I have a reproduction available here https://github.com/snyamathi/nextjs-mem-leak/tree/main - you'll notice the memory for NextJS climb until it runs out of memory and crashes, whereas the version prior to this change (`15.0.4-canary`) is unaffected.

Similarly, if we forego the dedupe cache by passing in a signal (https://github.com/vercel/next.js/blob/102f233a170e9df0b30c24a58c3953dc678ec330/packages/next/src/server/lib/dedupe-fetch.ts#L45) there is no memory leak either.

The PR changes here are applied as a patch and also fix the memory leak :)

```bash
docker compose pull
docker compose build
docker compose up -d
docker container stats
```

```
CONTAINER ID   NAME                          CPU %     MEM USAGE / LIMIT    MEM %     NET I/O           BLOCK I/O   PIDS
1b2f217c4d03   mem-next-og-1                 28.50%    124MiB / 1GiB        12.11%    993MB / 7.45MB    0B / 0B     23
7e76622a4a3e   mem-next-15.4.1-1             49.03%    1021MiB / 1GiB       99.69%    993MB / 7.5MB     0B / 0B     23
1bacb1a9b1cc   mem-next-15.0.4-canary.39-1   21.22%    115.6MiB / 1GiB      11.29%    991MB / 7.44MB    0B / 0B     23
df2a8ba5800e   mem-siege-1                   5.59%     12.56MiB / 31.2GiB   0.04%     9.73MB / 10.5MB   0B / 0B     102
46fab210c911   mem-next-patched-1            2.46%     91.34MiB / 1GiB      8.92%     2.38MB / 1.97MB   0B / 0B     23
365987089d03   mem-upstream-1                15.16%    137.2MiB / 31.2GiB   0.43%     16.8MB / 2.97GB   0B / 0B     23
```

Each container outputs the request number and current memory usage which are then plotted in order to observe the memory leak due to the custom `cloneResponse`.

Because the fix associates the correct response and stream, the previous regression does not happen again. We can confirm this by making requests to the page for each of the docker containers. The container using the version prior to the custom `cloneResponse` will error out, while the rest will not

```
$ curl -s localhost:3002 | htmlq --text '#error'
Response.clone: Body has already been consumed.
```

cc @wyattjoh @gnoff 

## Reproduction

https://github.com/snyamathi/nextjs-mem-leak/tree/main contains a number of reproductions

- https://github.com/snyamathi/nextjs-mem-leak/blob/main/index.mjs shows a pure javascript reproduction of the original undici bug
- https://github.com/snyamathi/nextjs-mem-leak/blob/main/page.js shows how that bug manifested itself in Next for the `Body has already been consumed` error
- https://github.com/snyamathi/nextjs-mem-leak/blob/main/route.js shows the memory leak due to the custom `cloneResponse` function

The rest of the files are supporting infrastructure to start docker containers of various versions and various patches showing how versions prior to the `cloneResponse` do not have the same memory leak, and how the patch (PR'd here) fixes the issue without regressing on the Undici issue.

Changed files:
filename: packages/next/src/server/lib/clone-response.ts
patch: @@ -1,3 +1,16 @@
+const noop = () => {}
+
+let registry: FinalizationRegistry<WeakRef<ReadableStream>> | undefined
+
+if (globalThis.FinalizationRegistry) {
+  registry = new FinalizationRegistry((weakRef: WeakRef<ReadableStream>) => {
+    const stream = weakRef.deref()
+    if (stream && !stream.locked) {
+      stream.cancel('Response object has been garbage collected').then(noop)
+    }
+  })
+}
+
 /**
  * Clones a response by teeing the body so we can return two independent
  * ReadableStreams from it. This avoids the bug in the undici library around
@@ -33,6 +46,24 @@ export function cloneResponse(original: Response): [Response, Response] {
     writable: false,
   })
 
+  // The Fetch Standard allows users to skip consuming the response body by
+  // relying on garbage collection to release connection resources.
+  // https://github.com/nodejs/undici?tab=readme-ov-file#garbage-collection
+  //
+  // To cancel the stream you then need to cancel both resulting branches.
+  // Teeing a stream will generally lock it for the duration, preventing other
+  // readers from locking it.
+  // https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/tee
+
+  // cloned2 is stored in a react cache and cloned for subsequent requests.
+  // It is the original request, and is is garbage collected by a
+  // FinalizationRegistry in Undici, but since we're tee-ing the stream
+  // ourselves, we need to cancel clone1's stream (the response returned from
+  // our dedupe fetch) when clone1 is reclaimed, otherwise we leak memory.
+  if (registry && cloned1.body) {
+    registry.register(cloned1, new WeakRef(cloned1.body))
+  }
+
   const cloned2 = new Response(body2, {
     status: original.status,
     statusText: original.statusText,