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
2 changed files with 63 additions and 6 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

@@ -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);
});