Event listeners should be cleaned up (MaxListenersExceededWarning - possible memory leak detected)

[prokopsimek]

We develop an app called DX Scanner using npm-check-updates to detect if the user has updated all dependencies or not. The DX Scanner can scan many repositories/projects simultaneously (see Promise.all) (ncu.run call).

I run the DX Scanner with a cmd yarn start https://github.com/yarnpkg/yarn and the app throws a warning
MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. Use emitter.setMaxListeners() to increase limit
so I was looking for the reason. The reason is

process.on('unhandledRejection')

here: https://github.com/tjunnone/npm-check-updates/blob/50b444ebec7f970bef34cb6d36eb1ace821116d0/lib/npm-check-updates.js#L389-L391
It can happen that more than 20 parallel scans run at the same time so the ncu runs out all 11 event listeners.

More about the behavior of the process.on in a for loop: https://stackoverflow.com/a/38225520/10826693 and https://stackoverflow.com/a/44446859/10826693.
I tried to change .on to .once but it didn't help.

I really don't want to set maxListeners to a higher value. I believe that it has a good reason to be set just for 11.

I was searching for some solutions and I have found an article about the removal of all listeners.

I would recommend to add process.removeAllListeners('uncaughtException'); to the end of run function in npm-check-updates but I am not sure about it. It's not possible to guarantee that all uncaughtException in a process is really from ncu.

I tried to remove those 3 lines in my node_modules/npm-check-updates folder and it works correctly.

Do you have any other idea how to solve this issue on the side of npm-check-updates?

[prokopsimek]

Proposal 2: Move all code around the process.on('unhandledRejection') to separated function and export that new function to use programmatically. The run function can be still the main function for CLI usage.

[raineorshine]

Thank you for the detailed report. What happens if you move the process.on('unhandledRejection') outside the run function? This would theoretically create a single listener no matter how many times run is called.

[prokopsimek]

@raineorshine It's ok if I changed it as you wrote.

Before:
https://github.com/tjunnone/npm-check-updates/blob/50b444ebec7f970bef34cb6d36eb1ace821116d0/lib/npm-check-updates.js#L385-L391

After:

// exit with non-zero error code when there is an unhandled promise rejection
process.on('unhandledRejection', err => {
  throw err;
});

/** main entry point */
async function run(options={}) {

[raineorshine]

Great! Would you be willing to submit a PR?

[prokopsimek]

@raineorshine I would! :)