Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

stream: Avoid nextTick warning filling read buffer

In the function that pre-emptively fills the Readable queue, it relies
on a recursion through:

stream.push(chunk) ->
maybeReadMore(stream, state) ->
  if (not reading more and < hwm) stream.read(0) ->
stream._read() ->
stream.push(chunk) -> repeat.

Since this was only calling read() a single time, and then relying on a
future nextTick to collect more data, it ends up causing a nextTick
recursion error (and potentially a RangeError, even) if you have a very
high highWaterMark, and are getting very small chunks pushed
synchronously in _read (as happens with TLS, or many simple test
streams).

This change implements a new approach, so that read(0) is called
repeatedly as long as it is effective (that is, the length keeps
increasing), and thus quickly fills up the buffer for streams such as
these, without any stacks overflowing.
  • Loading branch information...
commit cd2b9f542c34ce59d2df7820a6237f7911c702f3 1 parent 738347b
Isaac Z. Schlueter authored
12 lib/_stream_readable.js
@@ -386,17 +386,23 @@ function maybeReadMore(stream, state) {
386 386 if (!state.readingMore) {
387 387 state.readingMore = true;
388 388 process.nextTick(function() {
389   - state.readingMore = false;
390 389 maybeReadMore_(stream, state);
391 390 });
392 391 }
393 392 }
394 393
395 394 function maybeReadMore_(stream, state) {
396   - if (!state.reading && !state.flowing && !state.ended &&
397   - state.length < state.highWaterMark) {
  395 + var len = state.length;
  396 + while (!state.reading && !state.flowing && !state.ended &&
  397 + state.length < state.highWaterMark) {
398 398 stream.read(0);
  399 + if (len === state.length)
  400 + // didn't get any data, stop spinning.
  401 + break;
  402 + else
  403 + len = state.length;
399 404 }
  405 + state.readingMore = false;
400 406 }
401 407
402 408 // abstract method. to be overridden in specific implementation classes.
15 test/simple/test-stream2-unpipe-leak.js
@@ -24,6 +24,8 @@ var common = require('../common.js');
24 24 var assert = require('assert');
25 25 var stream = require('stream');
26 26
  27 +var chunk = new Buffer('hallo');
  28 +
27 29 var util = require('util');
28 30
29 31 function TestWriter() {
@@ -37,13 +39,15 @@ TestWriter.prototype._write = function(buffer, encoding, callback) {
37 39
38 40 var dest = new TestWriter();
39 41
  42 +// Set this high so that we'd trigger a nextTick warning
  43 +// and/or RangeError if we do maybeReadMore wrong.
40 44 function TestReader() {
41   - stream.Readable.call(this);
  45 + stream.Readable.call(this, { highWaterMark: 0x10000 });
42 46 }
43 47 util.inherits(TestReader, stream.Readable);
44 48
45 49 TestReader.prototype._read = function(size) {
46   - this.push(new Buffer('hallo'));
  50 + this.push(chunk);
47 51 };
48 52
49 53 var src = new TestReader();
@@ -61,3 +65,10 @@ assert.equal(dest.listeners('drain').length, 0);
61 65 assert.equal(dest.listeners('error').length, 0);
62 66 assert.equal(dest.listeners('close').length, 0);
63 67 assert.equal(dest.listeners('finish').length, 0);
  68 +
  69 +console.error(src._readableState);
  70 +process.on('exit', function() {
  71 + assert(src._readableState.length >= src._readableState.highWaterMark);
  72 + src._readableState.buffer.length = 0;
  73 + console.error(src._readableState);
  74 +});

0 comments on commit cd2b9f5

Please sign in to comment.
Something went wrong with that request. Please try again.