stream: Fix readableState.awaitDrain mechanism

In 6899094 (#2325), the conditions for increasing `readableState.awaitDrain` when writing to a piping destination returns false were changed so that they could not actually be met, effectively leaving `readableState.awaitDrain` with a constant value of 0. This patch changes the conditions to testing whether the stream for which `.write()` returned false is still a piping destination, which was likely the intention of the original patch. Fixes: #5820Fixes: #5257 PR-URL: #6023 Reviewed-By: Brian White <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: addaleax

lib/_stream_readable.js

@@ -538,9 +538,9 @@ Readable.prototype.pipe = function(dest, pipeOpts){
 // If the user unpiped during `dest.write()`, it is possible
 // to get stuck in a permanently paused state if that write
 // also returned false.
-if(state.pipesCount===1&&
-state.pipes[0]===dest&&
-src.listenerCount('data')===1&&
+// => Check whether `dest` is still a piping destination.
+if(((state.pipesCount===1&&state.pipes===dest)||
+(state.pipesCount>1&&state.pipes.indexOf(dest)!==-1))&&
 !cleanedUp){
 debug('false write response, pause',src._readableState.awaitDrain);
 src._readableState.awaitDrain++;

test/parallel/test-stream-pipe-await-drain.js

@@ -0,0 +1,40 @@
+'use strict';
+constcommon=require('../common');
+conststream=require('stream');
+
+// This is very similar to test-stream-pipe-cleanup-pause.js.
+
+constreader=newstream.Readable();
+constwriter1=newstream.Writable();
+constwriter2=newstream.Writable();
+
+// 560000 is chosen here because it is larger than the (default) highWaterMark
+// and will cause `.write()` to return false
+// See: https://github.com/nodejs/node/issues/5820
+constbuffer=Buffer.allocUnsafe(560000);
+
+reader._read=function(n){};
+
+writer1._write=common.mustCall(function(chunk,encoding,cb){
+this.emit('chunk-received');
+cb();
+},1);
+writer1.once('chunk-received',function(){
+setImmediate(function(){
+// This one should *not* get through to writer1 because writer2 is not
+// "done" processing.
+reader.push(buffer);
+});
+});
+
+// A "slow" consumer:
+writer2._write=common.mustCall(function(chunk,encoding,cb){
+// Not calling cb here to "simulate" slow stream.
+
+// This should be called exactly once, since the first .write() call
+// will return false.
+},1);
+
+reader.pipe(writer1);
+reader.pipe(writer2);
+reader.push(buffer);