stream: correctly pause and resume after once('readable')

Fixes: nodejs#24281 PR-URL: nodejs#24366 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
refack · Jan 10, 2019 · 96c5c7e · 96c5c7e
1 parent 7cce7fa
commit 96c5c7e
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js
@@ -114,6 +114,7 @@ function ReadableState(options, stream, isDuplex) {
   this.emittedReadable = false;
   this.readableListening = false;
   this.resumeScheduled = false;
+  this.paused = true;
 
   // Should close be emitted on destroy. Defaults to true.
   this.emitClose = options.emitClose !== false;
@@ -862,10 +863,16 @@ Readable.prototype.removeAllListeners = function(ev) {
 };
 
 function updateReadableListening(self) {
-  self._readableState.readableListening = self.listenerCount('readable') > 0;
+  const state = self._readableState;
+  state.readableListening = self.listenerCount('readable') > 0;
 
-  // crude way to check if we should resume
-  if (self.listenerCount('data') > 0) {
+  if (state.resumeScheduled && !state.paused) {
+    // flowing needs to be set to true now, otherwise
+    // the upcoming resume will not flow.
+    state.flowing = true;
+
+    // crude way to check if we should resume
+  } else if (self.listenerCount('data') > 0) {
     self.resume();
   }
 }
@@ -887,6 +894,7 @@ Readable.prototype.resume = function() {
     state.flowing = !state.readableListening;
     resume(this, state);
   }
+  state.paused = false;
   return this;
 };
 
@@ -917,6 +925,7 @@ Readable.prototype.pause = function() {
     this._readableState.flowing = false;
     this.emit('pause');
   }
+  this._readableState.paused = true;
   return this;
 };
 

diff --git a/test/parallel/test-stream-readable-readable-then-resume.js b/test/parallel/test-stream-readable-readable-then-resume.js
@@ -0,0 +1,29 @@
+'use strict';
+
+const common = require('../common');
+const { Readable } = require('stream');
+
+// This test verifies that a stream could be resumed after
+// removing the readable event in the same tick
+
+check(new Readable({
+  objectMode: true,
+  highWaterMark: 1,
+  read() {
+    if (!this.first) {
+      this.push('hello');
+      this.first = true;
+      return;
+    }
+
+    this.push(null);
+  }
+}));
+
+function check(s) {
+  const readableListener = common.mustNotCall();
+  s.on('readable', readableListener);
+  s.on('end', common.mustCall());
+  s.removeListener('readable', readableListener);
+  s.resume();
+}