Skip to content

Commit ea47189

Browse files
addaleaxBethGriggs
authored andcommitted
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: #26957 PR-URL: #26965 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 3c92926 commit ea47189

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

lib/_stream_readable.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ Readable.prototype.read = function(n) {
475475
ret = null;
476476

477477
if (ret === null) {
478-
state.needReadable = true;
478+
state.needReadable = state.length <= state.highWaterMark;
479479
n = 0;
480480
} else {
481481
state.length -= n;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { Readable } = require('stream');
4+
5+
// readable.resume() should not lead to a ._read() call being scheduled
6+
// when we exceed the high water mark already.
7+
8+
const readable = new Readable({
9+
read: common.mustNotCall(),
10+
highWaterMark: 100
11+
});
12+
13+
// Fill up the internal buffer so that we definitely exceed the HWM:
14+
for (let i = 0; i < 10; i++)
15+
readable.push('a'.repeat(200));
16+
17+
// Call resume, and pause after one chunk.
18+
// The .pause() is just so that we don’t empty the buffer fully, which would
19+
// be a valid reason to call ._read().
20+
readable.resume();
21+
readable.once('data', common.mustCall(() => readable.pause()));

0 commit comments

Comments
 (0)