Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: defer readable when sync #18515

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,11 +486,18 @@ function onEofChunk(stream, state) {
}
state.ended = true;

// emit 'readable' now to make sure it gets picked up.
state.needReadable = false;
if (!state.emittedReadable) {
state.emittedReadable = true;
emitReadable_(stream);
if (state.sync && state.length) {
// if we are sync and have data in the buffer, wait until next tick
// to emit the data. otherwise we risk emitting data in the flow()
// the readable code triggers during a read() call
emitReadable(stream);
} else {
// emit 'readable' now to make sure it gets picked up.
state.needReadable = false;
if (!state.emittedReadable) {
state.emittedReadable = true;
emitReadable_(stream);
}
}
}

Expand Down
67 changes: 67 additions & 0 deletions test/parallel/test-stream-pipe-flow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
'use strict';
const common = require('../common');
const { Readable, Writable, PassThrough } = require('stream');

{
let ticks = 17;

const rs = new Readable({
objectMode: true,
read: () => {
if (ticks-- > 0)
return process.nextTick(() => rs.push({}));
rs.push({});
rs.push(null);
}
});

const ws = new Writable({
highWaterMark: 0,
objectMode: true,
write: (data, end, cb) => setImmediate(cb)
});

rs.on('end', common.mustCall());
ws.on('finish', common.mustCall());
rs.pipe(ws);
}

{
let missing = 8;

const rs = new Readable({
objectMode: true,
read: () => {
if (missing--) rs.push({});
else rs.push(null);
}
});

const pt = rs
.pipe(new PassThrough({ objectMode: true, highWaterMark: 2 }))
.pipe(new PassThrough({ objectMode: true, highWaterMark: 2 }));

pt.on('end', function() {
wrapper.push(null);
});

const wrapper = new Readable({
objectMode: true,
read: () => {
process.nextTick(function() {
let data = pt.read();
if (data === null) {
pt.once('readable', function() {
data = pt.read();
if (data !== null) wrapper.push(data);
});
} else {
wrapper.push(data);
}
});
}
});

wrapper.resume();
wrapper.on('end', common.mustCall());
}
40 changes: 40 additions & 0 deletions test/parallel/test-stream-readable-pause-and-resume.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';

const { Readable } = require('stream');
const common = require('../common');

let ticks = 18;
let expectedData = 19;

const rs = new Readable({
objectMode: true,
read: () => {
if (ticks-- > 0)
return process.nextTick(() => rs.push({}));
rs.push({});
rs.push(null);
}
});

rs.on('end', common.mustCall());
readAndPause();

function readAndPause() {
// Does a on(data) -> pause -> wait -> resume -> on(data) ... loop.
// Expects on(data) to never fire if the stream is paused.
const ondata = common.mustCall((data) => {
rs.pause();

expectedData--;
if (expectedData <= 0)
return;

setImmediate(function() {
rs.removeListener('data', ondata);
readAndPause();
rs.resume();
});
}, 1); // only call ondata once

rs.on('data', ondata);
}