Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

stream: Properly handle large reads from push-streams

Problem 1: If stream.push() triggers a 'readable' event, and the user
calls `read(n)` with some n > the highWaterMark, then the push() will
return false (indicating that they should not push any more), but no
future 'readable' event is coming (because we're above the
highWaterMark).

Solution: return true from push() when needReadable is set.

Problem 2: A read(n) for n != 0, after the stream had encountered an
EOF, would not trigger the 'end' event if the EOF was pushed in
synchronously by the _read() function.

Solution: Check for ended in stream.read() and schedule an end event if
the length now equals 0.

Fix #4585
  • Loading branch information...
commit 7d97d92cef9b9c7d90dac0da2a2315d0aa50c715 1 parent 9668df8
@isaacs authored
View
15 lib/_stream_readable.js
@@ -108,8 +108,13 @@ Readable.prototype.push = function(chunk) {
// if it's past the high water mark, we can push in some more.
// Also, if it's still within the lowWaterMark, we can stand some
// more bytes. This is to work around cases where hwm=0 and
- // lwm=0, such as the repl.
- return rs.length < rs.highWaterMark || rs.length <= rs.lowWaterMark;
+ // lwm=0, such as the repl. Also, if the push() triggered a
+ // readable event, and the user called read(largeNumber) such that
+ // needReadable was set, then we ought to push more, so that another
+ // 'readable' event will be triggered.
+ return rs.needReadable ||
+ rs.length < rs.highWaterMark ||
+ rs.length <= rs.lowWaterMark;
};
// backwards compatibility.
@@ -228,6 +233,12 @@ Readable.prototype.read = function(n) {
if (state.length === 0 && !state.ended)
state.needReadable = true;
+ // If we happened to read() exactly the remaining amount in the
+ // buffer, and the EOF has been seen at this point, then make sure
+ // that we emit 'end' on the very next tick.
+ if (state.ended && !state.endEmitted && state.length === 0)
+ endReadable(this);
+
return ret;
};
View
82 test/simple/test-stream2-large-read-stall.js
@@ -0,0 +1,82 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common.js');
+var assert = require('assert');
+
+// If everything aligns so that you do a read(n) of exactly the
+// remaining buffer, then make sure that 'end' still emits.
+
+var READSIZE = 100;
+var PUSHSIZE = 20;
+var PUSHCOUNT = 1000;
+var HWM = 50;
+
+var Readable = require('stream').Readable;
+var r = new Readable({
+ highWaterMark: HWM
+});
+var rs = r._readableState;
+
+r._read = push;
+
+r.on('readable', function() {
+ console.error('>> readable');
+ do {
+ console.error(' > read(%d)', READSIZE);
+ var ret = r.read(READSIZE);
+ console.error(' < %j (%d remain)', ret && ret.length, rs.length);
+ } while (ret && ret.length === READSIZE);
+
+ console.error('<< after read()',
+ ret && ret.length,
+ rs.needReadable,
+ rs.length);
+});
+
+var endEmitted = false;
+r.on('end', function() {
+ endEmitted = true;
+ console.error('end');
+});
+
+var pushes = 0;
+function push() {
+ if (pushes > PUSHCOUNT)
+ return;
+
+ if (pushes++ === PUSHCOUNT) {
+ console.error(' push(EOF)');
+ return r.push(null);
+ }
+
+ console.error(' push #%d', pushes);
+ if (r.push(new Buffer(PUSHSIZE)))
+ setTimeout(push);
+}
+
+// start the flow
+var ret = r.read(0);
+
+process.on('exit', function() {
+ assert.equal(pushes, PUSHCOUNT + 1);
+ assert(endEmitted);
+});
Please sign in to comment.
Something went wrong with that request. Please try again.