Memory overflow in Readable.toWeb due to the default strategy

[lilsweetcaligula]

Version

v20.0.0-pre

Platform

Linux Mars 5.19.0-35-generic #36~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb 17 15:17:25 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

stream

What steps will reproduce the bug?

To reproduce, run the following script with the debugger enabled, i.e. node inspect mytest.mjs

// mytest.mjs

import fs from 'node:fs'
import process from 'node:process'
import { Readable } from 'node:stream'

const reportMemoryUsage = (maxMb, randomWebStream) => {
	const { arrayBuffers } = process.memoryUsage()

	if (arrayBuffers > maxMb * 1024 * 1024) {
		debugger
		process.exit(1)
	}
}

async function main() {
	const randomNodeStream = fs.createReadStream('/dev/urandom', { highWaterMark: 5556 })
	const randomWebStream = Readable.toWeb(randomNodeStream)

	setInterval(() => reportMemoryUsage(16, randomWebStream), 1e3)
}

main()

• When the debugger stops at the breakpoint, open up the repl, and print out the contents of randomWebStream, i.e. console.dir(randomWebStream, { depth: 32 }).
• Inspect the output of the queue in randomWebStream[Symbol(kState)].controller[Symbol(kState)].queue.

How often does it reproduce? Is there a required condition?

Always. However, the underlying stream that's being turned into the web stream must not be in the object mode.

What is the expected behavior? Why is that the expected behavior?

Because the underlying stream was not in the object mode and had its highWaterMark set at 5556 bytes, the expected behavior is for the web stream's underlying queue to also have the total size circa 5556 bytes.

What do you see instead?

In my case I see the queue has the length of 5556 elements, with each element being an instance of Uint8Array of size 5556 bytes by itself - thus producing a grand total of 30869136 bytes, - as opposed to the 5556 bytes which the highWaterMark of the underlying stream randomNodeStream had.

Additional information

This may be related to #46347.

The source of this behavior is likely this line:

node/lib/internal/webstreams/adapters.js

Lines 420 to 424 in fe514bf

	// When not running in objectMode explicitly, we just fall
	// back to a minimal strategy that just specifies the highWaterMark
	// and no size algorithm. Using a ByteLengthQueuingStrategy here
	// is unnecessary.
	return { highWaterMark };

This should probably use the ByteLengthQueuingStrategy instead, as previously suggested by @debadree25 #46347 (comment)

[lilsweetcaligula]

My output:

< ReadableStream {
<   [Symbol(kType)]: 'ReadableStream',
<   [Symbol(kState)]: <ref *1> {
<     disturbed: false,
<     reader: undefined,
<     state: 'readable',
<     storedError: undefined,
<     stream: undefined,
<     transfer: {
<       writable: undefined,
<       port1: undefined,
<       port2: undefined,
<       promise: undefined
<     },
<     controller: ReadableStreamDefaultController {
<       [Symbol(kType)]: 'ReadableStreamDefaultController',
<       [Symbol(kState)]: {
<         cancelAlgorithm: [Function: bound cancel],
<         closeRequested: false,
<         highWaterMark: 5556,
<         pullAgain: false,
<         pullAlgorithm: [Function: bound pull],
<         pulling: false,
<         queue: [
<           {
<             value: Uint8Array(5556) [
<                68,  58,  34, 241,  86,  28, 225,  57, 134, 151,  97, 210,
<               202,  53, 112,  41, 211, 174, 206,  99, 122, 169,   2, 151,
<                60, 189, 167, 183, 100, 204, 138,  91, 114,  68,  40,  54,
<               208,  26, 200, 105,  34, 101, 236, 249, 197, 131,  59,  96,
<               221, 162, 243, 119,   4,  55, 228, 212, 232, 226, 153, 153,
<                73, 137, 103, 109,  43, 165,  61, 168,  89, 251, 153,   1,
<                59, 174, 150,  60,  42,  10,  13, 114, 169, 222, 144,  46,
<               176, 177,  48, 146,  87, 176,  38, 135, 149, 175, 152,  55,
<               230,  85, 157, 128,
<               ... 5456 more items
<             ],
<             size: 1
<           },

[TRUNCATED]

<           {
<             value: Uint8Array(5556) [
<               105,  82,  92, 129, 142,  78,  16, 182,  94, 173, 208,  48,
<               250, 180,  81,  99, 240,  92, 187,   1,  26, 138,  41, 175,
<                60,  95,  91, 236,  70, 189, 130, 146,  48, 103, 159,  40,
<                18, 251, 183, 204,  96,  98, 225,  75,   2,  74,  81, 126,
<               200, 137, 151, 171,  81, 197, 161, 118, 181, 189,  23,  86,
<               154,  55,  38,  80,  35, 131, 163,  43,  80, 200, 211, 205,
<                70,  25,  67, 212, 196, 203,  19, 248, 232,  44,  57, 143,
<               203,  21, 165, 242, 100, 200,   8, 187, 252, 221, 229,  20,
<               232, 101,  44, 109,
<               ... 5456 more items
<             ],
<             size: 1
<           },
<           {
<             value: Uint8Array(5556) [
<               189,   7, 147, 159, 128,  93,  34, 247, 167,  28, 202,  89,
<               197,  76, 189,  37, 189, 244, 242, 166, 232,  16,  82, 102,
<                69, 247, 211, 149, 149, 210,  88, 110,  12, 103,  69, 184,
<                88, 162, 181, 123, 167, 244, 170, 165,  89, 197,  68,  72,
<                70,  48, 194, 207,  11, 255, 126,  67,  32, 148, 181,  14,
<               182,  78, 187, 110, 155, 232,  10,  76,  12, 132,  45, 168,
<               119, 193,  73, 229, 203, 253,  50,  90, 195, 128, 149, 240,
<                51, 179, 243, 139, 235, 141,  65,   5,  77, 120, 178,  43,
<                86,  98, 187, 212,
<               ... 5456 more items
<             ],
<             size: 1
<           }
<         ],
<         queueTotalSize: 5556,
<         started: true,
<         sizeAlgorithm: [Function (anonymous)],
<         stream: ReadableStream {
<           [Symbol(kType)]: 'ReadableStream',
<           [Symbol(kState)]: [Circular *1],
<           [Symbol(nodejs.webstream.isClosedPromise)]: [Object]
<         }
<       }
<     }
<   },
<   [Symbol(nodejs.webstream.isClosedPromise)]: {
<     promise: Promise { <pending> },
<     resolve: [Function (anonymous)],
<     reject: [Function (anonymous)]
<   }
< }
<

[lilsweetcaligula]

Suggested fix: #47129