Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Warning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added. #11

Closed
gschmottlach-xse opened this issue Feb 21, 2018 · 2 comments

Comments

@gschmottlach-xse
Copy link

This error occurred using npm v5.6.0 in a module that had a postinstall life-cycle step. Consistently I would get this error indicating there are too many process.on('SIGINT', ...) event registrations that aren't being removed. If I remove my postinstall step the error disappeared. If I substituted my postinstall step with an empty script I still received this error.

Digging into:

https://github.com/npm/npm/blob/latest/node_modules/npm-lifecycle/index.js

On line 297 the SIGINT event handler is registered:

process.once('SIGINT', procInterupt)

But on line 318 it removes a listener function for the wrong signal:

process.removeListener('SIGTERM', procInterupt)

Clearly, this is a mistake. It should instead be:

process.removeListener('SIGINT', procInterupt)

I made this change to my local copy of npm and verified that I know longer receive the warning. The SIGINT event function is now properly being removed during the npm lifecycle processing.

Let me know if you have any further questions . . .

@ljharb
Copy link

ljharb commented Feb 21, 2018

It’s not an error, it’s a warning, and it can usually be ignored when it doesn’t indicate a memory leak.

Nice find on the bug tho :-)

@gschmottlach-xse
Copy link
Author

Yes, you are correct. Strictly speaking, this is a warning. The difficulty, of course, is discerning whether the warning can be safely ignored or indicates another issue (or bug). Regardless, I thought it was worth fixing in order to reduce the "noise" during my module install.

Thanks for the prompt investigation and confirmation of the bug . . .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants