undici fetch has memory leak

[cjh980402]

Bug Description

When I use node-fetch or axios, then there are no memory leak like under screenshot.
However, I found memory leak when I use undici.fetch.

Reproducible By

I made request about 50 times~200times then I found that memory leak easily.

Expected Behavior

there are no memory leak.

Logs & Screenshots

Environment

Ubuntu 20.04
Node v16.13.0

Additional context

[ronag]

Do you have a repro?

[cjh980402]

import { fetch } from 'undici';
import { setTimeout } from 'timers/promises';

const arr = [];
console.log(process.memoryUsage().heapUsed);

for (let i = 0; i < 200; i++) {
    if (i % 10 == 0) {
        console.log(process.memoryUsage().heapUsed);
    }
    for (let j = 0; j < 20; j++) {
        await (await fetch('https://www.youtube.com/')).text();
    }
    arr.push((await fetch('https://naver.com')).text());
}

await Promise.all(arr);
console.log(process.memoryUsage().heapUsed);

await setTimeout(10000);

console.log(process.memoryUsage().heapUsed);

await setTimeout(100000);

console.log(process.memoryUsage().heapUsed);

It has this result.

There are some memory leak so finally memory usage is increased.

[mcollina]

At first glance, your code behaves more or less how I expected. There should not be any memory leaks there.

(I should probably spend some time in documenting how this happen - however I'm a bit short on time right now).

[Ethan-Arrowood]

It would be valuable to try to reproduce this issue with just undici.request to isolate the issue to the fetch implementation.

Some areas within fetch it could happen is in the Body Mixin. We should ensure that once something like .text() is awaited that the stream is closed and destroyed.

[mcollina]

@Ethan-Arrowood My understanding is that there is no issue. All the requests are still referenced in the array and they can't be collected.

[ronag]

@cjh980402 can you try again with the latest release?

[cjh980402]

As a result of testing, it is much more stable than the previous version.
However, I will have to wait and see the situation by applying it to the actual application.

[kyrylkov]

I can confirm quite severe memory leak still present with Node v16.13.1 and undici v4.11.3

The issue is described in Node.js non-heap memory leak.

As soon as we replace undici with node-fetch, the issue disappears.

However do we know if the issue is with undici itself or with Web Streams / Buffer.blob?

Also switching back to node-fetch results in smaller GC pause times and smaller heap utilization. See chart below where just before 6pm node-fetch was deployed instead of unidici.

[ronag]

@kyrylkov: Can you show how you use fetch?

[kyrylkov]

@ronag There are multiple cases. Let me collect some.

[ronag]

Can you try with this branch? https://github.com/nodejs/undici/tree/finalization

[kyrylkov]

  const accessTokenRs = await fetch(URL, {
    method: 'POST',
    headers: {
      'Content-Type': 'application/json',
    },
    body: JSON.stringify(params),
  });
  const accessTokenRsJson = await accessTokenRs.json();

  await fetch(`${url}/lists/${listId}/members/${userHash}`, {
    method: 'DELETE',
    headers: {Authorization: BASIC_AUTH_KEY},
  });

  const res = await fetch(`${url}/lists/${listId}/members`, {
    method: 'GET',
    headers: {Authorization: BASIC_AUTH_KEY},
  });
  const resJson = await res.json();

  const res = await fetch(getUrl, {
    method: 'GET',
    headers:{
      'Authorization': `Bearer ${TOKEN}`,
    },
  });
  const text = await res.text();

[kyrylkov]

Can you try with this branch? https://github.com/nodejs/undici/tree/finalization

Trying....

[ronag]

await fetch(`${url}/lists/${listId}/members/${userHash}`, {
    method: 'DELETE',
    headers: {Authorization: BASIC_AUTH_KEY},
  });

You need to consume the response. See, https://github.com/nodejs/undici#garbage-collection.