search

alxhotel / nat-api

:left_right_arrow: Fast port mapping with UPnP and NAT-PMP
MIT License
23 stars 13 forks source link

MaxListenersExceededWarning: Possible EventEmitter memory leak detected #27

Open achingbrain opened 2 years ago

achingbrain commented 2 years ago

After about 10 hours running I start seeing MaxListenersExceededWarning:

node:internal/process/warning:50 (node:81670) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 _device listeners added to [Ssdp]. Use emitter.setMaxListeners() to increase limit
    at _addListener (node:events:465:17)
    at Ssdp.addListener (node:events:487:10)
    at Ssdp.search (/Users/alex/Documents/Workspaces/ipfs/js-ipfs/node_modules/nat-api/lib/upnp/ssdp.js:84:10)
    at Client.findGateway (/Users/alex/Documents/Workspaces/ipfs/js-ipfs/node_modules/nat-api/lib/upnp/index.js:186:25)
    at Client.portMapping (/Users/alex/Documents/Workspaces/ipfs/js-ipfs/node_modules/nat-api/lib/upnp/index.js:23:10)
    at NatAPI._upnpMap (/Users/alex/Documents/Workspaces/ipfs/js-ipfs/node_modules/nat-api/index.js:315:22)
    at listOnTimeout (node:internal/timers:557:17)
    at processTimers (node:internal/timers:500:7)
cascandaliato commented 2 years ago

I believe the problem lies in this method: https://github.com/alxhotel/nat-api/blob/e404453ebca7e386fbd20f8dd7d86b8c9611058e/index.js#L312

Specifically, here: https://github.com/alxhotel/nat-api/blob/e404453ebca7e386fbd20f8dd7d86b8c9611058e/index.js#L327-L332

When autoupdate is enabled (default behavior), _upnpMap gets called at regular intervals and, each time it executes, it adds an additional timer. This means that, over time, you get more and more _upnpMap calls firing at the same time, until you reach a point where the underlying code registers too many listeners in a short amount of time.

Partially related: I feel like the same could happen if you map too many ports with the same TTL. You would have a spike in the number of listeners, except that in this case there is no leak because listeners are correctly unregistered.

@alxhotel, any chance you can have a look at my analysis? I'll be happy to create a PR.