Repository navigation
streams: calling end while ending will never invoke callback? #28667
Description
Activity
http/1 has the same "problem"
The callback is added as a listener of the
'finish'event so the behavior seems correct to me. The event is emitted only once.There is no memory leak because the callback is used only on the first
.end()call.I think adding an error to the callback if the stream has already emitted end could be accepted. We need to verify this would not break CITGM.
There should be enough state on Readable around to easily add a check.
I think adding an error to the callback if the stream has already emitted end could be accepted
You mean
'finish'? And what would you do with all callbacks between the firstwritable.end()and the actual'finish'event? For example.writable.on('finish', () => { writable.end(() => { // Called with an error. }); }); writable.end(() => { // Called when `'finish'` is emitted. }); writable.end(() => { // Called when `'finish'` is emitted? }); writable.end(() => { // Called when `'finish'` is emitted? }); // ...
https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L592
if
endingthe callback is never registered anywhere?Yes.
maybe?
writable.on('finish', () => { writable.end((err) => { // error }); }); writable.end((err) => { // ok }); writable.end((err) => { // error }); writable.end((err) => { // error });
I'm unsure...
- added a commit that references this issue
on Aug 20, 2019 This has been sorted
Calling
end()twice with callback will cause the second callback to never be invoked:e.g. the following will fail.
I'm not sure what the behavior should be here. Maybe calling the callback with an error? Either way, not calling the callback at all seems to me like it will cause problems and memory leaks.