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

Abort signal - Possible EventEmitter memory leak detected. 11 abort listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit #1716

Closed
ronag opened this issue Jun 21, 2022 · 9 comments

Comments

@ronag
Copy link

ronag commented Jun 21, 2022

When using abort signal with the esc client I get the following:

Possible EventEmitter memory leak detected. 11 abort listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

Is esc or maybe undici missing to release the abort signal handler?

@ronag
Copy link
Author

ronag commented Jun 21, 2022

I'm trying to get more info.

@ronag
Copy link
Author

ronag commented Jun 21, 2022

Might be related to pipelining.

@netlob
Copy link

netlob commented Jun 26, 2022

Getting this warning too 👍

@ronag
Copy link
Author

ronag commented Jun 27, 2022

So this has nothing to do with abort signal. It has to do with number of concurrent requests without signals.

https://github.com/elastic/elastic-transport-js/blob/main/src/connection/UndiciConnection.ts#L126

Every request without signal adds an abort listener to the connection. This is not optimal...

@Czpla
Copy link

Czpla commented Jun 28, 2022

Estou recebendo o mesmo aviso aqui também.

Não sei se essa issue pode ajudar em algo: node-fetch/node-fetch#1295

@slavaGanzin
Copy link

(node:24750) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 abort listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit
MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 abort listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit
    at _addListener (node:events:585:17)
    at EventEmitter.addListener (node:events:603:10)
    at addSignal (node_modules/undici/lib/api/abort-signal.js:35:19)
    at new RequestHandler (node_modules/undici/lib/api/api-request.js:68:5)
    at Pool.request (node_modules/undici/lib/api/api-request.js:170:25)
    at node_modules/undici/lib/api/api-request.js:163:15
    at new Promise (<anonymous>)
    at Pool.request (/node_modules/undici/lib/api/api-request.js:162:12)
    at Connection.request (node_modules/@elastic/transport/lib/connection/UndiciConnection.js:143:41)
    at SniffingTransport.request (node_modules/@elastic/transport/lib/Transport.js:412:75)
    at Client.GetApi [as get] (node_modules/@elastic/elasticsearch/lib/api/api/get.js:36:33)
    // my code
{
  emitter: EventEmitter {
    _events: [Object: null prototype] { abort: [Array] },
    _eventsCount: 1,
    _maxListeners: undefined,
    [Symbol(kCapture)]: false
  },
  type: 'abort',
  count: 11
}

@JoshMock
Copy link
Member

This was fixed and released in v8.8.1. See elastic/elastic-transport-js#63.

@daveyarwood
Copy link

daveyarwood commented Apr 16, 2024

I'm running into this currently, using @elastic/elasticsearch 8.13.1 and @elastic/transport 8.5.1. It seems the issue is unresolved.

See nodejs/undici#3131

@JoshMock
Copy link
Member

Thanks for linking to that issue @daveyarwood. I'll take another look and continue to keep track over on elastic/elastic-transport-js#63.

JoshMock added a commit to elastic/elastic-transport-js that referenced this issue May 1, 2024
A potential fix for
#63, largely
inspired by a community member's PR that was never merged:
#55

According to an Undici core committer in this comment
elastic/elasticsearch-js#1716 (comment)
the issue that triggers the MaxListenersExceededWarning, and possibly a
memory leak in some cases, is caused by attaching an EventEmitter to
each request by default when a per-request timeout is set, rather than
attaching an AbortSignal.

My assumption is that an EventEmitter was used because AbortSignal and
AbortController were not added to Node.js until v14.17.0, so we couldn't
guarantee v14 users would have it. I'm not certain why using
EventEmitters makes a difference memory-wise, but it does get rid of the
MaxListenersExceededWarning.
JoshMock added a commit to elastic/elastic-transport-js that referenced this issue May 21, 2024
A potential fix for
#63, largely
inspired by a community member's PR that was never merged:
#55

According to an Undici core committer in this comment
elastic/elasticsearch-js#1716 (comment)
the issue that triggers the MaxListenersExceededWarning, and possibly a
memory leak in some cases, is caused by attaching an EventEmitter to
each request by default when a per-request timeout is set, rather than
attaching an AbortSignal.

My assumption is that an EventEmitter was used because AbortSignal and
AbortController were not added to Node.js until v14.17.0, so we couldn't
guarantee v14 users would have it. I'm not certain why using
EventEmitters makes a difference memory-wise, but it does get rid of the
MaxListenersExceededWarning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants