From dd67cda545a7b6567779d4444bfee487cae35211 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 27 Jun 2025 22:05:20 -0700 Subject: [PATCH] Fixes #20615 (#20616) --- src/bun.js/api/bun/udp_socket.zig | 2 +- src/bun.js/bindings/CatchScope.zig | 41 +++++++++++--------------- src/bun.js/bindings/JSGlobalObject.zig | 2 +- src/bun.js/bindings/JSValue.zig | 6 ++-- src/bun.js/bindings/bindings.cpp | 7 +++-- src/bun.js/bindings/headers.h | 2 +- src/bun.js/virtual_machine_exports.zig | 8 ++--- src/env.zig | 1 + 8 files changed, 33 insertions(+), 36 deletions(-) diff --git a/src/bun.js/api/bun/udp_socket.zig b/src/bun.js/api/bun/udp_socket.zig index 0a94d13ac1..6e11447454 100644 --- a/src/bun.js/api/bun/udp_socket.zig +++ b/src/bun.js/api/bun/udp_socket.zig @@ -380,7 +380,7 @@ pub const UDPSocket = struct { const globalThis = this.globalThis; const vm = globalThis.bunVM(); - if (err.isTerminationException(vm.jsc)) { + if (err.isTerminationException()) { return; } if (callback == .zero) { diff --git a/src/bun.js/bindings/CatchScope.zig b/src/bun.js/bindings/CatchScope.zig index bcd98f5480..2d07c70ddd 100644 --- a/src/bun.js/bindings/CatchScope.zig +++ b/src/bun.js/bindings/CatchScope.zig @@ -22,9 +22,8 @@ const size = 56; const alignment = 8; bytes: [size]u8 align(alignment), -global: *jsc.JSGlobalObject, /// Pointer to `bytes`, set by `init()`, used to assert that the location did not change -location: if (Environment.allow_assert) *u8 else void, +location: if (Environment.ci_assert) *u8 else void, enabled: bool, pub const Enable = enum { @@ -46,13 +45,13 @@ pub fn init( /// If you need to do something different when there is an exception, leave enabled. /// If you are only using the scope to prove you handle exceptions correctly, you can pass /// `Environment.allow_assert` as `enabled`. - enable_condition: Enable, + comptime enable_condition: Enable, ) void { - const enabled = switch (enable_condition) { + const enabled = comptime switch (enable_condition) { .enabled => true, - .assertions_only => Environment.allow_assert, + .assertions_only => Environment.ci_assert, }; - if (enabled) { + if (comptime enabled) { CatchScope__construct( &self.bytes, global, @@ -65,8 +64,7 @@ pub fn init( } self.* = .{ .bytes = self.bytes, - .global = global, - .location = if (Environment.allow_assert) &self.bytes[0], + .location = if (Environment.ci_assert) &self.bytes[0], .enabled = enabled, }; } @@ -86,14 +84,14 @@ pub fn hasException(self: *CatchScope) bool { /// Get the thrown exception if it exists (like scope.exception() in C++) pub fn exception(self: *CatchScope) ?*jsc.Exception { - if (Environment.allow_assert) bun.assert(self.location == &self.bytes[0]); + if (comptime Environment.ci_assert) bun.assert(self.location == &self.bytes[0]); if (!self.enabled) return null; return CatchScope__pureException(&self.bytes); } /// Get the thrown exception if it exists, or if an unhandled trap causes an exception to be thrown pub fn exceptionIncludingTraps(self: *CatchScope) ?*jsc.Exception { - if (Environment.allow_assert) bun.assert(self.location == &self.bytes[0]); + if (comptime Environment.ci_assert) bun.assert(self.location == &self.bytes[0]); if (!self.enabled) return null; return CatchScope__exceptionIncludingTraps(&self.bytes); } @@ -106,7 +104,7 @@ pub fn returnIfException(self: *CatchScope) bun.JSError!void { /// Asserts there has not been any exception thrown. pub fn assertNoException(self: *CatchScope) void { - if (Environment.allow_assert) { + if (comptime Environment.ci_assert) { if (self.exception()) |e| self.assertionFailure(e); } } @@ -115,7 +113,7 @@ pub fn assertNoException(self: *CatchScope) void { /// Prefer over `assert(scope.hasException() == ...)` because if there is an unexpected exception, /// this function prints a trace of where it was thrown. pub fn assertExceptionPresenceMatches(self: *CatchScope, should_have_exception: bool) void { - if (Environment.allow_assert) { + if (comptime Environment.ci_assert) { // paranoid; will only fail if you manually changed enabled to false bun.assert(self.enabled); if (should_have_exception) { @@ -131,20 +129,17 @@ pub fn assertExceptionPresenceMatches(self: *CatchScope, should_have_exception: /// If non-termination exception, assertion failure. pub fn assertNoExceptionExceptTermination(self: *CatchScope) bun.JSExecutionTerminated!void { bun.assert(self.enabled); - return if (self.exception()) |e| - if (jsc.JSValue.fromCell(e).isTerminationException(self.global.vm())) - error.JSExecutionTerminated - else if (Environment.allow_assert) - self.assertionFailure(e) - else - // we got an exception other than the termination one, but we can't assert. - // treat this like the termination exception so we still bail out - error.JSExecutionTerminated - else {}; + if (self.exception()) |e| { + if (jsc.JSValue.fromCell(e).isTerminationException()) + return error.JSExecutionTerminated + else if (comptime Environment.ci_assert) + self.assertionFailure(e); + // Unconditionally panicing here is worse for our users. + } } pub fn deinit(self: *CatchScope) void { - if (Environment.allow_assert) bun.assert(self.location == &self.bytes[0]); + if (comptime Environment.ci_assert) bun.assert(self.location == &self.bytes[0]); if (!self.enabled) return; CatchScope__destruct(&self.bytes); self.bytes = undefined; diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index 85637512e9..1380682452 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -553,7 +553,7 @@ pub const JSGlobalObject = opaque { /// pub fn reportActiveExceptionAsUnhandled(this: *JSGlobalObject, err: bun.JSError) void { const exception = this.takeException(err); - if (!exception.isTerminationException(this.vm())) { + if (!exception.isTerminationException()) { _ = this.bunVM().uncaughtException(this, exception, false); } } diff --git a/src/bun.js/bindings/JSValue.zig b/src/bun.js/bindings/JSValue.zig index bbb5037a6b..2407342255 100644 --- a/src/bun.js/bindings/JSValue.zig +++ b/src/bun.js/bindings/JSValue.zig @@ -1138,9 +1138,9 @@ pub const JSValue = enum(i64) { null; } - extern fn JSC__JSValue__isTerminationException(this: JSValue, vm: *VM) bool; - pub fn isTerminationException(this: JSValue, vm: *VM) bool { - return JSC__JSValue__isTerminationException(this, vm); + extern fn JSC__JSValue__isTerminationException(this: JSValue) bool; + pub fn isTerminationException(this: JSValue) bool { + return JSC__JSValue__isTerminationException(this); } extern fn JSC__JSValue__toZigException(this: JSValue, global: *JSGlobalObject, exception: *ZigException) void; diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 983943a557..73ca4d9383 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -5287,10 +5287,13 @@ void JSC__VM__setExecutionTimeLimit(JSC::VM* vm, double limit) watchdog.setTimeLimit(WTF::Seconds { limit }); } -bool JSC__JSValue__isTerminationException(JSC::EncodedJSValue JSValue0, JSC::VM* arg1) +bool JSC__JSValue__isTerminationException(JSC::EncodedJSValue JSValue0) { JSC::Exception* exception = JSC::jsDynamicCast(JSC::JSValue::decode(JSValue0)); - return exception != NULL && arg1->isTerminationException(exception); + if (exception == nullptr) + return false; + + return exception->vm().isTerminationException(exception); } extern "C" void JSC__Exception__getStackTrace(JSC::Exception* arg0, JSC::JSGlobalObject* global, ZigStackTrace* trace) diff --git a/src/bun.js/bindings/headers.h b/src/bun.js/bindings/headers.h index 1f93ac5e7f..9b3cde8af6 100644 --- a/src/bun.js/bindings/headers.h +++ b/src/bun.js/bindings/headers.h @@ -256,7 +256,7 @@ CPP_DECL bool JSC__JSValue__isObject(JSC::EncodedJSValue JSValue0); CPP_DECL bool JSC__JSValue__isPrimitive(JSC::EncodedJSValue JSValue0); CPP_DECL bool JSC__JSValue__isSameValue(JSC::EncodedJSValue JSValue0, JSC::EncodedJSValue JSValue1, JSC::JSGlobalObject* arg2); CPP_DECL bool JSC__JSValue__isSymbol(JSC::EncodedJSValue JSValue0); -CPP_DECL bool JSC__JSValue__isTerminationException(JSC::EncodedJSValue JSValue0, JSC::VM* arg1); +CPP_DECL bool JSC__JSValue__isTerminationException(JSC::EncodedJSValue JSValue0); CPP_DECL bool JSC__JSValue__isUInt32AsAnyInt(JSC::EncodedJSValue JSValue0); CPP_DECL bool JSC__JSValue__jestDeepEquals(JSC::EncodedJSValue JSValue0, JSC::EncodedJSValue JSValue1, JSC::JSGlobalObject* arg2); CPP_DECL bool JSC__JSValue__jestDeepMatch(JSC::EncodedJSValue JSValue0, JSC::EncodedJSValue JSValue1, JSC::JSGlobalObject* arg2, bool arg3); diff --git a/src/bun.js/virtual_machine_exports.zig b/src/bun.js/virtual_machine_exports.zig index 7d23011496..e81efc744e 100644 --- a/src/bun.js/virtual_machine_exports.zig +++ b/src/bun.js/virtual_machine_exports.zig @@ -86,11 +86,9 @@ pub export fn Bun__queueTaskWithTimeout(global: *JSGlobalObject, task: *JSC.CppT pub export fn Bun__reportUnhandledError(globalObject: *JSGlobalObject, value: JSValue) callconv(.C) JSValue { JSC.markBinding(@src()); - // This JSGlobalObject might not be the main script execution context - // See the crash in https://github.com/oven-sh/bun/issues/9778 - const vm = JSC.VirtualMachine.get(); - if (!value.isTerminationException(vm.jsc)) { - _ = vm.uncaughtException(globalObject, value, false); + + if (!value.isTerminationException()) { + _ = globalObject.bunVM().uncaughtException(globalObject, value, false); } return .js_undefined; } diff --git a/src/env.zig b/src/env.zig index 4b39e571d9..bb84ac155d 100644 --- a/src/env.zig +++ b/src/env.zig @@ -26,6 +26,7 @@ pub const isX86 = @import("builtin").target.cpu.arch.isX86(); pub const isX64 = @import("builtin").target.cpu.arch == .x86_64; pub const isMusl = builtin.target.abi.isMusl(); pub const allow_assert = isDebug or isTest or std.builtin.Mode.ReleaseSafe == @import("builtin").mode; +pub const ci_assert = isDebug or isTest or enable_asan or (std.builtin.Mode.ReleaseSafe == @import("builtin").mode and is_canary); pub const show_crash_trace = isDebug or isTest or enable_asan; /// All calls to `@export` should be gated behind this check, so that code /// generators that compile Zig code know not to reference and compile a ton of