-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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: #5820 Fixes: #5257 PR-URL: #6023 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
- Loading branch information
1 parent
e6e46ae
commit 334cf5c
Showing
2 changed files
with
43 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
const stream = require('stream'); | ||
|
||
// This is very similar to test-stream-pipe-cleanup-pause.js. | ||
|
||
const reader = new stream.Readable(); | ||
const writer1 = new stream.Writable(); | ||
const writer2 = new stream.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 | ||
const buffer = Buffer(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); |