Skip to content

Commit c07abd7

Browse files
committed
stream: postpone setting flowing for on('readable')
Now state.flowing will be set only after all of the 'readable' listeners are gone and if we have at least one 'data' listener. * on('data') will not flow (flowing === null and not false) if there are 'readable' listeners * pipe() will work regardless of 'readable' listeners * isPause reports only user .pause call (before setting 'data' listener when there is already 'readable' listener also set flowing to false) * resume always sets stream to flowing state
1 parent 6fa9528 commit c07abd7

5 files changed

+111
-13
lines changed

lib/_stream_readable.js

+8-9
Original file line numberDiff line numberDiff line change
@@ -801,8 +801,8 @@ Readable.prototype.on = function(ev, fn) {
801801
// a few lines down. This is needed to support once('readable').
802802
state.readableListening = this.listenerCount('readable') > 0;
803803

804-
// Try start flowing on next tick if stream isn't explicitly paused
805-
if (state.flowing !== false)
804+
// Try start flowing on next tick for data if stream isn't explicitly paused
805+
if (!state.readableListening && state.flowing !== false)
806806
this.resume();
807807
} else if (ev === 'readable') {
808808
if (!state.endEmitted && !state.readableListening) {
@@ -855,13 +855,14 @@ Readable.prototype.removeAllListeners = function(ev) {
855855
function updateReadableListening(self) {
856856
const state = self._readableState;
857857
state.readableListening = self.listenerCount('readable') > 0;
858+
858859
// try to start flowing for 'data' listeners
859860
// (if pipesCount is not 0 then we have already 'flowed')
860-
if (!state.readableListening &&
861-
self.listenerCount('data') > 0 &&
862-
state.pipesCount === 0) {
861+
if (self.listenerCount('data') > 0 &&
862+
!self.isPaused() &&
863+
state.pipesCount === 0 &&
864+
!state.readableListening)
863865
self.resume();
864-
}
865866
}
866867

867868
function nReadingNextTick(self) {
@@ -875,9 +876,7 @@ Readable.prototype.resume = function() {
875876
var state = this._readableState;
876877
if (!state.flowing) {
877878
debug('resume');
878-
// we flow only if there is no one listening
879-
// for readable
880-
state.flowing = !state.readableListening;
879+
state.flowing = true;
881880
resume(this, state);
882881
}
883882
return this;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
// This test ensures that if we have both 'readable' and 'data'
6+
// listeners on Readable instance once all of the 'readable' listeners
7+
// are gone and there are still 'data' listeners stream will *not*
8+
// try to flow if it was explicitly paused.
9+
10+
const { Readable } = require('stream');
11+
12+
const r = new Readable({
13+
read: () => {},
14+
});
15+
16+
const data = ['foo', 'bar', 'baz'];
17+
18+
r.pause();
19+
20+
r.on('data', common.mustNotCall());
21+
r.on('end', common.mustNotCall());
22+
r.once('readable', common.mustCall());
23+
24+
for (const d of data)
25+
r.push(d);
26+
r.push(null);

test/parallel/test-stream-readable-and-data.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
const common = require('../common');
44

55
// This test ensures that if we have both 'readable' and 'data'
6-
// listeners on Readable instance once the 'readable' listeners
6+
// listeners on Readable instance once all of the 'readable' listeners
77
// are gone and there are still 'data' listeners stream will try
88
// to flow to satisfy the 'data' listeners.
99

@@ -23,7 +23,6 @@ r.once('end', common.mustCall(() => {
2323
assert.strictEqual(receivedData, data.join(''));
2424
}));
2525

26-
r.push(data[0]);
27-
r.push(data[1]);
28-
r.push(data[2]);
26+
for (const d of data)
27+
r.push(d);
2928
r.push(null);
+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
// This test ensures that if have 'readable' listener
6+
// on Readable instance it will not disrupt the pipe.
7+
8+
const assert = require('assert');
9+
const { Readable, Writable } = require('stream');
10+
11+
let receivedData = '';
12+
const w = new Writable({
13+
write: (chunk, env, callback) => {
14+
receivedData += chunk;
15+
callback();
16+
},
17+
});
18+
19+
const data = ['foo', 'bar', 'baz'];
20+
const r = new Readable({
21+
read: () => {},
22+
});
23+
24+
r.on('readable', common.mustCall());
25+
26+
r.pipe(w);
27+
r.push(data[0]);
28+
r.push(data[1]);
29+
r.push(data[2]);
30+
r.push(null);
31+
32+
w.on('finish', common.mustCall(() => {
33+
assert.strictEqual(receivedData, data.join(''));
34+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
// This test ensures that if we remove last 'readable' listener
6+
// on Readable instance that is piped it will not disrupt the pipe.
7+
8+
const assert = require('assert');
9+
const { Readable, Writable } = require('stream');
10+
11+
let receivedData = '';
12+
const w = new Writable({
13+
write: (chunk, env, callback) => {
14+
receivedData += chunk;
15+
callback();
16+
},
17+
});
18+
19+
const data = ['foo', 'bar', 'baz'];
20+
const r = new Readable({
21+
read: () => {},
22+
});
23+
24+
const listener = common.mustNotCall();
25+
r.on('readable', listener);
26+
27+
r.pipe(w);
28+
r.push(data[0]);
29+
30+
r.removeListener('readable', listener);
31+
32+
process.nextTick(() => {
33+
r.push(data[1]);
34+
r.push(data[2]);
35+
r.push(null);
36+
});
37+
38+
w.on('finish', common.mustCall(() => {
39+
assert.strictEqual(receivedData, data.join(''));
40+
}));

0 commit comments

Comments
 (0)