From 77e0318849aa229b24bb5f30d3a919d8e133c226 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 20 Oct 2019 10:13:57 +0200 Subject: [PATCH] stream: increase MAX_HWM MAX_HWM was added in 9208c89 where the highwatermark was changed to always increase in steps of highest power of 2 to prevent increasing hwm excessivly in tiny amounts. Why a limit was added on the highwatermark is unclear but breaks existing usage where a larger read size is used. The invariant for read(n) is that a buffer of size n is always returned. Considering a maximum ceiling on the buffer size breaks this invariant. This PR significantly increases the limit to make it less likely to break the previous invariant and also documents the limit. Fixes: https://github.com/nodejs/node/issues/29933 PR-URL: https://github.com/nodejs/node/pull/29938 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- doc/api/stream.md | 2 ++ lib/_stream_readable.js | 5 +++-- test/parallel/test-readable-large-hwm.js | 27 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-readable-large-hwm.js diff --git a/doc/api/stream.md b/doc/api/stream.md index b6da7d98a1f94d..2d437c0eacef3b 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -1010,6 +1010,8 @@ buffer will be returned. If the `size` argument is not specified, all of the data contained in the internal buffer will be returned. +The `size` argument must be less than or equal to 1 GB. + The `readable.read()` method should only be called on `Readable` streams operating in paused mode. In flowing mode, `readable.read()` is called automatically until the internal buffer is fully drained. diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 52bc5019103ea2..98216b411f5862 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -340,10 +340,11 @@ Readable.prototype.setEncoding = function(enc) { return this; }; -// Don't raise the hwm > 8MB -const MAX_HWM = 0x800000; +// Don't raise the hwm > 1GB +const MAX_HWM = 0x40000000; function computeNewHighWaterMark(n) { if (n >= MAX_HWM) { + // TODO(ronag): Throw ERR_VALUE_OUT_OF_RANGE. n = MAX_HWM; } else { // Get the next highest power of 2 to prevent increasing hwm excessively in diff --git a/test/parallel/test-readable-large-hwm.js b/test/parallel/test-readable-large-hwm.js new file mode 100644 index 00000000000000..d5bf25bc0e61c1 --- /dev/null +++ b/test/parallel/test-readable-large-hwm.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +const { Readable } = require('stream'); + +// Make sure that readable completes +// even when reading larger buffer. +const bufferSize = 10 * 1024 * 1024; +let n = 0; +const r = new Readable({ + read() { + // Try to fill readable buffer piece by piece. + r.push(Buffer.alloc(bufferSize / 10)); + + if (n++ > 10) { + r.push(null); + } + } +}); + +r.on('readable', () => { + while (true) { + const ret = r.read(bufferSize); + if (ret === null) + break; + } +}); +r.on('end', common.mustCall());