Missing "readable" event after some IO has occurred

[mjackson]

This issue is a continuation of a discussion that we're having on the mailing list.

When I get a new IncomingMessage from an http.get, and I wait for a bit, I consistently miss the readable event even though there are no other listeners already registered for that event. The following example illustrates the issue I'm seeing on node 0.10.1:

var http = require('http');

http.get('http://joyent.com/', function (response) {
  console.log('got response with status ' + response.statusCode);
  console.log('number of readable listeners: ' + response.listeners('readable').length);

  setTimeout(function () {
    response.on('readable', function () {
      console.log('readable');
    });

    response.on('end', function () {
      console.log('end');
    });
  }, 10);
});

As you can see from the example, I'm checking to make sure that there's nobody else listening for the readable event, so the stream shouldn't have already emitted it before I setup my handler inside the timeout. But I never get the readable event at all, or the end event. Instead, the process just exits.

The odd thing about this code is that I can sometimes get the readable event to fire if I use a very small timeout (like 1ms).

[isaacs]

Aha!!! This is an interesting edge case.

My point on the mailing list is that you don't have to do stream.read(0) to start the flow - adding a readable handler is enough. However, if the stream has already done some stream.push(chunk) then it already emitted a readable event, and you'll miss it. I was overlooking that case.

I consider this a bug, and I think it's easy to fix.

[mjackson]

@isaacs <3 thanks!

FWIW I took a stab at this last night. I tried setting the needReadable flag to true inside the readable.on handler when the event type is readable, but it didn't get me anywhere. After digging around a bit I found that the readable event was being triggered by someone else calling stream.push(chunk), so you're right about that.

[mjackson]

Thanks! I can confirm that the test cases you added to address this specific issue are passing.

However, we're also missing readable events when a stream has not yet reached its highWaterMark but has received an EOF. I'm not sure if that's supported or not, but it seems like it should be given the way streams already work. I added a failing test and am happy to discuss this in a separate issue if you'd like.

[isaacs]

Awesome, thanks for the failing test. I'll check that out and update the pull, probably tomorrow.

[isaacs]

@mjijackson Failing test fixed on 582c5b1. It's sufficiently different that it should be a separate commit, imo. Added to pull req.

[isaacs]

Fixed on eafa902 and 929e4d9. Thanks!

[mjackson]

Thanks @isaacs! Sorry for the slow response.