From 3c8350949edf17b88a2ff145ecb7ef4ec028a114 Mon Sep 17 00:00:00 2001 From: jlvivero Date: Thu, 28 Sep 2017 11:21:58 -0700 Subject: [PATCH 1/2] stream: fix disparity between buffer and the count This changes the disparity of bufferedRequestCount and the actual buffer on file _stream_writable.js Fixes: https://github.com/nodejs/node/issues/6758 --- lib/_stream_writable.js | 3 +- .../test-stream-writable-clear-buffer.js | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 test/sequential/test-stream-writable-clear-buffer.js diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 7de77958d56b4c..d87072a967bf2c 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -502,6 +502,7 @@ function clearBuffer(stream, state) { corkReq.finish = onCorkedFinish.bind(undefined, corkReq, state); state.corkedRequestsFree = corkReq; } + state.bufferedRequestCount = 0; } else { // Slow case, write chunks one-by-one while (entry) { @@ -512,6 +513,7 @@ function clearBuffer(stream, state) { doWrite(stream, state, false, len, chunk, encoding, cb); entry = entry.next; + state.bufferedRequestCount--; // if we didn't call the onwrite immediately, then // it means that we need to wait until it does. // also, that means that the chunk and cb are currently @@ -525,7 +527,6 @@ function clearBuffer(stream, state) { state.lastBufferedRequest = null; } - state.bufferedRequestCount = 0; state.bufferedRequest = entry; state.bufferProcessing = false; } diff --git a/test/sequential/test-stream-writable-clear-buffer.js b/test/sequential/test-stream-writable-clear-buffer.js new file mode 100644 index 00000000000000..de989826b34212 --- /dev/null +++ b/test/sequential/test-stream-writable-clear-buffer.js @@ -0,0 +1,34 @@ +'use strict'; +const common = require('../common'); +const Stream = require('stream'); +//this test ensures that the _writeableState.bufferedRequestCount and +// the actual buffered request count are the same +const assert = require('assert'); + +class StreamWritable extends Stream.Writable { + constructor() { + super({ objectMode: true }); + } + + //We need a timeout like on the original issue thread + //otherwise the code will never reach our test case + //this means this should go on the sequential folder. + _write(chunk, encoding, cb) { + setTimeout(cb, common.platformTimeout(10)); + } +} + +const testStream = new StreamWritable(); +testStream.cork(); + +for (let i = 1; i <= 5; i++) { + testStream.write(i, function() { + assert.strictEqual( + testStream._writableState.bufferedRequestCount, + testStream._writableState.getBuffer().length, + 'bufferedRequestCount variable is different from the actual length of' + + ' the buffer'); + }); +} + +testStream.end(); From 728405da2886066c5961f0757ae4d5b26315e60b Mon Sep 17 00:00:00 2001 From: jlvivero Date: Thu, 28 Sep 2017 15:02:44 -0700 Subject: [PATCH 2/2] change comment style --- test/sequential/test-stream-writable-clear-buffer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/sequential/test-stream-writable-clear-buffer.js b/test/sequential/test-stream-writable-clear-buffer.js index de989826b34212..dc859e3fb6b362 100644 --- a/test/sequential/test-stream-writable-clear-buffer.js +++ b/test/sequential/test-stream-writable-clear-buffer.js @@ -1,7 +1,7 @@ 'use strict'; const common = require('../common'); const Stream = require('stream'); -//this test ensures that the _writeableState.bufferedRequestCount and +// This test ensures that the _writeableState.bufferedRequestCount and // the actual buffered request count are the same const assert = require('assert'); @@ -10,9 +10,9 @@ class StreamWritable extends Stream.Writable { super({ objectMode: true }); } - //We need a timeout like on the original issue thread - //otherwise the code will never reach our test case - //this means this should go on the sequential folder. + // We need a timeout like on the original issue thread + // otherwise the code will never reach our test case + // this means this should go on the sequential folder. _write(chunk, encoding, cb) { setTimeout(cb, common.platformTimeout(10)); }