Skip to content

Return empty MarkedBlocks at end of collection, releasing their memory off the pause#293

Open
Jarred-Sumner wants to merge 2 commits into
mainfrom
claude/return-empty-blocks-at-end-of-collection
Open

Return empty MarkedBlocks at end of collection, releasing their memory off the pause#293
Jarred-Sumner wants to merge 2 commits into
mainfrom
claude/return-empty-blocks-at-end-of-collection

Release returned blocks' memory after the world resumes, not inside t…

5668b21
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 15, 2026 in 27m 9s

Code review found 1 potential issue

Found 3 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit Source/JavaScriptCore/heap/MarkedSpace.cpp:437-450 Data race on m_blockBytesAllocated in destroyDeferredBlocks (RESOURCE_USAGE builds)

Annotations

Check warning on line 450 in Source/JavaScriptCore/heap/MarkedSpace.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Data race on m_blockBytesAllocated in destroyDeferredBlocks (RESOURCE_USAGE builds)

In `ENABLE(RESOURCE_USAGE)` builds, `destroyDeferredBlocks()` runs after `changePhase()` has resumed the mutator, so `delete block` → `~Handle()` → `heap.didFreeBlock()` does `m_blockBytesAllocated -= capacity` on the collector thread while the resumed mutator's `didAllocateBlock()` does `+= capacity` — and `m_blockBytesAllocated` is a plain `size_t` (Heap.h:991), so this is a lost-update race. The audit's own table row 5 flags this field, but the "none of this touches shared state" comment here