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

MaxListenersExceededWarning with Stream.pipeline() #28432

Closed
ehmicky opened this issue Jun 26, 2019 · 9 comments
Closed

MaxListenersExceededWarning with Stream.pipeline() #28432

ehmicky opened this issue Jun 26, 2019 · 9 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@ehmicky
Copy link

ehmicky commented Jun 26, 2019

  • Version: v12.4.0
  • Platform: Linux my-laptop 5.0.0-19-generic #20-Ubuntu SMP Wed Jun 19 17:04:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: stream

index.js:

const { Readable, Writable, pipeline } = require('stream')
const readable = new Readable()
for (let i = 0; i <= 5; ++i) {
	pipeline(readable, new Writable(), () => {})
}
$ node index.js
(node:19205) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 close listeners added to [Readable]. Use emitter.setMaxListeners() to increase limit
(node:19205) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [Readable]. Use emitter.setMaxListeners() to increase limit

This happens even when pipeline() is called after readable has ended and been destroyed. Once a stream has ended, its pipeline() callback should be called then its listeners should be cleaned up. However this is not the case at the moment: eos() returns a cleanup function but this function is never called.

@Trott Trott added the stream Issues and PRs related to the stream subsystem. label Jun 26, 2019
@himself65
Copy link
Member

C:\Users\Himself65\Desktop\github\test>nvm use 12.16.1
Now using node v12.16.1 (64-bit)

C:\Users\Himself65\Desktop\github\test>node bundle.js
(node:20800) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 close listeners added to [Readable]. Use emitter.setMaxListeners() to incr
ease limit
(node:20800) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [Readable]. Use emitter.setMaxListeners() to increa
se limit

C:\Users\Himself65\Desktop\github\test>nvm use 13.10.1
Now using node v13.10.1 (64-bit)

C:\Users\Himself65\Desktop\github\test>node bundle.js
(node:21880) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [Readable]. Use emitter.setMaxListeners() to increa
se limit

@ronag
Copy link
Member

ronag commented Mar 11, 2020

I believe this was fixed in d016b9d which is labeled as semver-major and as it looks now won't make it into 12.x.

@ehmicky
Copy link
Author

ehmicky commented Mar 11, 2020

Thanks for the update!

Trying with 13.10.1 (which includes d016b9d) and getting the same result: there is still one MaxListenersExceededWarning. Shouldn't there be none?

@ronag

This comment has been minimized.

@ronag
Copy link
Member

ronag commented Mar 11, 2020

Hm, maybe you are right, it doesn't unregister the event listeners in this case.

@ronag
Copy link
Member

ronag commented Mar 11, 2020

@ehmicky: The docs say:

stream.pipeline() leaves dangling event listeners on the streams after the callback has been invoked. In the case of reuse of streams after failure, this can cause event listener leaks and swallowed errors.

@ronag
Copy link
Member

ronag commented Mar 11, 2020

This can partly be fixed. However, it still needs to leave an 'error' listener on the streams (which kind of defeats any other fix) in order to not break lots of other stuff.

@ehmicky
Copy link
Author

ehmicky commented Mar 11, 2020

Thanks. If I understand you correctly, this is intended and this issue can be closed?

@ronag
Copy link
Member

ronag commented Mar 11, 2020

Thanks. If I understand you correctly, this is intended and this issue can be closed?

Yes, it's an expected behavior.

@ronag ronag closed this as completed Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants