bun:jsc: release throw scope before JSONParse in generateHeapSnapshotForDebugging by robobun · Pull Request #36857 · oven-sh/bun

robobun · 2026-08-03T22:33:04Z

Repro

BUN_JSC_validateExceptionChecks=1 bun -e 'const s = require("bun:jsc").generateHeapSnapshotForDebugging(); console.log("ok", typeof s)'

ERROR: Unchecked JS exception:
    This scope can throw a JS exception: parseRecursively @ vendor/WebKit/Source/JavaScriptCore/runtime/LiteralParser.cpp:1395
    But the exception was unchecked as of this scope: functionGenerateHeapSnapshotForDebugging @ src/jsc/modules/BunJSCModule.h:801
ASSERTION FAILED: exception check validation failed

Exit 134 on debug/assert builds, deterministic. Release builds without assertions print ok object and exit 0.

Cause

functionGenerateHeapSnapshotForDebugging declares a DECLARE_THROW_SCOPE, calls scope.releaseAssertNoException() after building the snapshot JSON, then returns JSValue::encode(JSONParse(globalObject, ...)) without releasing the scope. JSONParse calls LiteralParser::parseRecursively which can throw, so the validator flags the scope as unchecked.

Fix

Wrap the return in RELEASE_AND_RETURN(scope, ...), matching the upstream jsc.cpp implementation of functionGenerateHeapSnapshot / functionGenerateHeapSnapshotForGCDebugging.

Verification

Added a test in test/js/bun/jsc/bun-jsc.test.ts that spawns with BUN_JSC_validateExceptionChecks=1 and asserts the snapshot returns an object with exit 0 and no unchecked-scope report. Fails before (exit 134, validator names functionGenerateHeapSnapshotForDebugging), passes after.

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

…ForDebugging functionGenerateHeapSnapshotForDebugging declares a throw scope, asserts no exception after building the snapshot, then returns JSValue::encode(JSONParse(...)) without releasing the scope. JSONParse (LiteralParser::parseRecursively) can throw, so under BUN_JSC_validateExceptionChecks=1 the process aborts with 'exception check validation failed' naming this function. Wrap the return in RELEASE_AND_RETURN(scope, ...) to match the upstream jsc.cpp implementation of the same function.

coderabbitai · 2026-08-03T22:33:11Z

Walkthrough

Heap snapshot generation now returns parsed JSON through RELEASE_AND_RETURN. A subprocess regression test validates successful execution with exception-check validation enabled.

Changes

Heap snapshot generation

Layer / File(s)	Summary
Preserve heap snapshot cleanup semantics `src/jsc/modules/BunJSCModule.h`	Heap snapshot JSON now returns through `RELEASE_AND_RETURN`.
Validate exception-check execution `test/js/bun/jsc/bun-jsc.test.ts`	A subprocess test verifies object output, clean exit, and no unchecked-exception scope diagnostics.

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 exception-scope fix in generateHeapSnapshotForDebugging.
Description check	✅ Passed	The description explains the cause, fix, reproduction, and verification, although it does not use the template headings.

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

robobun · 2026-08-03T22:33:12Z

Status: diff is green, ready for review. CI is failing on unrelated flakes.

Reproduced with BUN_JSC_validateExceptionChecks=1 bun-debug -e 'require("bun:jsc").generateHeapSnapshotForDebugging()' (exit 134, validator names functionGenerateHeapSnapshotForDebugging). After the one-line RELEASE_AND_RETURN change: exit 0, ok object. Full bun-jsc.test.ts suite: 37 pass / 0 fail on both CI runs.

CI (builds #88371, #88421): the new bun-jsc.test.ts test passed on every lane including debian-13-x64-asan. Remaining red lanes are unrelated to this change:

worker-transfer-terminate-stress.test.ts (x64-asan, build 88371 only): known intermittent, tracked by ASAN CI: ExceptionScope::assertNoException during worker terminate (worker-transfer-terminate-stress, separate from #34095) #34690; passed on the 88421 re-run.
run-crash-handler.test.ts (Windows x64, build 88421): PowerShell crash-upload timing; unrelated to a C++ throw-scope change in bun:jsc.
Assorted [flaky] tests that passed on retry (install-patch, webview-chrome, dlopen-duplicate-load, symbols, inspect-error-leak, napi node-gyp EBUSY on Windows aarch64).

Needs a maintainer to merge.

claude

LGTM — one-line RELEASE_AND_RETURN around a throwing tail call, matching the upstream jsc.cpp pattern.

What was reviewed:

Confirmed JSONParse is a throwing tail call under an active DECLARE_THROW_SCOPE; RELEASE_AND_RETURN is the canonical fix per REVIEW.md.
Checked sibling JSONParse sites in the same file — functionSamplingProfilerStackTraces releases via releaseAssertNoException() after the parse, and functionMemoryUsageStatistics has no throw scope, so neither shares this bug.
Test spawns with BUN_JSC_validateExceptionChecks=1, drains pipes concurrently, spreads bunEnv, and asserts on exit code + stdout + a filtered stderr view (not a bare "no panic" check).

Extended reasoning...

Overview

One-line change in src/jsc/modules/BunJSCModule.h: the return statement in functionGenerateHeapSnapshotForDebugging is wrapped in RELEASE_AND_RETURN(scope, ...) so the throw scope is released before JSONParse (which can throw via LiteralParser::parseRecursively). A regression test is added to test/js/bun/jsc/bun-jsc.test.ts that spawns a subprocess with BUN_JSC_validateExceptionChecks=1 and asserts exit 0, ok object on stdout, and no unchecked-scope diagnostics on stderr.

Security risks

None. This is a debug-assertion hygiene fix in a diagnostics-only API (bun:jsc heap snapshot). No new inputs are parsed, no trust boundaries change, and release-build behavior is unchanged.

Level of scrutiny

Low. RELEASE_AND_RETURN on a throwing tail call is the exact idiom REVIEW.md prescribes ("Throwing tail calls go through RELEASE_AND_RETURN"), and the PR description cites the matching upstream jsc.cpp implementations of functionGenerateHeapSnapshot / functionGenerateHeapSnapshotForGCDebugging. The change is mechanical and self-evidently correct.

Other factors

Sibling sites: I checked the other JSONParse callers in this file. functionSamplingProfilerStackTraces calls scope.releaseAssertNoException() immediately after JSONParse (releasing the scope), and functionMemoryUsageStatistics / the report lambda in functionRunProfiler either lack a throw scope or check the exception afterward — none share the exact unchecked-tail-return pattern fixed here.
Test quality: The test follows harness conventions — await using on the spawn, Promise.all on stdout/stderr/exited, {...bunEnv, ...} spread, and a combined-object assertion. It filters stderr for the specific validator diagnostic lines rather than doing a blanket "stderr is empty" check, which keeps it robust against unrelated debug logging. The comment honestly notes that on release builds the env var is a no-op and the test just exercises the snapshot path, so it will not vacuously fail on release CI lanes.
Verification: The robobun status comment reports the repro (exit 134 before, exit 0 after) and a clean 37-pass run of the full bun-jsc.test.ts suite.

github-actions · 2026-08-03T22:46:25Z

This PR may be a duplicate of:

jsc: treat DFG Plan::m_mustHandleValues as weak so queued compiles don't root user objects #34640 - Contains the byte-identical RELEASE_AND_RETURN(scope, ...) fix around JSONParse in functionGenerateHeapSnapshotForDebugging in src/jsc/modules/BunJSCModule.h (plus a test/js/bun/jsc/bun-jsc.test.ts change), so this PR's entire source change is already included there.

🤖 Generated with Claude Code

robobun · 2026-08-03T22:48:50Z

Not a duplicate of #34640. That PR is about making DFG Plan::m_mustHandleValues weak (a WebKit change, currently pinned to a preview build and carrying merge conflicts). It includes the same RELEASE_AND_RETURN line as a drive-by because its GC-rooting fixture calls generateHeapSnapshotForDebugging and would otherwise abort under exception-check validation; it has no test for the exception check itself.

This PR is the standalone fix for the exception-check finding with a dedicated validator test, no WebKit dependency. Whichever lands first, the other rebases cleanly: if this lands first, #34640 drops its one-line BunJSCModule.h hunk; if #34640 lands first, this becomes test-only.

claude

LGTM — one-line RELEASE_AND_RETURN around a throwing tail call, matching the standard JSC pattern.

What was reviewed

Confirmed JSONParse is a throwing tail call under a live DECLARE_THROW_SCOPE; RELEASE_AND_RETURN is the documented fix.
Checked sibling JSONParse sites in this file (functionSamplingProfilerStackTraces, functionMemoryUsageStatistics) — different scope patterns, not the same class.
Test follows harness conventions: {...bunEnv, ...}, concurrent pipe drain, combined-object assertion; on release builds validateExceptionChecks is a no-op so it degrades to a smoke test rather than a false pass.

Extended reasoning...

Overview

One-line source change in src/jsc/modules/BunJSCModule.h: functionGenerateHeapSnapshotForDebugging now wraps its return JSValue::encode(JSONParse(...)) in RELEASE_AND_RETURN(scope, ...). A new subprocess test in test/js/bun/jsc/bun-jsc.test.ts spawns with BUN_JSC_validateExceptionChecks=1 and asserts exit 0 with ok object on stdout and no unchecked-scope lines on stderr.

Security risks

None. generateHeapSnapshotForDebugging is a debug-only introspection API in bun:jsc; the change is purely exception-scope bookkeeping and does not touch parsing of untrusted input, auth, crypto, or permissions.

Level of scrutiny

Low. This is the textbook JSC exception-check idiom — REVIEW.md explicitly states "Throwing tail calls go through RELEASE_AND_RETURN", and the PR description notes upstream jsc.cpp's functionGenerateHeapSnapshot/functionGenerateHeapSnapshotForGCDebugging use the identical pattern. The prior scope.releaseAssertNoException() covers the snapshot-builder region; the new macro releases the scope for the throwing JSONParse at the tail. There is no behavioral change on release builds.

Other factors

No CODEOWNERS entry covers src/jsc/modules/ or this test file.
robobun verified locally: reproduced the exit-134 abort before, exit 0 after, full bun-jsc.test.ts suite 37/0.
Test structure matches repo conventions: bunExe()/bunEnv spread, await using for the process, Promise.all over stdout/stderr/exited, single combined-object expect. The stderr filter targets the validator's specific diagnostic lines rather than a broad substring, so unrelated debug-build noise won't flip it.
I looked at the two other JSONParse call sites in the same file for the "fix the whole class" rule: functionSamplingProfilerStackTraces computes the result then calls scope.releaseAssertNoException() (asserts engine-generated JSON never throws — a different, intentional pattern), and functionMemoryUsageStatistics declares no throw scope at all. Neither is the same shape as this fix, so scoping to the one function is correct.
The duplicate-PR bot flagged #34640; robobun already explained that PR carries this line as a drive-by inside a larger WebKit change with merge conflicts, and whichever lands first the other rebases cleanly.

robobun · 2026-08-03T23:22:07Z

^{Updated 6:59 PM PT - Aug 3rd, 2026}

❌ @robobun, your commit 0f9b35c has 1 failures in Build #88421 (All Failures):

test/cli/run/run-crash-handler.test.ts - pid 5988 invoked crashbypanic() handler on 🪟 2019 x64

🧪 To try this PR locally:

bunx bun-pr 36857

That installs a local version of the PR into your bun-36857 executable, so you can run:

bun-36857 --bun

robobun assigned Jarred-Sumner Aug 3, 2026

github-actions Bot added the claude label Aug 3, 2026

claude Bot reviewed Aug 3, 2026

View reviewed changes

ci: retrigger

0f9b35c

claude Bot reviewed Aug 3, 2026

View reviewed changes

coderabbitai Bot mentioned this pull request Aug 6, 2026

Propagate exceptions from boxed primitive unwrapping in YAML/TOML/JSON5 stringify #37040

Closed

This was referenced Aug 6, 2026

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

Open

bun:jsc: propagate JSONParse exceptions from samplingProfilerStackTraces #37065

Open

console.takeHeapSnapshot: report snapshot parse failures instead of aborting #37070

Open

Navigation Menu

Uh oh!

bun:jsc: release throw scope before JSONParse in generateHeapSnapshotForDebugging - #36857

bun:jsc: release throw scope before JSONParse in generateHeapSnapshotForDebugging#36857
robobun wants to merge 2 commits into
mainfrom
farm/315b8965/jsc-heap-snapshot-exception-check

robobun commented Aug 3, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

robobun commented Aug 3, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

robobun commented Aug 3, 2026

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Aug 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

robobun commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Repro

Cause

Fix

Verification

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

robobun commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

robobun commented Aug 3, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Aug 3, 2026 •

edited

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

robobun commented Aug 3, 2026 •

edited

Loading

robobun commented Aug 3, 2026 •

edited

Loading