jmm / pathmodify

In a nutshell, rewrite (AKA alias, map) `require()` IDs / paths / directories to different values in browserify.
MIT License
27 stars 1 forks source link

EventEmitter memory leak detected #6

Closed panzerdp closed 9 years ago

panzerdp commented 9 years ago

When using pathmodify on multiple bundles with watchify, sometimes I receive this error:

(node) warning: possible EventEmitter memory leak detected. 11 reset listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Browserify.addListener (events.js:239:17)
    at pathmodify (/home/dm/Projects/clipboardy-extension/node_modules/pathmodify/pathmodify.js:41:5)
    at Browserify.plugin (/home/dm/Projects/clipboardy-extension/node_modules/browserify/index.js:349:9)
    at Browserify.bundle (/home/dm/Projects/clipboardy-extension/gulpfile.js:111:10)
    at emitOne (events.js:77:13)
    at Browserify.emit (events.js:169:7)
    at notify [as _onTimeout] (/home/dm/Projects/clipboardy-extension/node_modules/watchify/index.js:149:15)
    at Timer.listOnTimeout (timers.js:89:15)

Is it something on the pathmodify side?

Thanks.

jmm commented 9 years ago

@panzerdp It's probably not really anything intrinsic to pathmodify. Can you show me the relevant code from your gulpfile? You're not calling b.plugin() in your update handler by chance, are you?

panzerdp commented 9 years ago

@jmm You're right, the problem was in calling watch.plugin(pathmodify, options) in update callback. Thanks four your help!

panzerdp commented 9 years ago

@jmm Probably it would be good to add a verification when plugin is attaching multiple times on the same watchify / browserify instance. Sometimes these kind of errors are hard to see :).

jmm commented 9 years ago

@panzerdp You're welcome.

I'll think over your suggestion. It's also fairly common for people to do that with transforms where that kind of state doesn't exist (the transform doesn't receive the browserify instance) and it results in multiple passes of the same transform running on each bundle.

panzerdp commented 9 years ago

@jmm After some evaluation on the issue, I think it doesn't make sense to add this verification. This will break the possibility to add many pathmodify instances with different options on the same watchify/browserify. The current warning: possible EventEmitter memory leak detected should be enough for understanding the problem. Thank you.

jmm commented 9 years ago

@panzerdp Ok, thanks for the feedback. If that warning arises from unintentionally calling b.plugin() in the update handler it's probably better overall not to squelch it as it could mask other problems -- like calling b.transform() there.