Compare commits

..

2 Commits

Author SHA1 Message Date
autofix-ci[bot]
3a00e9953b [autofix.ci] apply automated fixes 2026-02-20 05:29:54 +00:00
Claude Bot
1b982511c3 fix(timers): use-after-finalize crash in setImmediate cleanup paths
When a setImmediate callback's JS object is garbage collected after
clearImmediate or unref downgrades the JSRef to weak, the queued
immediate task would call downgrade() on an already-finalized JSRef,
hitting a debugAssert in debug builds and causing silent corruption
in release builds (manifesting as truncated multipart upload bodies).

Replace downgrade() with deinit() in the two cleanup paths where the
JSRef may already be finalized:
- runImmediateTask early-exit (cleared/unref'd immediates)
- fire() when tryGet() returns null (GC'd timer objects)

Also remove the debug-only panic in runImmediateTask's tryGet-null
path, since a null result is a valid state when GC runs between
cancel/unref and the queued task execution.

Closes #19097

Co-Authored-By: Claude <noreply@anthropic.com>
2026-02-20 05:27:57 +00:00
7 changed files with 85 additions and 128 deletions

View File

@@ -77,15 +77,18 @@ pub fn runImmediateTask(this: *TimerObjectInternals, vm: *VirtualMachine) bool {
(!this.flags.is_keeping_event_loop_alive and !vm.isEventLoopAliveExcludingImmediates()))
{
this.setEnableKeepingEventLoopAlive(vm, false);
this.this_value.downgrade();
// Use deinit() instead of downgrade() because the JSRef may already be
// in the finalized state (GC collected the JS object after cancel/unref
// downgraded the reference to weak).
this.this_value.deinit();
this.deref();
return false;
}
const timer = this.this_value.tryGet() orelse {
if (Environment.isDebug) {
@panic("TimerObjectInternals.runImmediateTask: this_object is null");
}
// The JS object was garbage collected. This can happen when the GC
// runs after the strong reference was downgraded (e.g. by cancel or
// unref) but before the queued immediate task executes.
this.setEnableKeepingEventLoopAlive(vm, false);
this.deref();
return false;
@@ -133,7 +136,9 @@ pub fn fire(this: *TimerObjectInternals, _: *const timespec, vm: *jsc.VirtualMac
const this_object = this.this_value.tryGet() orelse {
this.setEnableKeepingEventLoopAlive(vm, false);
this.flags.has_cleared_timer = true;
this.this_value.downgrade();
// Use deinit() instead of downgrade() because tryGet() returned null,
// which means the JSRef is in the finalized state.
this.this_value.deinit();
this.deref();
return;
};
@@ -504,7 +509,6 @@ const Debugger = @import("../../Debugger.zig");
const std = @import("std");
const bun = @import("bun");
const Environment = bun.Environment;
const assert = bun.assert;
const timespec = bun.timespec;

View File

@@ -22,7 +22,6 @@
#include "BunClientData.h"
#include "CallSite.h"
#include "ErrorStackTrace.h"
#include "JSDOMException.h"
#include "headers-handwritten.h"
using namespace JSC;
@@ -380,9 +379,6 @@ static String computeErrorInfoWithoutPrepareStackTrace(
RETURN_IF_EXCEPTION(scope, {});
message = instance->sanitizedMessageString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
} else if (auto* domException = jsDynamicCast<WebCore::JSDOMException*>(errorInstance)) {
name = domException->wrapped().name();
message = domException->wrapped().message();
}
}

View File

@@ -46,10 +46,6 @@
#include <wtf/PointerPreparations.h>
#include <wtf/URL.h>
#include "FormatStackTraceForJS.h"
#include "ZigGlobalObject.h"
#include <JavaScriptCore/Interpreter.h>
namespace WebCore {
using namespace JSC;
@@ -124,34 +120,6 @@ static const HashTableValue JSDOMExceptionConstructorTableValues[] = {
{ "DATA_CLONE_ERR"_s, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::ConstantInteger, NoIntrinsic, { HashTableValue::ConstantType, 25 } },
};
static void captureStackTraceForDOMException(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject)
{
if (!vm.topCallFrame)
return;
auto* zigGlobalObject = jsDynamicCast<Zig::GlobalObject*>(lexicalGlobalObject);
if (!zigGlobalObject)
zigGlobalObject = ::defaultGlobalObject(lexicalGlobalObject);
size_t stackTraceLimit = zigGlobalObject->stackTraceLimit().value();
if (stackTraceLimit == 0)
stackTraceLimit = Bun::DEFAULT_ERROR_STACK_TRACE_LIMIT;
WTF::Vector<JSC::StackFrame> stackTrace;
vm.interpreter.getStackTrace(errorObject, stackTrace, 0, stackTraceLimit);
if (stackTrace.isEmpty())
return;
unsigned int line = 0;
unsigned int column = 0;
String sourceURL;
JSValue result = Bun::computeErrorInfoWrapperToJSValue(vm, stackTrace, line, column, sourceURL, errorObject, nullptr);
if (result)
errorObject->putDirect(vm, vm.propertyNames->stack, result, 0);
}
template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSDOMExceptionDOMConstructor::construct(JSGlobalObject* lexicalGlobalObject, CallFrame* callFrame)
{
auto& vm = JSC::getVM(lexicalGlobalObject);
@@ -394,12 +362,26 @@ void JSDOMExceptionOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* conte
// #endif
// #endif
JSC::JSValue toJSNewlyCreated(JSC::JSGlobalObject* lexicalGlobalObject, JSDOMGlobalObject* globalObject, Ref<DOMException>&& impl)
JSC::JSValue toJSNewlyCreated(JSC::JSGlobalObject*, JSDOMGlobalObject* globalObject, Ref<DOMException>&& impl)
{
auto* wrapper = createWrapper<DOMException>(globalObject, WTF::move(impl));
auto& vm = globalObject->vm();
captureStackTraceForDOMException(vm, lexicalGlobalObject ? lexicalGlobalObject : globalObject, wrapper);
return wrapper;
// if constexpr (std::is_polymorphic_v<DOMException>) {
// #if ENABLE(BINDING_INTEGRITY)
// // const void* actualVTablePointer = getVTablePointer(impl.ptr());
// #if PLATFORM(WIN)
// void* expectedVTablePointer = __identifier("??_7DOMException@WebCore@@6B@");
// #else
// // void* expectedVTablePointer = &_ZTVN7WebCore12DOMExceptionE[2];
// #endif
// // If you hit this assertion you either have a use after free bug, or
// // DOMException has subclasses. If DOMException has subclasses that get passed
// // to toJS() we currently require DOMException you to opt out of binding hardening
// // by adding the SkipVTableValidation attribute to the interface IDL definition
// // RELEASE_ASSERT(actualVTablePointer == expectedVTablePointer);
// #endif
// }
return createWrapper<DOMException>(globalObject, WTF::move(impl));
}
JSC::JSValue toJS(JSC::JSGlobalObject* lexicalGlobalObject, JSDOMGlobalObject* globalObject, DOMException& impl)

View File

@@ -56,7 +56,8 @@ describe("DOMException in Node.js environment", () => {
expect(DOMException.DATA_CLONE_ERR).toBe(25);
});
it("inherits prototype properties from Error", () => {
// TODO: missing stack trace on DOMException
it.failing("inherits prototype properties from Error", () => {
const error = new DOMException("Test error");
expect(error.toString()).toBe("Error: Test error");
expect(error.stack).toBeDefined();

View File

@@ -67,7 +67,7 @@ describe("AbortSignal", () => {
function fmt(value: any) {
const res = {};
for (const key in value) {
if (key === "column" || key === "line" || key === "sourceURL" || key === "stack") continue;
if (key === "column" || key === "line" || key === "sourceURL") continue;
res[key] = value[key];
}
return res;

View File

@@ -0,0 +1,53 @@
import { expect, test } from "bun:test";
// Regression test for https://github.com/oven-sh/bun/issues/19097
// setImmediate callbacks whose JS objects get GC'd after clearImmediate
// would crash with a use-after-finalize panic when the queued immediate
// task tried to downgrade the already-finalized JSRef.
test("clearImmediate followed by GC does not crash", () => {
// Schedule many immediates and immediately clear them.
// This puts the immediate tasks in the event loop queue with
// has_cleared_timer=true. After downgrading to weak refs via cancel(),
// GC can finalize the JS objects, putting the JSRef into the .finalized
// state. When the queued tasks run and try to clean up, they must not
// assert on the finalized state.
for (let i = 0; i < 1000; i++) {
const id = setImmediate(() => {});
clearImmediate(id);
}
// Force garbage collection to finalize the cleared immediate objects.
Bun.gc(true);
// Let the event loop drain the queued immediate tasks.
return new Promise<void>(resolve => {
setImmediate(() => {
// If we get here without crashing, the bug is fixed.
resolve();
});
});
});
test("unref'd setImmediate with GC pressure does not crash", async () => {
// Schedule many unref'd immediates with GC pressure.
// Unref'd immediates can hit the early-exit path in runImmediateTask
// when the event loop has no other work. If GC finalizes the JS object
// between unref and the task running, the JSRef will be in .finalized state.
const promises: Promise<void>[] = [];
for (let batch = 0; batch < 10; batch++) {
for (let i = 0; i < 100; i++) {
const id = setImmediate(() => {});
// @ts-ignore - unref exists on Immediate
id.unref();
}
Bun.gc(false);
// Keep the event loop alive with a resolved promise to allow
// immediate tasks to drain.
await new Promise<void>(resolve => setTimeout(resolve, 0));
}
// If we got here without a crash, the fix works.
expect(true).toBe(true);
});

View File

@@ -1,79 +0,0 @@
import { expect, test } from "bun:test";
test("DOMException from new DOMException() has a stack trace", () => {
const e = new DOMException("test error", "AbortError");
expect(typeof e.stack).toBe("string");
expect(e.stack).toContain("AbortError: test error");
expect(e.stack).toContain("17877.test");
expect(e instanceof DOMException).toBe(true);
expect(e instanceof Error).toBe(true);
});
test("DOMException from AbortSignal.abort() has a stack trace", () => {
const signal = AbortSignal.abort();
try {
signal.throwIfAborted();
expect.unreachable();
} catch (err: any) {
expect(typeof err.stack).toBe("string");
expect(err.stack).toContain("AbortError");
expect(err.stack).toContain("The operation was aborted");
expect(err instanceof DOMException).toBe(true);
expect(err instanceof Error).toBe(true);
}
});
test("DOMException stack trace includes correct name and message", () => {
const e = new DOMException("custom message", "NotFoundError");
expect(typeof e.stack).toBe("string");
expect(e.stack).toStartWith("NotFoundError: custom message\n");
});
test("DOMException with default args has a stack trace", () => {
const e = new DOMException();
expect(typeof e.stack).toBe("string");
expect(e.name).toBe("Error");
expect(e.message).toBe("");
});
test("DOMException stack trace shows correct call site", () => {
function createException() {
return new DOMException("inner", "DataError");
}
const e = createException();
expect(typeof e.stack).toBe("string");
expect(e.stack).toContain("createException");
});
test("DOMException.stack is writable", () => {
const e = new DOMException("test", "AbortError");
expect(typeof e.stack).toBe("string");
e.stack = "custom stack";
expect(e.stack).toBe("custom stack");
});
test("DOMException from AbortSignal.abort() with custom reason has no stack on reason", () => {
const reason = "custom reason string";
const signal = AbortSignal.abort(reason);
try {
signal.throwIfAborted();
expect.unreachable();
} catch (err: any) {
// When a custom reason (non-DOMException) is used, it's thrown as-is
expect(err).toBe("custom reason string");
}
});
test("DOMException from AbortSignal.abort() with DOMException reason has stack", () => {
const reason = new DOMException("custom abort", "AbortError");
const signal = AbortSignal.abort(reason);
try {
signal.throwIfAborted();
expect.unreachable();
} catch (err: any) {
expect(err).toBe(reason);
expect(typeof err.stack).toBe("string");
expect(err.stack).toContain("AbortError: custom abort");
}
});