Stream not emitting end after removeListenerΒ #9002
Copy link
Copy link
Closed
Labels
questionIssues asking questions about Node.js.Issues asking questions about Node.js.streamIssues and PRs related to Node.js streams.Issues and PRs related to Node.js streams.
Description
- Version: v6.7.0
- Platform: Linux david-Latitude-E6440 4.4.0-38-generic Fix for path.resolve containing relative path to another drive in windowsΒ #57-Ubuntu SMP Tue Sep 6 15:42:33 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
- Subsystem: stream
This program prints "END":
const s = new require('stream').PassThrough();
function readable()
{
console.log(this.read());
}
s.end();
setTimeout(function ()
{
s.on('readable', readable);
s.on('end', function ()
{
console.log("END");
});
}, 1000);whereas this program does not:
const s = new require('stream').PassThrough();
function readable()
{
console.log(this.read());
}
s.on('readable', readable);
s.removeListener('readable', readable);
s.end();
setTimeout(function ()
{
s.on('end', function ()
{
console.log("END");
});
}, 1000);The only difference is that a readable listener is added and then removed before the stream is ended.
My question is whether the behaviour should be the same, given that there is no readable listener in both cases when the stream is ended.
I can see in the code why: At https://github.com/nodejs/node/blob/v6.7.0/lib/_stream_readable.js#L696 readableListening is set to true but it's never reset to false if all readable listeners are removed.
Before making any changes however, I'd like to ask whether this is intended behaviour?
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
questionIssues asking questions about Node.js.Issues asking questions about Node.js.streamIssues and PRs related to Node.js streams.Issues and PRs related to Node.js streams.
Activity
[-]Stream emitting end after removeListener[/-][+]Stream not emitting end after removeListener[/+]mscdex commentedon Oct 10, 2016
/cc @nodejs/streams
mcollina commentedon Oct 10, 2016
hey @davedoesdev!
What you describe is the currently intended behavior. Setting
on('readable')trigger as a side effectread(0). In fact,'end'is emitted straight away, because the read was triggered.This is what is triggered: https://github.com/nodejs/node/blob/v6.7.0/lib/_stream_readable.js#L710-L713
What you want is to "undo" that scheduled
read(0)operation.Β I'm not 100% convinced that we should support this. What's the use case for doing so?There probably is a different bug about
'readableListening'not being reset to false. But fixing it would not prevent the behavior you are describing here.davedoesdev commentedon Oct 10, 2016
I have two separate bits of code (separate modules). One which reads a header from a stream and another which then reads the rest of the stream. The second bit of code when passed the stream (after the first has finished processing) then registers its own
readableandendhandlers. It needs to know when the stream has ended.It's fine, I can get the second bit of code to check if the stream is already ended (using
_readableStateI guess).mcollina commentedon Oct 10, 2016
That is probably the best approach, yes.
davedoesdev commentedon Oct 10, 2016
Thanks for the help @mcollina