Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit b0225e5

Browse filesBrowse files
davedoesdevevanlucas
authored andcommitted
stream: ensure awaitDrain is increased once
Guard against the call to write() inside pipe's ondata pushing more data back onto the Readable, thus causing ondata to be called again. This is fine but results in awaitDrain being increased more than once. The problem with that is when the destination does drain, only a single 'drain' event is emitted, so awaitDrain in this case will never reach zero and we end up with a permanently paused stream. Fixes: #7278 PR-URL: #7292 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent ad1045c commit b0225e5
Copy full SHA for b0225e5

File tree

Expand file treeCollapse file tree

2 files changed

+36
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+36
-1
lines changed
Open diff view settings
Collapse file

‎lib/_stream_readable.js‎

Copy file name to clipboardExpand all lines: lib/_stream_readable.js
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,11 +549,17 @@ Readable.prototype.pipe = function(dest, pipeOpts) {
549549
ondrain();
550550
}
551551

552+
// If the user pushes more data while we're writing to dest then we'll end up
553+
// in ondata again. However, we only want to increase awaitDrain once because
554+
// dest will only emit one 'drain' event for the multiple writes.
555+
// => Introduce a guard on increasing awaitDrain.
556+
var increasedAwaitDrain = false;
552557
src.on('data', ondata);
553558
function ondata(chunk) {
554559
debug('ondata');
560+
increasedAwaitDrain = false;
555561
var ret = dest.write(chunk);
556-
if (false === ret) {
562+
if (false === ret && !increasedAwaitDrain) {
557563
// If the user unpiped during `dest.write()`, it is possible
558564
// to get stuck in a permanently paused state if that write
559565
// also returned false.
@@ -563,6 +569,7 @@ Readable.prototype.pipe = function(dest, pipeOpts) {
563569
!cleanedUp) {
564570
debug('false write response, pause', src._readableState.awaitDrain);
565571
src._readableState.awaitDrain++;
572+
increasedAwaitDrain = true;
566573
}
567574
src.pause();
568575
}
Collapse file
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
const common = require('../common');
3+
const stream = require('stream');
4+
5+
// A writable stream which pushes data onto the stream which pipes into it,
6+
// but only the first time it's written to. Since it's not paused at this time,
7+
// a second write will occur. If the pipe increases awaitDrain twice, we'll
8+
// never get subsequent chunks because 'drain' is only emitted once.
9+
const writable = new stream.Writable({
10+
write: common.mustCall((chunk, encoding, cb) => {
11+
if (chunk.length === 32 * 1024) { // first chunk
12+
readable.push(new Buffer(33 * 1024)); // above hwm
13+
}
14+
cb();
15+
}, 3)
16+
});
17+
18+
// A readable stream which produces two buffers.
19+
const bufs = [new Buffer(32 * 1024), new Buffer(33 * 1024)]; // above hwm
20+
const readable = new stream.Readable({
21+
read: function() {
22+
while (bufs.length > 0) {
23+
this.push(bufs.shift());
24+
}
25+
}
26+
});
27+
28+
readable.pipe(writable);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.