joyent/node — pull request #4838 by kanongil (2013-02-23 16:50:00+00:00)
Title: stream: Fix schizophrenic _read() callback behavior

There is a need to clarify the relationship between `push()`, `_read()`, and its callback. I proposes the attached patch, which tries to limit the changes needed for existing extensions.

**Reasoning**
Currently, the `read()` callback is used inconsistently when combined with `push()`. If `push()` is used, it will signal that any pending `_read()` has happened, resulting in a new `_read()` call before any async callbacks have been handled.

This is illustrated in the attached revision to the `test-stream-push-order.js` test which fails on master.

The problem can be fixed by always waiting for the callback before calling _read() again but this would break the pattern described in the `push()` documentation, since it never calls the callback.

**The patch**
With this patch, every time `_read(n, cb)` is called you are expected to eventually call the callback before another `_read()` will be called.

As a new option, you when you omit the callback in your `_read()` function definition the Readable will infer a sync callback with empty data right after you return. This allows the `push()` pattern to continue working by simply removing the arguments.

As a sidenote, this also fixes `_read()` being called incorrectly during `push()`'es.