Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Long-lived AbortSignals and undici cause MaxListenersExceededWarnings #3157

Closed
achingbrain opened this issue Apr 22, 2024 · 0 comments · Fixed by #3158
Closed

Long-lived AbortSignals and undici cause MaxListenersExceededWarnings #3157

achingbrain opened this issue Apr 22, 2024 · 0 comments · Fixed by #3158
Labels
bug Something isn't working

Comments

@achingbrain
Copy link

achingbrain commented Apr 22, 2024

Version

v20.11.0

Platform

Darwin MacBook-Pro-194.localdomain 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000 arm64

Subsystem

undici

What steps will reproduce the bug?

Making multiple fetch requests with the same AbortSignal causes MaxListenersExceededWarnings to appear in the console with a high number of listeners.

If undici is adding event listeners without removing them, this is a memory leak.

import http from 'http'

const PORT = 49823

http.createServer((req, res) => {
  res.writeHead(200, {'Content-Type': 'text/plain'})
  res.write('Hello World!')
  res.end()
}).listen(PORT)

const controller = new AbortController()

for (let i = 0; i < 200; i++) {
  // make request
  const res = await fetch(`http://localhost:${PORT}`, {
    signal: controller.signal
  })

  // drain body
  await res.text()

  // in theory any added event listeners added by undici should/could be removed
  // now as the request has finished so there's nothing left to abort.
}

There is a repro repo here: https://github.com/achingbrain/node-fetch-maxlistenersexceededwarning

How often does it reproduce? Is there a required condition?

Every time. No required condition.

What is the expected behavior? Why is that the expected behavior?

Undici should remove any added event listeners when there is nothing left to abort (e.g. the request has ended and so either errored or the body has been consumed/cancelled).

What do you see instead?

(node:37396) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 101 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
(node:37396) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 102 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
(node:37396) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 103 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
...etc

Additional information

A workaround appears to be to create a request-specific signal and abort it if the long lived signal aborts, taking care to remove any added listeners on the long-lived signal when we are done with the request.

See: https://github.com/achingbrain/node-fetch-maxlistenersexceededwarning/blob/main/workaround.js

This may be a duplicate of nodejs/node#52203 - please close this issue if you consider it so, but the workaround above may be of use to someone if there's no way to get undici to clean up after itself.

achingbrain referenced this issue in ipfs/helia Apr 22, 2024
Node's `undici` module appears to be adding listeners for the `"abort"` event on any passed AbortSignal but then not cleaning them up.

This casues many `MaxListenersExceededWarning`s to appear in the console and likely causes a memory leak.

To work around this, use a short-lived `AbortSignal` for each request that can be garbage collected along with the request itself.

See https://github.com/nodejs/node/issues/52635 for more information.
achingbrain referenced this issue in ipfs/helia Apr 22, 2024
Node's `undici` module appears to be adding listeners for the `"abort"` event on any passed AbortSignal but then not cleaning them up.

This casues many `MaxListenersExceededWarning`s to appear in the console and likely causes a memory leak.

To work around this, use a short-lived `AbortSignal` for each request that can be garbage collected along with the request itself.

See https://github.com/nodejs/node/issues/52635 for more information.
@VoltrexKeyva VoltrexKeyva changed the title Long-lived AbortSignals and unidici cause MaxListenersExceededWarnings Long-lived AbortSignals and undici cause MaxListenersExceededWarnings Apr 22, 2024
achingbrain referenced this issue in ipfs/helia Apr 22, 2024
Node's `undici` module appears to be adding listeners for the `"abort"` event on any passed AbortSignal but then not cleaning them up.

This casues many `MaxListenersExceededWarning`s to appear in the console and likely causes a memory leak.

To work around this, use a short-lived `AbortSignal` for each request that can be garbage collected along with the request itself.

See https://github.com/nodejs/node/issues/52635 for more information.
@aduh95 aduh95 transferred this issue from nodejs/node Apr 24, 2024
@mcollina mcollina added the bug Something isn't working label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants