Clone fetch response buffer to main thread heap for proper GC reclamation#28743
Clone fetch response buffer to main thread heap for proper GC reclamation#28743
Conversation
…tion The scheduled_response_buffer in FetchTasklet is written to on the HTTP thread, so its backing memory is allocated on the HTTP thread's mimalloc heap. When this memory is later freed from the main JS thread during GC finalization (Blob.finalize → Store.deref → Bytes.deinit), mimalloc puts the freed block on the HTTP thread's delayed-free list. Since the HTTP thread is idle after fetches complete, it never processes that list, so the pages are never returned to the OS. Fix: when transferring the response buffer to the body value on the main thread (in onBodyReceived and toBodyValue), clone the data into a fresh allocation on the main thread's mimalloc heap. The old HTTP-thread buffer is freed immediately (cross-thread free, goes to delayed list, but is small overhead). The body data now lives on the main thread's heap, so GC finalization properly reclaims it. Closes #28741
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
Updated 5:34 AM PT - Apr 1st, 2026
❌ @autofix-ci[bot], your commit 8772e0b has 4 failures in
🧪 To try this PR locally: bunx bun-pr 28743That installs a local version of the PR into your bun-28743 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes modify fetch response body buffering in Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Problem
Fetch response body memory is never returned to the OS after GC collects the Blob/Response objects (#28741, #12941, #28427).
Reproduction:
Node.js releases ~73% of memory in the same scenario; Bun released 0%.
Root Cause
FetchTasklet.callback()runs on the HTTP thread, where it appends response data toscheduled_response_bufferviawrite(). This allocates the buffer on the HTTP thread's mimalloc heap.When the body is later consumed and GC finalizes the Blob,
Store.deref()→Bytes.deinit()callsmi_free()on the main JS thread. In mimalloc, cross-thread frees go to the allocating thread's delayed-free list. Since the HTTP thread is idle after fetches complete, it never processes that list — so the pages are never returned to the OS.Manual
new Blob([data])works fine because the data is allocated on the main thread from the start.Fix
In
cloneResponseBufferToMainThread(), when transferring the response buffer fromFetchTaskletto the body value (which always happens on the main thread inonBodyReceived/toBodyValue), clone the data into a fresh main-thread allocation. The old HTTP-thread buffer is freed immediately.Verification
The test spawns a local server, fetches 30×1MB blobs, drops all references, forces GC, and asserts >20% memory is reclaimed. Passes with the fix, fails on system bun.
Closes #28741