Conversation
heapUsed read JSC's sizeAfterLastEdenCollection(), which is
totalBytesVisited + extraMemorySize(), so ArrayBuffer backing stores and
other off-heap bytes landed in it. heapTotal read blockBytesAllocated(),
which counts only MarkedBlocks. The two numbers described different
things, and heapUsed could exceed heapTotal by ~60x:
const bufs = Array.from({ length: 40 }, () => new ArrayBuffer(4 << 20));
process.memoryUsage();
// heapUsed: 33743574, heapTotal: 617472
Take both from the marked space instead: heapTotal is
objectSpace().capacity() and heapUsed is objectSpace().size(), so
heapUsed <= heapTotal holds by construction and off-heap bytes stay in
external/arrayBuffers, where node reports them.
objectSpace().size() sums the mark bits of every block, so it only
changes when a collection does. Cache it per collection, keyed on
newlyAllocatedVersion(), which advances in MarkedSpace::endMarking().
Reads stay O(1): on a 496MB heap, process.memoryUsage() is 4.19us
against 4.16us before, while the uncached walk would be 891us.
heapUsed now also responds to full collections. It previously only
tracked eden ones, so Bun.gc(true) left it unchanged, which made the
diagnostics_channel leak test pass without comparing anything. Assert
there that an unsubscribed channel is collected instead.
|
Updated 9:49 AM PT - Jul 5th, 2026
❌ @robobun, your commit e44bbf7 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 33368That installs a local version of the PR into your bun-33368 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
WalkthroughAdds a cached ChangesHeap Usage Accounting Update
Sequence Diagram(s)sequenceDiagram
participant JS as JS code
participant MemoryUsage as Process_functionMemoryUsage
participant Process as Process::heapUsedBytes
participant ObjectSpace as vm.heap.objectSpace()
JS->>MemoryUsage: process.memoryUsage()
MemoryUsage->>ObjectSpace: capacity()
MemoryUsage->>Process: heapUsedBytes(vm)
Process->>ObjectSpace: newlyAllocatedVersion()
alt version changed
Process->>ObjectSpace: size()
Process->>Process: cache updated bytes
else version unchanged
Process->>Process: return cached bytes
end
Process-->>MemoryUsage: heapUsed
MemoryUsage-->>JS: {heapTotal, heapUsed, ...}
Estimated code review effort: 3/5 Related issues: Related PRs: None specified. Suggested labels: performance, node.js-apis, tests Suggested reviewers: None specified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/jsc/bindings/BunProcess.h`:
- Around line 36-40: The heapUsedBytes cache in BunProcess can return a stale
zero on the first call because m_heapUsedVersion uses the same nullVersion
sentinel as newlyAllocatedVersion(). Update BunProcess::heapUsedBytes() to treat
the cache as uninitialized on first read, using a distinct sentinel or an
explicit first-call initialization path so objectSpace.size() is computed before
returning.
In `@test/js/node/process/process.test.js`:
- Around line 581-642: This subprocess-based invariant test is still defined as
a sequential `it(...)`, but the testing guideline prefers concurrent execution
for process/file I/O cases. Update the `process.memoryUsage()` test to use
`test.concurrent` or place it under a concurrent `describe` alongside the
surrounding tests, keeping the existing `Bun.spawn`/`proc` assertions intact. If
the file already has a concurrent wrapper, no change is needed; otherwise
convert this specific test so it can run in parallel safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a9bd916-6580-4b82-ba03-ad027338a6bc
📒 Files selected for processing (4)
src/jsc/bindings/BunProcess.cppsrc/jsc/bindings/BunProcess.htest/js/node/diagnostics_channel/diagnostics_channel.test.tstest/js/node/process/process.test.js
JSC::MarkedSpace::nullVersion instead of a bare 0, so the declaration shows why the sentinel can never collide with a live version.
|
Confirmed #20793 is the same bug and added Their report ( Both review findings are checked and withdrawn:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…marking A full collection's MarkedSpace::beginMarking() bumps m_markingVersion, which stales every block's mark bits, and flips every precise allocation. The mutator runs JS through the concurrent part of marking, so a MarkedSpace::size() walk in that window sums a nearly empty heap. Since newlyAllocatedVersion() only advances in endMarking(), a first read for a given version that lands there would cache the torn value and serve it for the rest of the collection. Fall back on the last snapshot while a full collection is marking. Eden collections only bump m_edenVersion, leaving the existing marks valid, so they are not skipped: a blanket isMarking() guard reports zero, because a collection is typically already marking by the first memoryUsage() call. Assert that heapUsed never reads as an empty heap.
The snapshot served while a full collection is marking can be several collections old, since the cache is only refreshed by a memoryUsage() call landing outside that window. capacity() meanwhile is read fresh, and it falls as freeBlock() and sweepPreciseAllocations() hand memory back. A snapshot taken when the heap was large, paired with a capacity measured after it shrank, is heapUsed > heapTotal again. Read heapTotal once and clamp heapUsed to it. Within one version the cached size is safe already, because a sweep only frees blocks that held no marked cells, so this only binds the stale-version fallback. Also stop reporting an empty heap when that fallback has nothing cached: a torn walk is a real measurement, and not latching it keeps it from being served for the rest of the collection. The new test drops 60 large arrays and checks the pair across the shrink. It fails on main twice over, because blockBytesAllocated() counts MarkedBlocks but not the PreciseAllocations those arrays live in, so heapTotal reads 554KB while 28MB is committed.
There was a problem hiding this comment.
Thanks — eb62449 addresses both points from my last round (the std::min clamp against heapTotal and the nullVersion fall-through to a non-latching walk), and the new shrinking-heap test covers the stale-snapshot case. I'm deferring to a human for the semantic sign-off: this redefines heapUsed to step at collections rather than climb continuously, and the concurrent-GC-phase reasoning is subtle enough that a maintainer familiar with the JSC heap should confirm it.
Extended reasoning...
Overview
This PR changes process.memoryUsage() in src/jsc/bindings/BunProcess.{cpp,h} to source both heapTotal and heapUsed from JSC's MarkedSpace (objectSpace().capacity() / objectSpace().size()) instead of the previous mismatched pair (blockBytesAllocated() / sizeAfterLastEdenCollection()), fixing the heapUsed > heapTotal invariant violation reported in #20793. It adds a per-Process cache keyed on newlyAllocatedVersion() to keep the block walk O(1)-amortized, with a guard that skips the walk during a Full GC's concurrent marking phase (where mark bits are stale) and a std::min clamp at the call site so the reported heapUsed can never exceed the heapTotal it ships with. Two subprocess tests are added in process.test.js and one vacuous test in diagnostics_channel.test.ts is rewritten to actually assert what it was meant to.
What changed since my last review
My prior inline comment (10:58 UTC) flagged two residual issues in the Full-GC guard added in d6bb5e2: a stale cached value could exceed a shrunk capacity(), and a first-ever call landing in that window would return the initial 0. Commit eb62449 addresses both — the clamp is applied at the caller (covering every return path, not just the guard's), and the nullVersion case now does an unlatched walk with a comment explaining why it must not be cached. A second test exercising the shrinking-heap path was also added. I consider that thread resolved.
Security risks
None. This is read-only heap accounting for a diagnostic API; no user input flows into it and no state is mutated beyond the two cache fields on Process.
Level of scrutiny
Medium-high. The change is small in line count but the correctness argument depends on non-obvious JSC GC internals: which collector phases the mutator runs through, when markingVersion vs newlyAllocatedVersion advance, and what MarkedSpace::size() sums under concurrent marking. Two real bugs were found and fixed during review, which suggests the space is subtle. It also intentionally changes user-observable semantics — heapUsed now steps at collections rather than climbing per-allocation (documented in the PR's "Known limit" section) — which is a Node-compat tradeoff a maintainer should ratify.
Other factors
The PR is well-tested (invariant sampled across 40 allocations, growth/shrink both covered, existing suites re-checked), the author's responses to review have been thorough and source-cited, and the bug-hunting system found nothing on the current revision. I'm not approving because the concurrent-GC reasoning and the behavioral-semantics change both warrant a human maintainer's sign-off rather than a bot's.
x64-baseline left 4 of the 1000 channels alive and the test asserted zero. Conservative stack scanning pins whichever ones still have a pointer in a live stack slot or register, and no amount of retrying clears that, so asking for all of them is asking JSC for a guarantee it does not make. Assert they are collectable instead. Retaining a reference, which is what the test guards against, leaves all 1000 alive, against the handful the scan pins, so the bound still separates the two by more than an order of magnitude. Verified by holding a strong reference: 1000 survive and the assertion fails.
|
CI caught a real flake, in my own test rather than the fix. Pushed 6f36b5f.
Conservative stack scanning pins whichever channels still have a pointer in a live stack slot or register, and retrying does not clear that, so "all 1000" is a guarantee JSC does not make. The assertion now checks they are collectable: retaining a reference, which is the thing the test guards against, leaves every one of the 1000 alive, against the handful the scan pins. Confirmed both directions locally, including on debug+ASAN: The existing Where the rest of CI stands: the last two completed builds ran 282 and 281 lanes green. The only other failure is
On the deferral above: agreed that the semantic change wants a human. The short version of what needs ratifying is that |
There was a problem hiding this comment.
Both of my earlier findings are addressed and I found nothing new, but this reworks process.memoryUsage() around JSC's concurrent-GC internals (version-keyed cache, full-marking guard, clamp) and changes heapUsed's observable semantics — worth a human sign-off on the design and the diagnostics_channel test rewrite.
Extended reasoning...
Overview
This PR changes process.memoryUsage() to source heapTotal and heapUsed from the same JSC accounting (objectSpace().capacity() / objectSpace().size()) so heapUsed <= heapTotal holds. Because MarkedSpace::size() walks every block, it adds a per-collection cache on Process keyed by newlyAllocatedVersion(), plus a guard that skips the walk during a Full GC's concurrent marking (where mark bits are torn), a fallback for the never-cached first call, and a std::min clamp at the pair-production site. It rewrites the diagnostics_channel leak test (previously vacuous — heapUsed never moved across gc(true)) to assert channels are collectable via WeakRef, and adds two subprocess tests covering off-heap growth and heap shrink.
Security risks
None. This is read-only heap introspection with no user-controlled input reaching allocation, parsing, or privileged operations.
Level of scrutiny
Medium-high. The change is small in line count but its correctness rests on non-obvious JSC GC invariants (when m_markingVersion vs m_newlyAllocatedVersion advance, which collection scopes stale mark bits, how m_capacity moves relative to a cached size()). It went through two rounds of real bugs found in review before landing at the current shape, and two of the guard paths (torn read during concurrent full marking; the clamp) are acknowledged as untestable because the window can't be forced deterministically. It also changes user-observable behavior: heapUsed now steps at collections rather than climbing per-allocation like V8's — the PR calls this out as a known limit, but a maintainer should agree that's an acceptable trade.
Other factors
- Both of my prior inline findings (torn read during concurrent marking; stale cache exceeding fresh capacity + first-call zero) were confirmed real and fixed in d6bb5e2 and eb62449; those threads are resolved.
- The diagnostics_channel test rewrite changes what is asserted (WeakRef collectability with a
< total/10threshold, vs the old heapUsed comparison). The new assertion is more direct but the threshold and the 20-roundgc(true)loop are worth a human glance for flakiness. - The cache state lives on
Process(per-VM), which is the right place for per-VM state per CLAUDE.md; new fields are plain scalars so no GC-visitor changes needed. - Test coverage for the headline invariant is solid; the untestable paths are defended by cited JSC source lines and
BUN_JSC_logGCtraces in the thread, but not by CI.
|
Nothing actionable, but two things worth putting on the record, one of them a bug that is not mine and not in this diff. On the A real bug the test rewrite walked into. While hardening it I found that import { subscribe, hasSubscribers } from "node:diagnostics_channel";
subscribe("keep", () => {});
for (let i = 0; i < 10; i++) { await new Promise(r => setImmediate(r)); globalThis.gc(); }
console.log(hasSubscribers("keep"));
// node: true
// bun: false
This reproduces on I left it out of this PR deliberately. It is a different subsystem, the fix changes retention semantics, and this PR has already been through three rounds. Happy to send it separately, with |
CI status: the diff is green, one lane is broken on infrastructureSummarising five builds so a reviewer does not have to dig, because this PR will not show a fully green board and the reason has nothing to do with the code. The diff passes. The last build that ran to completion (#68548) was 283 lanes green, 1 red. The recurring red is
Everything else that ever went red, and what it was:
The one real failure CI caught was my own: the leak test asserted all 1000 channels get collected, and conservative stack scanning left 4 alive on x64-baseline. Fixed, and verified both directions (a strong reference leaves 1000 alive and fails the assertion). The I spent my one re-roll on e44bbf7; it came back to the same darwin timeout. Not pushing more, since each push only queues another build into the same broken lane. Where this stands for review
The one thing that needs a maintainer's judgement, stated plainly: |
|
Correcting one thing in my summary above: I called the Build #68575 (
That same SIGKILL is hitting three unrelated branches right now:
all with the identical For completeness on why it cannot be this diff: So: the diff is green, the two red lanes are a Buildkite artifact-download timeout and a test that is OOMing for everyone today. My one re-roll is spent and came back to the same pair, so I am not pushing again. Nothing here needs a code change from me. This is ready for a maintainer whenever the semantic question above is settled. |
…39593) ### Problem - `process.memoryUsage().heapUsed` does not change after a full collection. It keeps the figure of the last eden collection. After `Bun.gc(true)` frees 10 MB, `heapUsed` still reports the 10 MB, and it is larger than `heapTotal`. - In a process that runs without the JIT (`BUN_JSC_useJIT=0`), `heapUsed` is 0 for the life of the process. JSC turns off generational collection in that mode (`VM::isInMiniMode()`), so every collection is a full one. - In a fresh process, `heapUsed` is 0 right after `Bun.gc(true)`. The full collection serves the pending startup request, so no eden collection has run. - Cause: `Process_functionMemoryUsage` in `src/jsc/bindings/BunProcess.cpp` reads `heap.sizeAfterLastEdenCollection()`. `Heap::updateAllocationLimits()` writes that counter after an eden collection only. A full collection writes `m_sizeAfterLastFullCollect`. The counter that is current after both, `m_sizeAfterLastCollect`, has no accessor. ### Fix - `JSVMClientData` owns a `JSC::HeapObserver` (`Bun::HeapSizeAfterLastCollection`, `src/jsc/bindings/BunClientData.h`). It attaches to the heap when the VM is created and detaches when the VM is destroyed. `didGarbageCollect(scope)` copies the counter of the scope that just ran. `process.memoryUsage()` reports that copy. - The copy is exact. `Heap::runEndPhase()` calls `updateAllocationLimits()`, which stores the size of the collection in the counter for its scope and in `m_sizeAfterLastCollect`, and then `didFinishCollection()`, which notifies the observers with the same scope. So the copy always equals `m_sizeAfterLastCollect`. - The read stays O(1), so `process.memoryUsage()` stays usable in a monitoring loop. `heap.size()` would be current too, but it walks every block of the heap. - The observer runs in the end phase of a collection, while the mutator is stopped. The mutator reads the value after it resumes. This is the same contract under which it reads JSC's own `sizeAfterLast*Collection()` counters today. - The client data is created right after the VM (`Zig__GlobalObject__create`), before any global object exists, so the observer sees every collection of the heap. Each worker has its own VM and its own copy. `~VM` deletes the client data while the heap is still alive, so the detach is safe. - `test/js/node/diagnostics_channel/diagnostics_channel.test.ts`, "references are not leaked", compared `heapUsed` from before a loop with `heapUsed` after a full collection. The two numbers were always equal, because `heapUsed` did not move across the full collection. With this change the first number dates from an earlier collection in the file and the comparison fails. The test now checks what the node test is after: once unsubscribed, nothing holds the channels. It holds a `WeakRef` per channel and counts the ones that survive `gc(true)`. Locally 1 or 2 of 1000 survive (conservative stack scanning). A retained reference keeps all 1000 alive. - Verified with `test/js/node/process/process.test.js`, describe "process.memoryUsage().heapUsed reports the most recent collection". One child runs a full, an eden, and a full collection and checks that `heapUsed` equals the figure each one returned. One child runs with `BUN_JSC_useJIT=false`. Both fail on the released build (`heapUsed` is `[0, eden, eden]` and `0`) and pass with this change. - Also run with the debug build: `test/js/node/diagnostics_channel/diagnostics_channel.test.ts`, `test/js/bun/globals.test.js` ("cleans up memory" now measures a real drop), `test/js/bun/jsc/bun-jsc.test.ts`, `test/js/node/v8/v8-module.test.ts`, and the node tests `test-memory-usage.js`, `test-sqlite-template-tag.js` (its leak check reads 2.71 MB before and 2.76 MB after, against a 1.5x bound), `test-v8-collect-gc-profile*.js`, `test-v8-stats.js`, `test-worker-heap-statistics.js`, `test-gc-tls-external-memory.js`. Related PRs. #39541 touches the same line of `BunProcess.cpp`. It adds a fallback to the full counter when the eden counter is 0, for the fresh process case above, and it collects once when both are 0. With this observer in place, that fallback reduces to one check of `heapSizeAfterLastCollection()`. Whichever lands second resolves a small conflict there. #33368 changes what `heapUsed` and `heapTotal` measure (it takes both from the marked space, for #20793). This PR keeps the current measure and only fixes which collection it comes from. ### Background JSC's collector is generational. An eden collection marks only the objects allocated since the previous collection. A full collection marks the whole heap. At the end of either one, `Heap::updateAllocationLimits()` computes the live size (bytes visited plus the extra memory that live cells reported). It stores it in `m_sizeAfterLastEdenCollect` or `m_sizeAfterLastFullCollect`, depending on the scope of the collection, and always in `m_sizeAfterLastCollect`. Only the first two have accessors. A `JSC::HeapObserver` is an interface with `willGarbageCollect()` and `didGarbageCollect(CollectionScope)`. `Heap::addObserver()` registers one. The heap calls `didGarbageCollect` from `Heap::didFinishCollection()`, in the end phase of every collection. The end phase runs with the world stopped (`worldShouldBeSuspended()` in `CollectorPhase.cpp`), which can be on the collector thread. `GCProfilerObserver` in `src/jsc/bindings/NodeV8.h` is the existing observer in this codebase. It reads the same counters by scope. `JSVMClientData` (`src/jsc/bindings/BunClientData.h`) is Bun's per VM data. It is created in `JSVMClientData::create()` right after the VM, and `~VM` deletes it after `heap.lastChanceToFinalize()`, while the heap, a member of the VM, is still alive. <details> <summary>Repro on the released build and with this change</summary> ```js let a = []; for (let i = 0; i < 200000; i++) a.push({ i, s: "x" + i }); console.log(process.memoryUsage().heapUsed); a = null; console.log(Bun.gc(true)); console.log(process.memoryUsage()); ``` Released build (1.4.0): ``` 13324137 156982 { rss: 66416640, heapTotal: 636928, heapUsed: 13324137, external: 25830, arrayBuffers: 0 } ``` With this change: ``` 13438091 156412 { rss: 389029888, heapTotal: 636928, heapUsed: 156412, external: 28236, arrayBuffers: 0 } ``` Without the JIT, released build, after allocating 300000 objects: `heapUsed` is `0`. With this change: `11833741`. </details>
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-05, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. The linked issue (#20793) stays open. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
1 participant
Fixes #20793
What does this PR do?
Sources
process.memoryUsage()'sheapTotalandheapUsedfrom the same heap, so thatheapUsed <= heapTotalholds. TodayheapUsedcan exceedheapTotalby ~55x.Repro
heapUsedexceedsheapTotalby ~55x. Anything that divides the two (autoscalers, heap-pressure watchdogs, APM dashboards) gets a meaningless ratio.Cause
The two fields came from different accountings.
heapUsedwasHeap::sizeAfterLastEdenCollection(). JSC computes that inupdateAllocationLimits()asm_totalBytesVisited + extraMemorySize(), andextraMemorySize()ism_extraMemorySize + m_deprecatedExtraMemorySize + m_arrayBuffers.size(). So every ArrayBuffer backing store was counted as JS heap.heapTotalwasHeap::blockBytesAllocated(), which only counts MarkedBlocks and WeakBlocks, and knows nothing about extra memory. It also misses PreciseAllocations entirely, so large backing stores are invisible to it: 60 arrays of 60k elements commit 28MB andmainstill reportsheapTotal: 554KB.A second, quieter problem:
m_sizeAfterLastEdenCollectis only written by eden collections. A full collection never updates it, soheapUseddid not move acrossBun.gc(true)at all:Fix
Take both numbers from the marked space:
heapTotal=vm.heap.objectSpace().capacity()heapUsed=vm.heap.objectSpace().size()MarkedSpace::size()sumsmarkCount() * cellSize()per block plus marked precise allocations, andm_capacitycountsMarkedBlock::blockSizeper block plus every precise allocation'scellSize(). So read together,size() <= capacity()holds by construction. Off-heap bytes stay inexternal/arrayBuffers, which is where node reports them and where they already were.heapUsedis reported asstd::min(heapUsedBytes(), heapTotal). The two are read together on the normal path, but the snapshot served during a collection (below) can be several collections old, andm_capacityfalls in the meantime asfreeBlock()andsweepPreciseAllocations()hand memory back. Live bytes can never exceed committed bytes, so clamping is the honest bound on a value that is a stale approximation by construction. Within one version a cache hit needs no clamp: a sweep only frees blocks whosemarkCount()was 0, and those contributed nothing to the cachedsize().Keeping the read O(1)
MarkedSpace::size()walks every block. It is also a sum of mark bits, and outside of marking those only change when a collection does, so caching it per collection returns the identical number for an O(1) read. The key isobjectSpace().newlyAllocatedVersion(), which advances inMarkedSpace::endMarking()on every collection, eden and full.nullVersionis 0 and never live, so it doubles as the "not computed yet" sentinel.The walk has to be skipped while a full collection is marking. Its
beginMarking()bumpsm_markingVersion, which stales every block's marks (MarkedBlock::markCount()then returns 0) and flips every precise allocation, and the mutator keeps running JS through the concurrent part of marking, so a walk there sums a nearly empty heap. SincenewlyAllocatedVersion()only advances atendMarking(), a first read for a given version landing in that window would cache the torn value and serve it for the rest of the collection. Eden collections only bumpm_edenVersionand leave the marks valid, so they are not skipped: a collection is usually already marking by the firstprocess.memoryUsage()call, and blanket-skipping onisMarking()makes that first read return zero.With nothing cached yet the fallback has nothing to serve, so it takes the torn walk rather than claim an empty heap, and deliberately does not latch it.
Release build,
process.memoryUsage()in a loop:heap.size()process.memoryUsage.rss(), which is only the/proc/self/statmread, is 4.0 us at every size. The heap accounting is free; the syscall is the whole cost, before and after.Known limit
heapUsedis the live set as of the last collection, so it steps at collections rather than climbing with each allocation the way V8'sused_heap_sizedoes. JSC does maintain a gross allocation counter (Heap::totalBytesAllocatedThisCycle(), private), and it is cheap: the marked-space site isLocalAllocator::allocateSlowCase, called once per free-list refill rather than per object. ButHeap::didAllocate()is also called fromreportExtraMemoryAllocatedSlowCase()and fromHeap::addReference(cell, ArrayBuffer*), so that counter folds in the same off-heap bytes this PR is removing fromheapUsed. A continuously-climbingheapUsedwould need a new marked-space-only counter in JSC.How did you verify your code works?
test/js/node/process/process.test.jssamplesprocess.memoryUsage()after each of 40 x 4 MiB ArrayBuffer allocations and asserts no sample violates the invariant, that the backing stores show up inarrayBuffers(andexternalcovers them), thatheapUsedstill tracks JS object allocation, and that no sample reads as an empty heap.Fails on
mainwith 32/40 samples violating, passes here.A second test drops 60 arrays of 60k elements and checks the pair across the shrink, covering the path where
freeBlock()andsweepPreciseAllocations()pull capacity down (24776KB -> 496KB). It fails onmaintwice over, sinceblockBytesAllocated()never saw those butterflies in the first place.There is no regression test for the torn read during full marking, nor for the clamp: I could not force that window deterministically, and the guard is one bool read. The mechanism is the JSC source quoted above, plus
BUN_JSC_logGC=1showing the mutator being handed the world back mid-FullCollection.test/js/node/diagnostics_channel/diagnostics_channel.test.tscomparedheapUsedbefore the work againstheapUsedafter agc(true). SinceheapUsednever moved across a full collection, both reads returned the same byte count and the assertion could not fail. It now asserts directly that unsubscribed channels are only weakly held, which is what the node test it ports approximates. Conservative stack scanning pins whichever ones still have a pointer in a live stack slot, so the assertion is that they are collectable rather than collected: retaining a reference leaves all 1000 alive, against the handful the scan pins. Confirmed both ways by holding a strong reference.Other suites checked
test/js/node/test/parallel/test-memory-usage.js,test/js/bun/globals.test.js("cleans up memory", which was also comparing two identical numbers and now measures a real drop),test/js/bun/jsc/bun-jsc.test.ts: all pass.