Memory leak when returning ReadableStream as Response body

[devcsrj]

Describe the bug

In a +server.ts, returning a Response with body of type ReadableStream works....but does not close the ReadableStream. This causes the stream to stay open, never de-allocated by the GC.

Reproduction

export const GET = async () => {
   const response = await fetch('https://example.com');
   if (!response.ok) {
      error(404, 'Not found');
   }

   const h = response.headers;
   const keys = ['content-type', 'content-length', 'last-modified', 'date', 'etag', 'age'];
   const headers: Record<string, string> = {};
   for (const key of keys) {
      const val = h.get(key);
      if (val) {
	headers[key] = val;
       }
   }
	
   return new Response(response.body, {   // <-- this works fine, but doesn't close `response.body` automatically
      headers
   });
}

Logs

N/A

System Info

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 97.95 MB / 16.00 GB
    Shell: 3.7.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.11.0 - ~/.local/share/nvm/v20.11.0/bin/node
    npm: 10.2.4 - ~/.local/share/nvm/v20.11.0/bin/npm
    pnpm: 9.4.0 - /opt/homebrew/bin/pnpm
    bun: 1.0.0 - ~/.bun/bin/bun
    Watchman: 2024.06.10.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 128.0.6613.137
    Safari: 17.4.1
  npmPackages:
    @sveltejs/adapter-node: ^5.2.2 => 5.2.2 
    @sveltejs/kit: ^2.5.26 => 2.5.26 
    @sveltejs/vite-plugin-svelte: ^3.1.2 => 3.1.2 
    svelte: ^4.2.19 => 4.2.19 
    vite: ^5.4.3 => 5.4.3

Severity

serious, but I can work around it

Additional Information

I discovered this after seeing my app's memory usage trending up. After doing a heapdump, I saw thousands of unreleased Client object (i.e.: the fetch). To test my theory, I changed the code to:

+   const body = await response.arrayBuffer();
+   return new Response(body, {
-   return new Response(response.body, {
      headers
   });
}

This works, which I expected because the arrayBuffer (and json()/text()) consumes the stream completely. However, if the stream is huge, this will pose a problem.

Question: What's the proper way to pass a stream to Response?

[eltigerchino]

Have you tried cloning the response body?

   // Clone the response before passing it to a new Response
   const clonedResponse = response.clone();  // Clone the response safely

   return new Response(clonedResponse.body, {  // Now pipe the cloned body.
      headers
   });

[devcsrj]

Sadly it does not work. On our memory graphs, it went coming trending up again when this snippet is applied

Edit: Does not work as in, it doesn't actually stream the fetch's response body to the returned Response. It would still buffer the entire thing in memory. I haven't confirmed the part on whether the GC de-allocates the Client

[devcsrj]

I have a new attempt:

  const stream = new ReadableStream({
    start(controller) {
      const reader = response.body!.getReader();
	function push() {
	  reader
	    .read()
	    .then(({ done, value }) => {
		if (done) {
			controller.close();
			return;
		}

		controller.enqueue(value);
		push();
	     })
              .catch((err) => {
		controller.error(err);
	     });
	}
	push();
       }
  });

  return new Response(stream, {
    headers
  });

I have yet to verify it the stream is closed properly. I'll get back to this when I do