From 5c72c6d2028866409b1dd17ba70825d8eeb07c87 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 28 Mar 2019 20:46:39 +0100
Subject: [PATCH] stream: do not unconditionally call `_read()` on `resume()`

`readable.resume()` calls `.read(0)`, which in turn previously set
`needReadable = true`, and so a subsequent `.read()` call would
call `_read()` even though enough data was already available.

This can lead to elevated memory usage, because calling `_read()`
when enough data is in the readable buffer means that backpressure
is not being honoured.

Fixes: https://github.com/nodejs/node/issues/26957
---
 lib/_stream_readable.js                       |  2 +-
 .../test-stream-readable-resume-hwm.js        | 21 +++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-stream-readable-resume-hwm.js

diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js
index 81e8de310c240f..6198da5ca6be76 100644
--- a/lib/_stream_readable.js
+++ b/lib/_stream_readable.js
@@ -475,7 +475,7 @@ Readable.prototype.read = function(n) {
     ret = null;
 
   if (ret === null) {
-    state.needReadable = true;
+    state.needReadable = state.length <= state.highWaterMark;
     n = 0;
   } else {
     state.length -= n;
diff --git a/test/parallel/test-stream-readable-resume-hwm.js b/test/parallel/test-stream-readable-resume-hwm.js
new file mode 100644
index 00000000000000..3f0bbad243b0a2
--- /dev/null
+++ b/test/parallel/test-stream-readable-resume-hwm.js
@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const { Readable } = require('stream');
+
+// readable.resume() should not lead to a ._read() call being scheduled
+// when we exceed the high water mark already.
+
+const readable = new Readable({
+  read: common.mustNotCall(),
+  highWaterMark: 100
+});
+
+// Fill up the internal buffer so that we definitely exceed the HWM:
+for (let i = 0; i < 10; i++)
+  readable.push('a'.repeat(200));
+
+// Call resume, and pause after one chunk.
+// The .pause() is just so that we don’t empty the buffer fully, which would
+// be a valid reason to call ._read().
+readable.resume();
+readable.once('data', common.mustCall(() => readable.pause()));