Memory leak if add external fetch inside load function

[stalkerg]

Describe the bug

Very fast memory leak, basically we hold all response strings and all fetch data from remote servers. It seems like because we have circular links for abort callback inside node-fetch(it uses WeakRef and etc, but it's not working).

Reproduction

Create a new test project from the starter and add into +layout.server.js the load function:

import { error } from '@sveltejs/kit';

/** @type {import('./$types').LayoutServerLoad} */
export async function load({ fetch }) {
  
  let res;
  try {
    res = await fetch(
      `http://localhost:8989/api/v3/profile?lang=en`,
      {
        credentials: 'include',
      },
    );
  } catch (err) {
    console.error(err);
    return;
  }

  if (res.status !== 200) {
    console.error('Could not load profile', res.status, res.url);
    throw error(500, 'Could not load profile');
  }

  const { env, user } = await res.json();

  return {
    sessionUser: user,
    env,
  };
}

it's can be any other external (outside sveltekit) request. And run wrk or ab over any page. You can build it for the node adapter and run it or it works even in dev. I am use next command to debug: ORIGIN=http://localhost:3000 /usr/bin/node --es-module-specifier-resolution=node --max_old_space_size=200 --inspect --expose-gc build/

Logs

No response

System Info

System:
    OS: Linux 6.1 Gentoo Linux
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
    Memory: 10.09 GB / 31.26 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.12.1 - /usr/bin/node
    npm: 8.19.2 - /usr/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0 => 1.0.0 
    @sveltejs/adapter-node: ^1.0.0 => 1.0.0 
    @sveltejs/kit: ^1.0.0 => 1.0.1 
    svelte: ^3.54.0 => 3.55.0 
    vite: ^4.0.0 => 4.0.2

Severity

blocking all usage of SvelteKit

Additional Information

I found this after deploying my project to production... now I am in a really bad situation because nodejs process is restarted each 30mins.

[stalkerg]

If I am using fetch from the node directly instead load function argument - this issue is gone.

[stalkerg]

hmm because of this https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/node/polyfills.js#L1 even if nodejs support fetch we are using undici implementation.

[dummdidumm]

hmm because of this https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/node/polyfills.js#L1 even if nodejs support fetch we are using undici implementation.

The native fetch implementation of node is undici, so there's no difference (except the undici version which is newer through the polyfill)

[stalkerg]

okey, I found the root of the leak it's how we tricky use handleFetch here https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/server/fetch.js#L25

if I return our internal fetch as is, it's working fine, but if by: (({ request, fetch }) => fetch(request)) it's leaked.

[stalkerg]

sorry, it's a little bit different: if I use return await fetch(info, init) it has no leak but if I start using return await fetch(original_request) it's leaked

I am hacking now create_fetch function.

UPDATE: yes, even this stub is leak:

export function create_fetch({ event, options, state, get_cookie_header }) {
	return async (info, init) => {
		return await fetch(new Request(info, init));
	};
}

but this has not leaked:

export function create_fetch({ event, options, state, get_cookie_header }) {
	return async (info, init) => {
		return await fetch(info, init);
	};
}

it means Request itself leaked or how fetch working with Request.

[stalkerg]

okey, hot fix is here https://github.com/nodejs/undici/pull/1824 seems like it's a serious issue, and we must wait for a new release.

[machak]

this should be fixed in https://github.com/nodejs/undici/releases/tag/v5.15.0

[stalkerg]

Cool! Now we should update deps for kit.

[dummdidumm]

Happened in #8459, release soon

[stalkerg]

They break it again https://github.com/nodejs/undici/pull/2000

Can we revert undici update? @dummdidumm