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

EventEmitter memory leak #7

Closed
75lb opened this issue Jun 9, 2014 · 15 comments
Closed

EventEmitter memory leak #7

75lb opened this issue Jun 9, 2014 · 15 comments
Assignees
Labels

Comments

@75lb
Copy link

75lb commented Jun 9, 2014

if you save this script as index.js and run it:

var connect = require('connect');
var serveStatic = require('serve-static');

var app = connect();

app.use(serveStatic('.'));
app.listen(8000);

then browse to http://localhost:8000/index.js and with the cache disabled refresh the page 9+ times you get this error:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Socket.EventEmitter.addListener (events.js:160:15)
    at Socket.Readable.on (_stream_readable.js:688:33)
    at module.exports (/Users/Lloyd/Documents/75lb/tmp/node_modules/serve-static/node_modules/send/node_modules/finished/index.js:13:10)
    at SendStream.stream (/Users/Lloyd/Documents/75lb/tmp/node_modules/serve-static/node_modules/send/lib/send.js:526:3)
    at SendStream.send (/Users/Lloyd/Documents/75lb/tmp/node_modules/serve-static/node_modules/send/lib/send.js:470:8)
    at /Users/Lloyd/Documents/75lb/tmp/node_modules/serve-static/node_modules/send/lib/send.js:382:10
    at Object.oncomplete (fs.js:107:15)
@75lb
Copy link
Author

75lb commented Jun 9, 2014

v1.1.0 seems ok

@dougwilson
Copy link
Contributor

So does this mean I can close it, since a new version does not have the issue?

@75lb
Copy link
Author

75lb commented Jun 9, 2014

v1.1.0 is an old version - i rolled back to it..

@dougwilson
Copy link
Contributor

OK. So you are saying it worked in v1.1.0, but doesn't work now, I get it :)

@dougwilson
Copy link
Contributor

The version after 1.1.0 is when the finished started to be used, which I see it is causing the emitter leak in the stack trace. I believe there is an open issue regarding that in that project.

@dougwilson dougwilson added the bug label Jun 9, 2014
@75lb
Copy link
Author

75lb commented Jun 9, 2014

exactly.. the issue is with releases 1.2.0 and 1.2.1.. 1.1.0 was the first working release i could find

@75lb
Copy link
Author

75lb commented Jun 9, 2014

yeah.. could something could be done in send to avoid the onFinish being repeatedly added?

@dougwilson
Copy link
Contributor

could something could be done in send to avoid the onFinish being repeatedly added

It's only added once per request, as far as I know.

@dougwilson
Copy link
Contributor

I'll try to figure out a fix for this today :)

@dougwilson dougwilson self-assigned this Jun 9, 2014
@75lb
Copy link
Author

75lb commented Jun 9, 2014

good news :)

@dougwilson
Copy link
Contributor

I have a fix in send, so I'll be working it down stream now :)

@dougwilson
Copy link
Contributor

@75lb can you test the current version of master to see if the issue is gone for you? npm install expressjs/serve-static

@75lb
Copy link
Author

75lb commented Jun 9, 2014

boom shake the room! works fine for me.. thanks for the quick turnaround

@dougwilson
Copy link
Contributor

Thanks. I'm releasing a patch version now that I have your confirmation :)

@dougwilson
Copy link
Contributor

thanks for the quick turnaround

I have your example that allowed me to reproduce to thank :)

@expressjs expressjs locked and limited conversation to collaborators Jul 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants