Skip to content

Commit 7bcf603

Browse files
Jarred-SumnerJarred-Sumner
authored
Fix setTimeout(() => {}, 0) and align setImmediate behavior with Node.js (oven-sh#6674)
* Fix setTimeout(() => {}, 0) * Align `setImmediate` with Node.js * Update event_loop.zig * Update test * use Bun.sleep --------- Co-authored-by: Jarred Sumner <[email protected]>
1 parent c700a70 commit 7bcf603

File tree

9 files changed

+169
-30
lines changed

9 files changed

+169
-30
lines changed

‎src/bun.js/api/bun.zig

+28-3
Original file line numberDiff line numberDiff line change
@@ -3630,7 +3630,6 @@ pub const Timer = struct {
36303630
var map = vm.timer.maps.get(kind);
36313631

36323632
// setImmediate(foo)
3633-
// setTimeout(foo, 0)
36343633
if (kind == .setTimeout and interval == 0) {
36353634
var cb: CallbackJob = .{
36363635
.callback = JSC.Strong.create(callback, globalThis),
@@ -3651,7 +3650,7 @@ pub const Timer = struct {
36513650
job.task = CallbackJob.Task.init(job);
36523651
job.ref.ref(vm);
36533652

3654-
vm.enqueueTask(JSC.Task.init(&job.task));
3653+
vm.enqueueImmediateTask(JSC.Task.init(&job.task));
36553654
if (vm.isInspectorEnabled()) {
36563655
Debugger.didScheduleAsyncCall(globalThis, .DOMTimer, Timeout.ID.asyncID(.{ .id = id, .kind = kind }), !repeat);
36573656
}
@@ -3693,6 +3692,31 @@ pub const Timer = struct {
36933692
);
36943693
}
36953694

3695+
pub fn setImmediate(
3696+
globalThis: *JSGlobalObject,
3697+
callback: JSValue,
3698+
arguments: JSValue,
3699+
) callconv(.C) JSValue {
3700+
JSC.markBinding(@src());
3701+
const id = globalThis.bunVM().timer.last_id;
3702+
globalThis.bunVM().timer.last_id +%= 1;
3703+
3704+
const interval: i32 = 0;
3705+
3706+
const wrappedCallback = callback.withAsyncContextIfNeeded(globalThis);
3707+
3708+
Timer.set(id, globalThis, wrappedCallback, interval, arguments, false) catch
3709+
return JSValue.jsUndefined();
3710+
3711+
return TimerObject.init(globalThis, id, .setTimeout, interval, wrappedCallback, arguments);
3712+
}
3713+
3714+
comptime {
3715+
if (!JSC.is_bindgen) {
3716+
@export(setImmediate, .{ .name = "Bun__Timer__setImmediate" });
3717+
}
3718+
}
3719+
36963720
pub fn setTimeout(
36973721
globalThis: *JSGlobalObject,
36983722
callback: JSValue,
@@ -3705,7 +3729,8 @@ pub const Timer = struct {
37053729

37063730
const interval: i32 = @max(
37073731
countdown.coerce(i32, globalThis),
3708-
0,
3732+
// It must be 1 at minimum or setTimeout(cb, 0) will seemingly hang
3733+
1,
37093734
);
37103735

37113736
const wrappedCallback = callback.withAsyncContextIfNeeded(globalThis);

‎src/bun.js/bindings/ZigGlobalObject.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -2207,7 +2207,8 @@ static inline EncodedJSValue functionPerformanceNowBody(JSGlobalObject* globalOb
22072207
return JSValue::encode(jsDoubleNumber(result));
22082208
}
22092209

2210-
static inline EncodedJSValue functionPerformanceGetEntriesByNameBody(JSGlobalObject* globalObject) {
2210+
static inline EncodedJSValue functionPerformanceGetEntriesByNameBody(JSGlobalObject* globalObject)
2211+
{
22112212
auto& vm = globalObject->vm();
22122213
auto* global = reinterpret_cast<GlobalObject*>(globalObject);
22132214
auto* array = JSC::constructEmptyArray(globalObject, nullptr);
@@ -2297,7 +2298,6 @@ JSC_DEFINE_HOST_FUNCTION(functionPerformanceNow, (JSGlobalObject * globalObject,
22972298
return functionPerformanceNowBody(globalObject);
22982299
}
22992300

2300-
23012301
JSC_DEFINE_HOST_FUNCTION(functionPerformanceGetEntriesByName, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
23022302
{
23032303
return functionPerformanceGetEntriesByNameBody(globalObject);
@@ -3438,8 +3438,8 @@ JSC_DEFINE_CUSTOM_GETTER(BunCommonJSModule_getter, (JSGlobalObject * globalObjec
34383438
}
34393439
return JSValue::encode(returnValue);
34403440
}
3441-
// This implementation works the same as setTimeout(myFunction, 0)
3442-
// TODO: make it more efficient
3441+
3442+
extern "C" JSC__JSValue Bun__Timer__setImmediate(JSC__JSGlobalObject* arg0, JSC__JSValue JSValue1, JSC__JSValue JSValue3);
34433443
// https://developer.mozilla.org/en-US/docs/Web/API/Window/setImmediate
34443444
JSC_DEFINE_HOST_FUNCTION(functionSetImmediate,
34453445
(JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
@@ -3480,7 +3480,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetImmediate,
34803480
}
34813481
arguments = JSValue(argumentsArray);
34823482
}
3483-
return Bun__Timer__setTimeout(globalObject, JSC::JSValue::encode(job), JSC::JSValue::encode(jsNumber(0)), JSValue::encode(arguments));
3483+
return Bun__Timer__setImmediate(globalObject, JSC::JSValue::encode(job), JSValue::encode(arguments));
34843484
}
34853485

34863486
JSValue getEventSourceConstructor(VM& vm, JSObject* thisObject)

‎src/bun.js/event_loop.zig

+53-4
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,19 @@ comptime {
608608
pub const DeferredRepeatingTask = *const (fn (*anyopaque) bool);
609609
pub const EventLoop = struct {
610610
tasks: if (JSC.is_bindgen) void else Queue = undefined,
611+
612+
/// setImmediate() gets it's own two task queues
613+
/// When you call `setImmediate` in JS, it queues to the start of the next tick
614+
/// This is confusing, but that is how it works in Node.js.
615+
///
616+
/// So we have two queues:
617+
/// - next_immediate_tasks: tasks that will run on the next tick
618+
/// - immediate_tasks: tasks that will run on the current tick
619+
///
620+
/// Having two queues avoids infinite loops creating by calling `setImmediate` in a `setImmediate` callback.
621+
immediate_tasks: Queue = undefined,
622+
next_immediate_tasks: Queue = undefined,
623+
611624
concurrent_tasks: ConcurrentTask.Queue = ConcurrentTask.Queue{},
612625
global: *JSGlobalObject = undefined,
613626
virtual_machine: *JSC.VirtualMachine = undefined,
@@ -670,11 +683,11 @@ pub const EventLoop = struct {
670683
}
671684
}
672685

673-
pub fn tickWithCount(this: *EventLoop) u32 {
686+
pub fn tickQueueWithCount(this: *EventLoop, comptime queue_name: []const u8) u32 {
674687
var global = this.global;
675688
var global_vm = global.vm();
676689
var counter: usize = 0;
677-
while (this.tasks.readItem()) |task| {
690+
while (@field(this, queue_name).readItem()) |task| {
678691
defer counter += 1;
679692
switch (task.tag()) {
680693
.Microtask => {
@@ -922,10 +935,18 @@ pub const EventLoop = struct {
922935
this.drainMicrotasksWithGlobal(global);
923936
}
924937

925-
this.tasks.head = if (this.tasks.count == 0) 0 else this.tasks.head;
938+
@field(this, queue_name).head = if (@field(this, queue_name).count == 0) 0 else @field(this, queue_name).head;
926939
return @as(u32, @truncate(counter));
927940
}
928941

942+
pub fn tickWithCount(this: *EventLoop) u32 {
943+
return this.tickQueueWithCount("tasks");
944+
}
945+
946+
pub fn tickImmediateTasks(this: *EventLoop) void {
947+
_ = this.tickQueueWithCount("immediate_tasks");
948+
}
949+
929950
pub fn tickConcurrent(this: *EventLoop) void {
930951
_ = this.tickConcurrentWithCount();
931952
}
@@ -994,6 +1015,8 @@ pub const EventLoop = struct {
9941015

9951016
ctx.onAfterEventLoop();
9961017
// this.afterUSocketsTick();
1018+
} else {
1019+
this.flushImmediateQueue();
9971020
}
9981021
}
9991022

@@ -1016,8 +1039,25 @@ pub const EventLoop = struct {
10161039
if (loop.num_polls > 0 or loop.active > 0) {
10171040
this.processGCTimer();
10181041
loop.tickWithTimeout(timeoutMs, ctx.jsc);
1042+
this.flushImmediateQueue();
10191043
ctx.onAfterEventLoop();
10201044
// this.afterUSocketsTick();
1045+
} else {
1046+
this.flushImmediateQueue();
1047+
}
1048+
}
1049+
1050+
pub fn flushImmediateQueue(this: *EventLoop) void {
1051+
// If we can get away with swapping the queues, do that rather than copying the data
1052+
if (this.immediate_tasks.count > 0) {
1053+
this.immediate_tasks.write(this.next_immediate_tasks.readableSlice(0)) catch unreachable;
1054+
this.next_immediate_tasks.head = 0;
1055+
this.next_immediate_tasks.count = 0;
1056+
} else if (this.next_immediate_tasks.count > 0) {
1057+
var prev_immediate = this.immediate_tasks;
1058+
var next_immediate = this.next_immediate_tasks;
1059+
this.immediate_tasks = next_immediate;
1060+
this.next_immediate_tasks = prev_immediate;
10211061
}
10221062
}
10231063

@@ -1041,6 +1081,7 @@ pub const EventLoop = struct {
10411081

10421082
this.processGCTimer();
10431083
loop.tick(ctx.jsc);
1084+
10441085
ctx.onAfterEventLoop();
10451086
this.tickConcurrent();
10461087
this.tick();
@@ -1064,8 +1105,11 @@ pub const EventLoop = struct {
10641105
if (loop.active > 0) {
10651106
this.processGCTimer();
10661107
loop.tick(ctx.jsc);
1108+
this.flushImmediateQueue();
10671109
ctx.onAfterEventLoop();
10681110
// this.afterUSocketsTick();
1111+
} else {
1112+
this.flushImmediateQueue();
10691113
}
10701114
}
10711115

@@ -1078,7 +1122,7 @@ pub const EventLoop = struct {
10781122

10791123
var ctx = this.virtual_machine;
10801124
this.tickConcurrent();
1081-
1125+
this.tickImmediateTasks();
10821126
this.processGCTimer();
10831127

10841128
var global = ctx.global;
@@ -1149,6 +1193,11 @@ pub const EventLoop = struct {
11491193
this.tasks.writeItem(task) catch unreachable;
11501194
}
11511195

1196+
pub fn enqueueImmediateTask(this: *EventLoop, task: Task) void {
1197+
JSC.markBinding(@src());
1198+
this.next_immediate_tasks.writeItem(task) catch unreachable;
1199+
}
1200+
11521201
pub fn enqueueTaskWithTimeout(this: *EventLoop, task: Task, timeout: i32) void {
11531202
// TODO: make this more efficient!
11541203
var loop = this.virtual_machine.event_loop_handle orelse @panic("EventLoop.enqueueTaskWithTimeout: uSockets event loop is not initialized");

‎src/bun.js/javascript.zig

+27-3
Original file line numberDiff line numberDiff line change
@@ -629,9 +629,9 @@ pub const VirtualMachine = struct {
629629
}
630630

631631
pub fn isEventLoopAlive(vm: *const VirtualMachine) bool {
632-
return vm.active_tasks > 0 or
633-
vm.event_loop_handle.?.active > 0 or
634-
vm.event_loop.tasks.count > 0;
632+
return vm.active_tasks +
633+
@as(usize, vm.event_loop_handle.?.active) +
634+
vm.event_loop.tasks.count + vm.event_loop.immediate_tasks.count + vm.event_loop.next_immediate_tasks.count > 0;
635635
}
636636

637637
const SourceMapHandlerGetter = struct {
@@ -1008,6 +1008,10 @@ pub const VirtualMachine = struct {
10081008
this.eventLoop().enqueueTask(task);
10091009
}
10101010

1011+
pub inline fn enqueueImmediateTask(this: *VirtualMachine, task: Task) void {
1012+
this.eventLoop().enqueueImmediateTask(task);
1013+
}
1014+
10111015
pub inline fn enqueueTaskConcurrent(this: *VirtualMachine, task: *JSC.ConcurrentTask) void {
10121016
this.eventLoop().enqueueTaskConcurrent(task);
10131017
}
@@ -1046,6 +1050,8 @@ pub const VirtualMachine = struct {
10461050
if (!this.has_enabled_macro_mode) {
10471051
this.has_enabled_macro_mode = true;
10481052
this.macro_event_loop.tasks = EventLoop.Queue.init(default_allocator);
1053+
this.macro_event_loop.immediate_tasks = EventLoop.Queue.init(default_allocator);
1054+
this.macro_event_loop.next_immediate_tasks = EventLoop.Queue.init(default_allocator);
10491055
this.macro_event_loop.tasks.ensureTotalCapacity(16) catch unreachable;
10501056
this.macro_event_loop.global = this.global;
10511057
this.macro_event_loop.virtual_machine = this;
@@ -1137,6 +1143,12 @@ pub const VirtualMachine = struct {
11371143
vm.regular_event_loop.tasks = EventLoop.Queue.init(
11381144
default_allocator,
11391145
);
1146+
vm.regular_event_loop.immediate_tasks = EventLoop.Queue.init(
1147+
default_allocator,
1148+
);
1149+
vm.regular_event_loop.next_immediate_tasks = EventLoop.Queue.init(
1150+
default_allocator,
1151+
);
11401152
vm.regular_event_loop.tasks.ensureUnusedCapacity(64) catch unreachable;
11411153
vm.regular_event_loop.concurrent_tasks = .{};
11421154
vm.event_loop = &vm.regular_event_loop;
@@ -1240,6 +1252,12 @@ pub const VirtualMachine = struct {
12401252
vm.regular_event_loop.tasks = EventLoop.Queue.init(
12411253
default_allocator,
12421254
);
1255+
vm.regular_event_loop.immediate_tasks = EventLoop.Queue.init(
1256+
default_allocator,
1257+
);
1258+
vm.regular_event_loop.next_immediate_tasks = EventLoop.Queue.init(
1259+
default_allocator,
1260+
);
12431261
vm.regular_event_loop.tasks.ensureUnusedCapacity(64) catch unreachable;
12441262
vm.regular_event_loop.concurrent_tasks = .{};
12451263
vm.event_loop = &vm.regular_event_loop;
@@ -1372,6 +1390,12 @@ pub const VirtualMachine = struct {
13721390
vm.regular_event_loop.tasks = EventLoop.Queue.init(
13731391
default_allocator,
13741392
);
1393+
vm.regular_event_loop.immediate_tasks = EventLoop.Queue.init(
1394+
default_allocator,
1395+
);
1396+
vm.regular_event_loop.next_immediate_tasks = EventLoop.Queue.init(
1397+
default_allocator,
1398+
);
13751399
vm.regular_event_loop.tasks.ensureUnusedCapacity(64) catch unreachable;
13761400
vm.regular_event_loop.concurrent_tasks = .{};
13771401
vm.event_loop = &vm.regular_event_loop;

‎src/cli/test_command.zig

+7-2
Original file line numberDiff line numberDiff line change
@@ -976,8 +976,10 @@ pub const TestCommand = struct {
976976
vm.eventLoop().tick();
977977

978978
var prev_unhandled_count = vm.unhandled_error_counter;
979-
while (vm.active_tasks > 0) {
980-
if (!jest.Jest.runner.?.has_pending_tests) jest.Jest.runner.?.drain();
979+
while (vm.active_tasks > 0) : (vm.eventLoop().flushImmediateQueue()) {
980+
if (!jest.Jest.runner.?.has_pending_tests) {
981+
jest.Jest.runner.?.drain();
982+
}
981983
vm.eventLoop().tick();
982984

983985
while (jest.Jest.runner.?.has_pending_tests) {
@@ -991,6 +993,9 @@ pub const TestCommand = struct {
991993
prev_unhandled_count = vm.unhandled_error_counter;
992994
}
993995
}
996+
997+
vm.eventLoop().flushImmediateQueue();
998+
994999
switch (vm.aggressive_garbage_collection) {
9951000
.none => {},
9961001
.mild => {

‎test/harness.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export async function expectMaxObjectTypeCount(
4848
gc(true);
4949
for (const wait = 20; maxWait > 0; maxWait -= wait) {
5050
if (heapStats().objectTypeCounts[type] <= count) break;
51-
await new Promise(resolve => setTimeout(resolve, wait));
51+
await Bun.sleep(wait);
5252
gc();
5353
}
5454
expect(heapStats().objectTypeCounts[type]).toBeLessThanOrEqual(count);
@@ -60,7 +60,7 @@ export function gcTick(trace = false) {
6060
trace && console.trace("");
6161
// console.trace("hello");
6262
gc();
63-
return new Promise(resolve => setTimeout(resolve, 0));
63+
return Bun.sleep(0);
6464
}
6565

6666
export function withoutAggressiveGC(block: () => unknown) {

‎test/js/web/timers/setImmediate.test.ts

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { test, expect } from "bun:test";
2+
3+
test("setImmediate doesn't block the event loop", async () => {
4+
const incomingTimestamps = [];
5+
var hasResponded = false;
6+
var expectedTime = "";
7+
const server = Bun.serve({
8+
port: 0,
9+
async fetch(req) {
10+
await new Promise(resolve => setTimeout(resolve, 50).unref());
11+
function queuey() {
12+
incomingTimestamps.push(Date.now());
13+
if (!hasResponded) setImmediate(queuey);
14+
}
15+
setImmediate(queuey);
16+
return new Response((expectedTime = Date.now().toString(10)));
17+
},
18+
});
19+
20+
const resp = await fetch(`http://${server.hostname}:${server.port}/`);
21+
expect(await resp.text()).toBe(expectedTime);
22+
hasResponded = true;
23+
server.stop(true);
24+
});

‎test/js/web/timers/setTimeout.test.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,20 @@ it("clearTimeout", async () => {
6767
expect(called).toBe(false);
6868
});
6969

70+
it("setImmediate runs after setTimeout cb", async () => {
71+
var ranFirst = -1;
72+
setTimeout(() => {
73+
if (ranFirst === -1) ranFirst = 1;
74+
}, 0);
75+
setImmediate(() => {
76+
if (ranFirst === -1) ranFirst = 0;
77+
});
78+
79+
await Bun.sleep(5);
80+
81+
expect(ranFirst).toBe(1);
82+
});
83+
7084
it("setTimeout(() => {}, 0)", async () => {
7185
var called = false;
7286
setTimeout(() => {
@@ -80,10 +94,10 @@ it("setTimeout(() => {}, 0)", async () => {
8094
expect(called).toBe(true);
8195
var ranFirst = -1;
8296
setTimeout(() => {
83-
if (ranFirst === -1) ranFirst = 1;
97+
if (ranFirst === -1) ranFirst = 0;
8498
}, 1);
8599
setTimeout(() => {
86-
if (ranFirst === -1) ranFirst = 0;
100+
if (ranFirst === -1) ranFirst = 1;
87101
}, 0);
88102

89103
await new Promise((resolve, reject) => {

0 commit comments

Comments
 (0)