Repository navigation
fix(ext/web): fix promise chain leak of ReadableStream - #30871
Conversation
littledivy
left a comment
There was a problem hiding this comment.
Nice fix!
Can you add a test in tests/unit/streams_test.ts that tries to trigger a stack overflow? A depth of 30_000 should be enough to reproduce.
Deno.test(async function readableStreamEmittingManyChunks() {
const serverPort = 4594;
const stopSignal = new AbortController();
let count = 0;
function startServer() {
Deno.serve({ port: serverPort, signal: stopSignal.signal }, async (req) => {
return new Response(
new ReadableStream({
pull(controller) {
if (count < 30_000_000) {
controller.enqueue(new Uint8Array([0]));
} else {
controller.close();
}
count += 1;
},
}),
);
});
}
async function startClient() {
const response = await fetch(`http://localhost:${serverPort}`);
const reader = response.body?.getReader();
if (!reader) {
throw new Error("client: failed to get reader from response");
}
while (true) {
const { done } = await reader.read();
if (done) break;
}
}
startServer();
await startClient();
stopSignal.abort();
});I used Deno.serve because the issue occurred when consuming a ReadableStream through Deno.serve. I tried adjusting the depth using count, but in my environment, I had to specify a fairly large number to run out of memory. Results vary depending on the environment, and it takes a considerable amount of time, so it doesn't seem like a reliable test. Is there a better way? |
|
I used a method to measure maximum heap memory usage. Since it seemed unstable to compare the results when mixed with other test tasks, I ran the task in a separate Deno instance. Before the modification, the ratio (after/before) was between 7 and 8, while after the modification, it was between 1.3 and 1.4. Therefore, I set the cutoff value at 1.5. |
bartlomieju
left a comment
There was a problem hiding this comment.
LGTM, thanks, this is a very good fix
Closes #30298
This problem occurs because promise chain keep growing(like a stack overflow caused by a recursive function) while the stream is running. To solve this, used a loop.