bun:jsc: add missing exception checks in heapStats()

[robobun]

bun:jsc's heapStats() (functionMemoryUsageStatistics in src/jsc/modules/BunJSCModule.h) declared no throw scope and never checked for pending exceptions:

• JSONParse(globalObject, String::fromUTF8(json)) over the mimalloc stats JSON can throw (out of memory while building the parse tree). Only parsed.isEmpty() was handled; a pending exception was not.

• arg0.getObject()->get(globalObject, "dump") then ran with that exception pending. When the caller passed { dump: ... } the property exists, which is exactly the condition asserted by JSC::JSObject::get, and the assert that keeps showing up in fuzzing on assert-enabled builds:

  ASSERTION FAILED: !scope.exception() || vm.hasPendingTerminationException() || !hasProperty
  JSObjectInlines.h(137) : JSC::JSObject::get(JSGlobalObject *, PropertyName)
  

  On release builds the same state runs the user's dump getter with an exception pending.

• A throwing dump accessor (or Proxy trap) left its exception pending through toBoolean, the mi_heap_dump_json branch, and the second JSONParse, and the function returned a normal object with the exception still set.

Fix

Declare a throw scope at the top and RETURN_IF_EXCEPTION after the four calls that can throw: both JSONParse calls, the dump property read, and the dump mode string read (toWTFString resolves ropes and can throw on OOM). A parse failure that does not throw still takes the existing jsNull() fallback. On a throw the function now propagates the error instead of continuing into property reads and user JS with an exception pending, matching the neighboring functions in this file (functionSetRandomSeed, functionDrainMicrotasks).

Verification

The new test runs every heapStats() path (plain, {dump: true}, a rope "blocks" string, a throwing getter, a throwing Proxy trap) in a child process with BUN_JSC_validateExceptionChecks=1. Now that the function declares a throw scope, that option makes debug builds abort on any unchecked throw here. Removing the check after the property read reproduces the abort deterministically:

This scope can throw a JS exception: get @ JSObjectInlines.h:133
But the exception was unchecked as of this scope: functionMemoryUsageStatistics @ BunJSCModule.h:230
ASSERTION FAILED: exception check validation failed

The JSONParse checks are policed by the same mechanism (LiteralParser's scopes simulate a throw into this function's scope). The getter and Proxy cases additionally pin that a user exception from the dump lookup propagates to the caller and that heapStats() keeps working afterwards. On release builds the option is a no-op and the test just exercises the paths.

A note on reproducing the original assert: the unfixed function declares no throw scope, and JSC's exception check validation only polices functions that declare one (a callee's simulated throw is skipped when the next scope down sits across the VM entry frame, see ThrowScope::~ThrowScope). The only real trigger on the unfixed code is OOM inside JSONParse of mimalloc's own small, valid stats JSON, which a test cannot stage. Throwing accessors and Proxy traps complete cleanly end to end on the unfixed build because the pending exception surfaces at the host call boundary (verified against an unfixed debug build, with and without BUN_JSC_validateExceptionChecks=1). So the test guards the fixed shape, and fails if any of the now-checked throws goes unchecked again, rather than reproducing the OOM-only assert.

Related: #36857 fixes the same pattern in generateHeapSnapshotForDebugging.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/jsc/heapStats-mimalloc.test.ts

[coderabbitai]

Walkthrough

Changes

heapStats exception safety

Layer / File(s)	Summary
Add exception checks src/jsc/modules/BunJSCModule.h	functionMemoryUsageStatistics now checks JavaScript exceptions during JSON parsing, dump-option access and conversion, and heap-dump parsing.
Validate exception behavior test/js/bun/jsc/heapStats-mimalloc.test.ts	Tests cover throwing getters, Proxy traps, rope strings, error propagation, recovery, clean output, and subprocess termination.

Possibly related PRs

• oven-sh/bun#36857: Adds similar exception-safety checks in BunJSCModule.h.
• oven-sh/bun#37025: Adds throw-scope checks around JavaScript-convertible operations.
• oven-sh/bun#37026: Handles exceptions from JavaScript property access in Bun/JSC code.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the module and the primary change: adding missing exception checks in heapStats().
Description check	✅ Passed	The description explains the change, affected operations, preserved behavior, and detailed verification through targeted exception-safety tests.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical addition of a throw scope and four RETURN_IF_EXCEPTION checks following the standard JSC pattern.

What was reviewed:

• Confirmed mi_free(json) runs before each exception check, so the mimalloc buffer doesn't leak on the throw path.
• toBoolean is non-throwing in JSC, so no check needed there; toWTFString (rope resolution) is correctly checked.
• Test drains stdout/stderr/exited concurrently, spreads bunEnv, and asserts a combined object with exitCode/stdout as the load-bearing checks.
• PR description explains why the test guards the fixed shape rather than reproducing the OOM-only original assert — reasonable given the trigger cannot be staged.

Extended reasoning...

Overview

This PR adds missing JSC exception checks to functionMemoryUsageStatistics (heapStats() in bun:jsc). The change is 5 lines in src/jsc/modules/BunJSCModule.h: one DECLARE_THROW_SCOPE at function entry and four RETURN_IF_EXCEPTION macros after the calls that can throw — two JSONParse calls, the get(globalObject, "dump") property read (which can run user getters/Proxy traps), and toWTFString (rope resolution can OOM). A new test in test/js/bun/jsc/heapStats-mimalloc.test.ts exercises each path under BUN_JSC_validateExceptionChecks=1.

Security risks

None. bun:jsc is an internal debugging module. The change strictly narrows behavior — it now propagates exceptions instead of continuing with one pending. No new user-controlled data paths are introduced.

Level of scrutiny

Low-to-medium. The pattern is mechanical and matches neighboring functions in the same file (functionSetRandomSeed, functionDrainMicrotasks, functionSerialize). I checked the two subtleties that matter for this pattern: (1) resource cleanup ordering — both mi_free(json) calls happen before RETURN_IF_EXCEPTION, so no leak on the throw path; (2) toBoolean is non-throwing per JSC semantics so needs no check, while toWTFString is correctly checked because rope resolution allocates. The final plain return (no RELEASE_AND_RETURN) matches the file's convention and is safe because the last potentially-throwing call on every path is checked.

Other factors

The test follows harness conventions: subprocess with concurrent pipe draining, {...bunEnv, ...} spread, combined-object assertion. The uncheckedScopes stderr filter is diagnostic; the load-bearing assertions are stdout: "OK\n" and exitCode: 0 (a validateExceptionChecks abort would fail both), plus the in-script checks that getter/Proxy exceptions propagate as "boom"/"trap". The PR description candidly explains that the test cannot fail on the unfixed build (no throw scope → validation doesn't police it; the only real trigger is unstageable OOM in JSONParse), so it guards against regressing the fixed shape rather than reproducing the original assert. Given that constraint is well-documented and the native change is trivially correct by inspection, this is fine to approve.