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 <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 17 insertions, 5 deletions

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