-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
stream: avoid pause with unpipe in buffered write
If a pipe is cleaned up (due to unpipe) during a write that returned false, the source stream can get stuck in a paused state. Fixes: #2323 PR-URL: #2325 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
- Loading branch information
Showing
2 changed files
with
51 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,38 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const stream = require('stream'); | ||
|
||
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/2323 | ||
const buffer = new 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() { | ||
reader.unpipe(writer1); | ||
reader.pipe(writer2); | ||
reader.push(buffer); | ||
setImmediate(function() { | ||
reader.push(buffer); | ||
setImmediate(function() { | ||
reader.push(buffer); | ||
}); | ||
}); | ||
}); | ||
|
||
writer2._write = common.mustCall(function(chunk, encoding, cb) { | ||
cb(); | ||
}, 3); | ||
|
||
reader.pipe(writer1); | ||
reader.push(buffer); |
2 comments
on commit 6899094
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this going to be released on the binary versions ? Its been like 2 months and still the problem exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DasithKuruppu This commit has been in node releases since v5.0.0 (and not v4.x since it's a behavioral change).
@mscdex Hope you don’t mind my asking, but are you sure this path can actually be taken? Since 2ff499c,
pipesCount === 1
seems to imply thatstate.pipes
is not an array, sostate.pipes === dest
would have been correct here?