Blob regression in v19.8.x

[KhafraDev]

Version

v19.8.0 and v19.8.1

Platform

Linux DESKTOP-L4O1H93 4.4.0-19041-Microsoft #2311-Microsoft Tue Nov 08 17:09:00 PST 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

buffer

What steps will reproduce the bug?

import { randomBytes } from 'crypto'

const random = randomBytes(256)
const chunks = []

for (let i = 0; i < random.length; i += 2) {
  chunks.push(random.subarray(i, i + 2))
}

await new Blob(chunks).arrayBuffer()

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

always

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

The test should pass

What do you see instead?

node:internal/blob:277
      reader.pull((status, buffer) => {
             ^

RangeError: Maximum call stack size exceeded
    at readNext (node:internal/blob:277:14)
    at BlobReader.<anonymous> (node:internal/blob:293:9)
    at readNext (node:internal/blob:277:14)
    at BlobReader.<anonymous> (node:internal/blob:293:9)
    at readNext (node:internal/blob:277:14)
    at BlobReader.<anonymous> (node:internal/blob:293:9)
    at readNext (node:internal/blob:277:14)
    at BlobReader.<anonymous> (node:internal/blob:293:9)
    at readNext (node:internal/blob:277:14)
    at BlobReader.<anonymous> (node:internal/blob:293:9)

Node.js v19.8.1

Additional information

this was discovered in a test in undici that works in v19.7 and below, but fails in v19.8
https://github.com/nodejs/undici/blob/a9ef50944e917695b1789ad62fc5e1bb9451d483/test/fetch/client-fetch.js#L202

[ronag]

@jasnell

[ronag]

Is reader.pull calling the callback without process.nextTick?

[KhafraDev]

Yeah, adding a queueMicrotask fixes it, but it seemed like an issue that should be fixed in c++ to me. edit: I spent a while learning how the new Blob/data queue system worked and now I think this is a good fix lol

diff --git a/lib/internal/blob.js b/lib/internal/blob.js
index 4188d999f7..9c6be6981f 100644
--- a/lib/internal/blob.js
+++ b/lib/internal/blob.js
@@ -69,6 +69,8 @@ const {
   CountQueuingStrategy,
 } = require('internal/webstreams/queuingstrategies');

+const { queueMicrotask } = require('internal/process/task_queues');
+
 const kHandle = Symbol('kHandle');
 const kType = Symbol('kType');
 const kLength = Symbol('kLength');
@@ -284,7 +286,7 @@ class Blob {
         }
         if (buffer !== undefined)
           buffers.push(buffer);
-        readNext();
+        queueMicrotask(() => readNext());
       });
     };
     readNext();

[rluvaton]

so I kinda did git bisect and I'm pretty sure this regression was created in one of those:

• 36f36b9
• 9b6270a

[KhafraDev]

Yes it was