Stream not emitting end after removeListener

[davedoesdev]

• 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?

[mscdex]

/cc @nodejs/streams

[mcollina]

hey @davedoesdev!

What you describe is the currently intended behavior. Setting on('readable') trigger as a side effect read(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]

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 readable and end handlers. 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 _readableState I guess).

[mcollina]

That is probably the best approach, yes.

[davedoesdev]

Thanks for the help @mcollina