[v24.x backport] buffer: fix Blob.stream() leaking source buffer#64664
Open
gabrielbryk wants to merge 2 commits into
Open
[v24.x backport] buffer: fix Blob.stream() leaking source buffer#64664gabrielbryk wants to merge 2 commits into
gabrielbryk wants to merge 2 commits into
Conversation
nodejs-github-bot
added
c++
Blob.prototype.stream() registered a wakeup callback on the underlying source's start() and never released it. The strong Reader::wakeup_ handle kept the reader -- and through it the blob's DataQueue and backing store -- reachable as a GC root, so the source buffer leaked on every stream() call. On Node 26+, streaming a 1 MiB blob 300 times retained ~300 MiB in process.memoryUsage().arrayBuffers while the V8 heap stayed small. Register the wakeup lazily in pull() and clear it on every terminal or idle path (EOS, error, cancel, backpressure), mirroring the cleanup already done by the async iterator path. The strong handle now only lives while a pull is in flight, so the reader and its backing store become collectable once the stream finishes, errors, is cancelled, or goes idle under backpressure. Fixes: nodejs#63574 Signed-off-by: semimikoh <ejffjeosms@gmail.com> PR-URL: nodejs#63577 Fixes: nodejs#63574 Reviewed-By: James M Snell <jasnell@gmail.com> (cherry picked from commit 243905a)
Backport prerequisite for the previous commit (backport of nodejs#63577). The Blob.stream() leak fix clears the reader's wakeup handle by calling reader.setWakeup(undefined) on every terminal/idle path. On main the C++ Blob::Reader::SetWakeup already tolerates an undefined argument (resetting the strong wakeup_ handle), but that guard is not present on v24.x -- it landed incidentally in the large QUIC implementation commit cf91d18 (nodejs#62876), which is not itself being backported. Extract only the minimal node_blob.cc SetWakeup hunk from that commit so the JS fix works on v24.x. Without it, setWakeup(undefined) aborts with `Assertion failed: args[0]->IsFunction()` at src/node_blob.cc. The NotifyPull(fin)/pull_pending_ coalescing from the same upstream commit is QUIC-specific, not required by the leak fix, and is intentionally omitted. Refs: nodejs#63577 Refs: nodejs#62876 Signed-off-by: Gabe Bryk <gbryk11@gmail.com>
gabrielbryk
force-pushed
the
backport-63577-to-v24.x
branch
from
July 22, 2026 18:25
34a4610 to
2aa101b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #63577 to v24.x. It doesn't cherry-pick cleanly on its own: the
JS fix clears the reader's wakeup handle with
reader.setWakeup(undefined),but on v24.x the C++
Blob::Reader::SetWakeupstill asserts a functionargument. The guard that makes it accept
undefinedonly landed as part ofthe QUIC commit cf91d18 (#62876), which isn't on this line, so a plain
cherry-pick builds but aborts the first time a stream reaches a terminal/idle
path:
The second commit extracts only that four-line
node_blob.ccguard from#62876; the rest of that commit (the
NotifyPull(fin)/pull_pending_coalescing) is QUIC-specific and not needed here. It could instead be done as
a prerequisite backport of #62876, or folded into the cherry-pick, if a
reviewer prefers.
parallel/test-blob*, including the newtest-blob-stream-gc.jsfrom #63577,passes on a local build; the #63574 reproducer holds
arrayBuffersflat where24.16.0 leaks the source buffer on every stream.