Repository navigation
Conversation
|
Preview build of 9415508: |
robobun
force-pushed
the
robobun/69b487cb/console-client-exception-handoff
branch
from
October 11, 2026 08:48
cc38fc8 to
7d297f5
Compare
…oleClient call
Nine host functions in ConsoleObject.cpp declare a ThrowScope, call a
ConsoleClient hook, and return with no check and no release. A hook
that declares a ThrowScope of its own leaves a simulated throw pending
(Bun's messageWithTypeAndLevel does). With validateExceptionChecks the
destructor of the caller's scope then aborts:
ERROR: Unchecked JS exception:
This scope can throw a JS exception: messageWithTypeAndLevel
But the exception was unchecked as of this scope: consoleProtoFuncAssert @ runtime/ConsoleObject.cpp:218
assert, count, countReset, profile, profileEnd, takeHeapSnapshot, time,
timeLog and timeEnd call scope.release() before the client call (12
call sites). The caller of the host function owns the check after that.
scope.release() is empty when exception scope verification is off, so
the code of a release build does not change.
robobun
force-pushed
the
robobun/69b487cb/console-client-exception-handoff
branch
from
October 16, 2026 07:15
7d297f5 to
9415508
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
console.assert(false)aborts a debug build of Bun undervalidateExceptionChecks=1:But the exception was unchecked as of this scope: consoleProtoFuncAssert @ runtime/ConsoleObject.cpp:218.runtime/ConsoleObject.cppdeclare a ThrowScope, call aConsoleClienthook, and return with no check or release (12 exits). Bun's hooks can throw.assertaborts today: only its hook declares a ThrowScope (since Add jsc-exception-lint and fix the missing exception checks it finds bun#40410).Fix
scope.release()before the client call (11 sites), so their caller owns the check.assertloses its scope and itsRETURN_IF_EXCEPTION, becauseJSValue::toBoolean()cannot throw.console.assertforms abort before, 0 after. With a ThrowScope in each Bun hook: 12 of 12 exits abort before, 0 after.Background
validateExceptionChecks, a ThrowScope destructor marks a check as needed unless it returns to JIT code. The next unreleased ThrowScope asserts on that mark.RETURN_IF_EXCEPTIONafter each client call (a load and a branch in release code,scope.release()is free), and a change in Bun's hook only (hides the 12 exits).Downsides
ConsoleObject.cppwas byte-identical to upstream. It now differs by +11/-3 lines.console.assertno longer polls VM traps. Its loop does (check_traps). A Worker spinning inconsole.assert(true)terminates in 10 of 10 runs before and after.consoleProtoFuncAssertshrinks from 522 to 404 bytes, the other eight are identical.Notes
The 12 exits, by line on
main:assert:229,count:243,countReset:257,profile:271 and :278,profileEnd:292 and :299,takeHeapSnapshot:313 and :320,time:334,timeLog:348,timeEnd:362. The 17 other console host functions declare no scope and need no change.Why upstream does not see it.
JSGlobalObjectConsoleClientand the WebCore client stringify under a catch scope and never throw. Bun'smessageWithTypeAndLevel(src/jsc/bindings/ConsoleObject.cpp:33in oven-sh/bun) declares a ThrowScope, throws a TypeError for a badconsole.tableargument, and ends withRELEASE_AND_RETURNinto a formatter that can run user code.console.assert(false, { [Symbol.for("nodejs.util.inspect.custom")]() { throw new Error("boom") } })throwsboomto the caller in Bun and in Node.js.No JSTests test. The
jscshell has no console client that throws, so a stress test cannot fail before this change. The regression test is in oven-sh/bun#44950 (test/js/bun/jsc/exception-checks.test.ts, four rows, each exits 134 on the parent commit).All runs are debug ASAN builds of Bun on Linux x64 that differ only in the WebKit build: the parent commit (
0c06faad, Bun's current pin) and this PR's preview build (autobuild-preview-pr-790-cc38fc87).Forms. Each alone under the validator:
console.assert(false),console.assert(),console.assert(false, "x"),console.assert(0, {a:1}),console.assert(null, "%s:%d", "a", 1),console.assert("", new Error("e")),Reflect.apply(console.assert, console, [false, "x"]),[0].forEach(console.assert),JSON.stringify({ toJSON: console.assert }).console.assert(true)exits 0 on both builds.The 11 exits that are inert today.
scripts/jsc-exception-lintin oven-sh/bun) on this file, with summaries forConsoleClient.cpp,ScriptArguments.cppandScriptCallStackFactory.cpp: 12 findings before (allunchecked-exit, one for each exit), 0 after.RELEASE_AND_RETURNin each of Bun's eight other hooks, and one snippet for each exit. Before: 12 of 12 abort, each at its own scope (consoleProtoFuncCount @ ConsoleObject.cpp:235,consoleProtoFuncTime @ :326, and so on). After: 0 of 12.takeHeapSnapshothook because of exits :313 and :320.[console-client]entries. They can go after this change.Simulated throws for each call under the validator (gdb hit count on
JSC::ThrowScope::simulateThrow, 1,001 calls minus 101 calls, divided by 900).console.count("x"): 0 before and after.console.log("x"): 0 and 0.console.timeLog("l", {}): 5 and 5.console.assert(true): 0 and 0.console.assert(false): abort at the first call before, 0 after. In a validator build each of the 11 call sites now stores one flag byte (m_isReleased).Release code. This file at the parent commit and at this commit, each compiled alone with clang 23.1.2 and the flags of the
bun-webkit-linux-amd64-ltobuild (-O3, without-flto).consoleProtoFuncAssert: 522 to 404 bytes, 149 to 119 instructions. The path ofconsole.assert(true)through it: 55 to 46 instructions, 8 to 7 conditional branches. The other 26 functions of the file: 0 differingobjdumplines.VM traps.
RETURN_IF_EXCEPTIONalso handles pending VM traps. The one removed fromassertwas such a poll.for (let i = 0; i < n; i++) console.assert(true)compiles toloop_hint,check_traps, ...,call_ignore_result, so the loop polls on each iteration. Worker test: the worker posts one message and spins infor (;;) console.assert(true), the parent waits 300 ms and callsterminate(). Interleaved, 10 runs for each build: 10 of 10 terminate on both. Median 380 ms (241 to 491) before, 430 ms (237 to 550) after. The difference is inside the spread. I did not make a release build of Bun with this change.