From 269103a0e5e30cc217bde1660087e87dfc722b8a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 13 Mar 2019 22:26:18 +0100 Subject: stream: fix regression introduced in #26059 In #26059, we introduced a bug that caused 'readable' to be nextTicked on EOF of a ReadableStream. This breaks the dicer module on CITGM. That change was partially reverted to still fix the bug in #25810 and not break dicer. See: https://github.com/nodejs/node/pull/26059 Fixes: https://github.com/nodejs/node/issues/25810 PR-URL: https://github.com/nodejs/node/pull/26643 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/_stream_readable.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'lib/_stream_readable.js') diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 968c7b258c..fe77c44186 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -512,12 +512,24 @@ function onEofChunk(stream, state) { } } state.ended = true; - state.needReadable = false; - // We are not protecting if emittedReadable = true, - // so 'readable' gets scheduled anyway. - state.emittedReadable = true; - process.nextTick(emitReadable_, stream); + if (state.sync) { + // If we are sync, 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; + state.emittedReadable = true; + // We have to emit readable now that we are EOF. Modules + // in the ecosystem (e.g. dicer) rely on this event being sync. + if (state.ended) { + emitReadable_(stream); + } else { + process.nextTick(emitReadable_, stream); + } + } } // Don't emit readable right away in sync mode, because this can trigger -- cgit v1.2.3