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.

Sign up for GitHub

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

Jump to bottom

Piping a simple program to stdout gives "MaxListenersExceededWarning: Possible EventEmitter memory leak detected when" #16767

Closed
mafintosh opened this issue Nov 5, 2017 · 8 comments
Closed

Piping a simple program to stdout gives "MaxListenersExceededWarning: Possible EventEmitter memory leak detected when" #16767

mafintosh opened this issue Nov 5, 2017 · 8 comments
Labels
console Issues and PRs related to the console subsystem.

Comments

@mafintosh
Copy link
Member

  • Version: v8.9.0
  • Platform: Linux brunhilde 4.13.8-1-ARCH deps: update openssl to 1.0.1j #1 SMP PREEMPT Wed Oct 18 11:49:44 CEST 2017 x86_64 GNU/Linux
  • Subsystem: console.js

When running a program that produces a lot of output that is consumed by another program in the pipeline, Node gives memory leak errors.

To reproduce try saving this simple program:

// save as bug.js
for (var i = 0; i < 10000; i++) {
  console.log('line ' + i)
}

Then run

node bug.js | less

And quit less (type q). This results in the following warning being printed out:

(node:31009) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added. Use emitter.setMaxListeners() to increase limit

@mcollina mentioned it is most likely a bug here, https://github.com/nodejs/node/blob/master/lib/console.js#L91-L119

(In general it would be useful as well if the memory leak errors printed out a stack trace so it was easier to hunt down the causes)

/cc @mcollina @addaleax
@mscdex
Copy link
Contributor

mscdex commented Nov 5, 2017 •
edited
Loading

(In general it would be useful as well if the memory leak errors printed out a stack trace so it was easier to hunt down the causes)

You mean --trace-warnings ?

@addaleax addaleax added the console Issues and PRs related to the console subsystem. label Nov 5, 2017
@mscdex mscdex added the v8.x label Nov 5, 2017
@addaleax
Copy link
Member

addaleax commented Nov 5, 2017

Fwiw, I couldn’t reproduce this with master or with v8.9.0… but yes, --trace-warnings should give a good hint at what’s going on.

@mscdex
Copy link
Contributor

mscdex commented Nov 5, 2017 •
edited
Loading

I'm also not able to reproduce this on Linux.

@mcollina
Copy link
Member

mcollina commented Nov 5, 2017

I think it's this block: https://github.com/nodejs/node/blob/master/lib/console.js#L81-L87.
What I think is happening is that there are a bunch of writes that are about to error, and we add a lot of 'error' in the meanwhile.

@mcollina
Copy link
Member

mcollina commented Nov 5, 2017

I can reproduce it on Mac OS X, you just need to wait some seconds before hitting q within less.

@mcollina
Copy link
Member

mcollina commented Nov 5, 2017

My fix is working, I'll send a PR soon.

@mafintosh
Copy link
Member Author

@mscdex I'm also on linux and can reproduce. --trace-warnings solves it for me but when other users report a memleak or you get a hard to reproduce one it would be nice if a stack happened per default.

@mcollina mcollina mentioned this issue Nov 5, 2017
Closed
4 tasks
mcollina added a commit to mcollina/node that referenced this issue Nov 5, 2017
@mcollina
console: avoid adding infinite error listeners
8608334 
If the console destination is a unix pipe (net.Socket), write() is
async. If the destination is broken, we are adding an 'error' event
listener to avoid a process crash. This PR makes sure that we are adding
that listener only once.

Fixes: nodejs#16767
@mafintosh
Copy link
Member Author

Thanks for the quick turnaround everyone :)

gibfahn pushed a commit that referenced this issue Nov 8, 2017
@mcollina @gibfahn
console: avoid adding infinite error listeners
9df1e8f 
If the console destination is a unix pipe (net.Socket), write() is
async. If the destination is broken, we are adding an 'error' event
listener to avoid a process crash. This PR makes sure that we are adding
that listener only once.

Fixes: #16767

PR-URL: #16770
Fixes: #16767
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
evanlucas pushed a commit that referenced this issue Nov 13, 2017
@mcollina @evanlucas
console: avoid adding infinite error listeners
bed0560 
If the console destination is a unix pipe (net.Socket), write() is
async. If the destination is broken, we are adding an 'error' event
listener to avoid a process crash. This PR makes sure that we are adding
that listener only once.

Fixes: #16767

PR-URL: #16770
Fixes: #16767
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
console Issues and PRs related to the console subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mcollina @mscdex @mafintosh @addaleax