From 574ff328f548a7f6dfd831eadc3dc3ae7f474849 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 29 Apr 2025 23:34:38 -0700 Subject: [PATCH] update --- src/bun.js/WTFTimer.zig | 140 ++++++++ src/bun.js/api/Timer.zig | 305 +++++++++++------- src/bun.js/bindings/BunObject.cpp | 2 +- src/bun.js/bindings/NodeTimerObject.cpp | 64 +--- src/bun.js/bindings/headers.h | 5 +- src/bun.js/bindings/node/NodeTimers.cpp | 15 +- src/bun.js/event_loop.zig | 14 +- src/bun.js/jsc/host_fn.zig | 84 ++--- .../test-timers-immediate-queue-throw.js | 56 ++++ .../test-timers-timeout-to-interval.js | 12 + 10 files changed, 462 insertions(+), 235 deletions(-) create mode 100644 src/bun.js/WTFTimer.zig create mode 100644 test/js/node/test/parallel/test-timers-immediate-queue-throw.js create mode 100644 test/js/node/test/parallel/test-timers-timeout-to-interval.js diff --git a/src/bun.js/WTFTimer.zig b/src/bun.js/WTFTimer.zig new file mode 100644 index 0000000000..613efb2e1d --- /dev/null +++ b/src/bun.js/WTFTimer.zig @@ -0,0 +1,140 @@ +const std = @import("std"); +const bun = @import("bun"); +const jsc = bun.jsc; +const VirtualMachine = jsc.VirtualMachine; +const EventLoopTimer = bun.api.Timer.EventLoopTimer; + +/// This is WTF::RunLoop::TimerBase from WebKit +const RunLoopTimer = opaque { + pub fn fire(this: *RunLoopTimer) void { + WTFTimer__fire(this); + } +}; + +/// A timer created by WTF code and invoked by Bun's event loop +const WTFTimer = @This(); + +vm: *VirtualMachine, +run_loop_timer: *RunLoopTimer, +event_loop_timer: EventLoopTimer, +imminent: *std.atomic.Value(?*WTFTimer), +repeat: bool, +lock: bun.Mutex = .{}, + +pub const new = bun.TrivialNew(WTFTimer); + +pub fn init(run_loop_timer: *RunLoopTimer, js_vm: *VirtualMachine) *WTFTimer { + const this = WTFTimer.new(.{ + .vm = js_vm, + .imminent = &js_vm.eventLoop().imminent_gc_timer, + .event_loop_timer = .{ + .next = .{ + .sec = std.math.maxInt(i64), + .nsec = 0, + }, + .tag = .WTFTimer, + .state = .CANCELLED, + }, + .run_loop_timer = run_loop_timer, + .repeat = false, + }); + + return this; +} + +pub export fn WTFTimer__runIfImminent(vm: *VirtualMachine) void { + vm.eventLoop().runImminentGCTimer(); +} + +pub fn run(this: *WTFTimer, vm: *VirtualMachine) void { + if (this.event_loop_timer.state == .ACTIVE) { + vm.timer.remove(&this.event_loop_timer); + } + this.runWithoutRemoving(); +} + +inline fn runWithoutRemoving(this: *const WTFTimer) void { + this.run_loop_timer.fire(); +} + +pub fn update(this: *WTFTimer, seconds: f64, repeat: bool) void { + // There's only one of these per VM, and each VM has its own imminent_gc_timer + this.imminent.store(if (seconds == 0) this else null, .seq_cst); + + if (seconds == 0.0) { + return; + } + + const modf = std.math.modf(seconds); + var interval = bun.timespec.now(); + interval.sec += @intFromFloat(modf.ipart); + interval.nsec += @intFromFloat(modf.fpart * std.time.ns_per_s); + if (interval.nsec >= std.time.ns_per_s) { + interval.sec += 1; + interval.nsec -= std.time.ns_per_s; + } + + this.vm.timer.update(&this.event_loop_timer, &interval); + this.repeat = repeat; +} + +pub fn cancel(this: *WTFTimer) void { + this.lock.lock(); + defer this.lock.unlock(); + this.imminent.store(null, .seq_cst); + if (this.event_loop_timer.state == .ACTIVE) { + this.vm.timer.remove(&this.event_loop_timer); + } +} + +pub fn fire(this: *WTFTimer, _: *const bun.timespec, _: *VirtualMachine) EventLoopTimer.Arm { + this.event_loop_timer.state = .FIRED; + this.imminent.store(null, .seq_cst); + this.runWithoutRemoving(); + return if (this.repeat) + .{ .rearm = this.event_loop_timer.next } + else + .disarm; +} + +pub fn deinit(this: *WTFTimer) void { + this.cancel(); + bun.destroy(this); +} + +export fn WTFTimer__create(run_loop_timer: *RunLoopTimer) ?*anyopaque { + if (VirtualMachine.is_bundler_thread_for_bytecode_cache) { + return null; + } + + return init(run_loop_timer, VirtualMachine.get()); +} + +export fn WTFTimer__update(this: *WTFTimer, seconds: f64, repeat: bool) void { + this.update(seconds, repeat); +} + +export fn WTFTimer__deinit(this: *WTFTimer) void { + this.deinit(); +} + +export fn WTFTimer__isActive(this: *const WTFTimer) bool { + return this.event_loop_timer.state == .ACTIVE or (this.imminent.load(.seq_cst) orelse return false) == this; +} + +export fn WTFTimer__cancel(this: *WTFTimer) void { + this.cancel(); +} + +export fn WTFTimer__secondsUntilTimer(this: *WTFTimer) f64 { + this.lock.lock(); + defer this.lock.unlock(); + if (this.event_loop_timer.state == .ACTIVE) { + const until = this.event_loop_timer.next.duration(&bun.timespec.now()); + const sec: f64, const nsec: f64 = .{ @floatFromInt(until.sec), @floatFromInt(until.nsec) }; + return sec + nsec / std.time.ns_per_s; + } + return std.math.inf(f64); +} + +extern fn WTFTimer__fire(this: *RunLoopTimer) void; diff --git a/src/bun.js/api/Timer.zig b/src/bun.js/api/Timer.zig index 2c3457d79a..451b7be5a5 100644 --- a/src/bun.js/api/Timer.zig +++ b/src/bun.js/api/Timer.zig @@ -299,47 +299,6 @@ pub const All = struct { setImmediate, }; - fn set( - id: i32, - globalThis: *JSGlobalObject, - callback: JSValue, - request: SetRequest, - arguments_array_or_zero: JSValue, - ) JSC.JSValue { - JSC.markBinding(@src()); - var vm = globalThis.bunVM(); - const kind: Kind = request; - - const js = switch (request) { - .setImmediate => ImmediateObject.init(globalThis, id, callback, arguments_array_or_zero), - .setTimeout, .setInterval => |countdown| TimeoutObject.init(globalThis, id, kind, countdown, callback, arguments_array_or_zero), - }; - - if (vm.isInspectorEnabled()) { - Debugger.didScheduleAsyncCall( - globalThis, - .DOMTimer, - ID.asyncID(.{ .id = id, .kind = kind.big() }), - kind != .setInterval, // single_shot - ); - } - return js; - } - - pub fn setImmediate( - globalThis: *JSGlobalObject, - callback: JSValue, - arguments: JSValue, - ) callconv(.c) JSValue { - JSC.markBinding(@src()); - const id = globalThis.bunVM().timer.last_id; - globalThis.bunVM().timer.last_id +%= 1; - - const wrappedCallback = callback.withAsyncContextIfNeeded(globalThis); - - return set(id, globalThis, wrappedCallback, .setImmediate, arguments); - } - const TimeoutWarning = enum { TimeoutOverflowWarning, TimeoutNegativeWarning, @@ -420,38 +379,70 @@ pub const All = struct { return countdown_int; } - pub fn setTimeout( - globalThis: *JSGlobalObject, - callback: JSValue, + // Bun.sleep + // a setTimeout that uses a promise instead of a callback + pub fn sleep( + global: *JSGlobalObject, + promise: JSValue, countdown: JSValue, - arguments: JSValue, - overflow_behavior: CountdownOverflowBehavior, ) JSError!JSValue { JSC.markBinding(@src()); - const id = globalThis.bunVM().timer.last_id; - globalThis.bunVM().timer.last_id +%= 1; + bun.debugAssert(promise != .zero and countdown != .zero); + const vm = global.bunVM(); + const id = vm.timer.last_id; + vm.timer.last_id +%= 1; - const countdown_int = try globalThis.bunVM().timer.jsValueToCountdown(globalThis, countdown, overflow_behavior, true); + const countdown_int = try vm.timer.jsValueToCountdown(global, countdown, .clamp, true); + const wrapped_promise = promise.withAsyncContextIfNeeded(global); + return TimeoutObject.init(global, id, .setTimeout, countdown_int, wrapped_promise, .undefined); + } - const wrappedCallback = callback.withAsyncContextIfNeeded(globalThis); + pub fn setImmediate( + global: *JSGlobalObject, + callback: JSValue, + arguments: JSValue, + ) JSError!JSValue { + JSC.markBinding(@src()); + bun.debugAssert(callback != .zero and arguments != .zero); + const vm = global.bunVM(); + const id = vm.timer.last_id; + vm.timer.last_id +%= 1; - return set(id, globalThis, wrappedCallback, .{ .setTimeout = countdown_int }, arguments); + const wrapped_callback = callback.withAsyncContextIfNeeded(global); + return ImmediateObject.init(global, id, wrapped_callback, arguments); + } + + pub fn setTimeout( + global: *JSGlobalObject, + callback: JSValue, + arguments: JSValue, + countdown: JSValue, + ) JSError!JSValue { + JSC.markBinding(@src()); + bun.debugAssert(callback != .zero and arguments != .zero and countdown != .zero); + const vm = global.bunVM(); + const id = vm.timer.last_id; + vm.timer.last_id +%= 1; + + const wrapped_callback = callback.withAsyncContextIfNeeded(global); + const countdown_int = try global.bunVM().timer.jsValueToCountdown(global, countdown, .one_ms, true); + return TimeoutObject.init(global, id, .setTimeout, countdown_int, wrapped_callback, arguments); } pub fn setInterval( - globalThis: *JSGlobalObject, + global: *JSGlobalObject, callback: JSValue, - countdown: JSValue, arguments: JSValue, + countdown: JSValue, ) JSError!JSValue { JSC.markBinding(@src()); - const id = globalThis.bunVM().timer.last_id; - globalThis.bunVM().timer.last_id +%= 1; + bun.debugAssert(callback != .zero and arguments != .zero and countdown != .zero); + const vm = global.bunVM(); + const id = vm.timer.last_id; + vm.timer.last_id +%= 1; - const wrappedCallback = callback.withAsyncContextIfNeeded(globalThis); - - const countdown_int = try globalThis.bunVM().timer.jsValueToCountdown(globalThis, countdown, .one_ms, true); - - return set(id, globalThis, wrappedCallback, .{ .setInterval = countdown_int }, arguments); + const wrapped_callback = callback.withAsyncContextIfNeeded(global); + const countdown_int = try global.bunVM().timer.jsValueToCountdown(global, countdown, .one_ms, true); + return TimeoutObject.init(global, id, .setInterval, countdown_int, wrapped_callback, arguments); } fn removeTimerById(this: *All, id: i32) ?*TimeoutObject { @@ -550,8 +541,9 @@ pub const All = struct { } comptime { - @export(&setImmediate, .{ .name = "Bun__Timer__setImmediate" }); - @export(&JSC.host_fn.wrap5(setTimeout), .{ .name = "Bun__Timer__setTimeout" }); + @export(&JSC.host_fn.wrap3(setImmediate), .{ .name = "Bun__Timer__setImmediate" }); + @export(&JSC.host_fn.wrap3(sleep), .{ .name = "Bun__Timer__sleep" }); + @export(&JSC.host_fn.wrap4(setTimeout), .{ .name = "Bun__Timer__setTimeout" }); @export(&JSC.host_fn.wrap4(setInterval), .{ .name = "Bun__Timer__setInterval" }); @export(&JSC.host_fn.wrap2(clearImmediate), .{ .name = "Bun__Timer__clearImmediate" }); @export(&JSC.host_fn.wrap2(clearTimeout), .{ .name = "Bun__Timer__clearTimeout" }); @@ -585,7 +577,7 @@ pub const TimeoutObject = struct { kind: Kind, interval: u31, callback: JSValue, - arguments_array_or_zero: JSValue, + arguments: JSValue, ) JSValue { // internals are initialized by init() const timeout = bun.new(TimeoutObject, .{ .ref_count = .init(), .internals = undefined }); @@ -598,8 +590,18 @@ pub const TimeoutObject = struct { kind, interval, callback, - arguments_array_or_zero, + arguments, ); + + if (globalThis.bunVM().isInspectorEnabled()) { + Debugger.didScheduleAsyncCall( + globalThis, + .DOMTimer, + ID.asyncID(.{ .id = id, .kind = kind.big() }), + kind != .setInterval, + ); + } + return js_value; } @@ -655,40 +657,36 @@ pub const TimeoutObject = struct { return TimeoutObject.js.callbackGetCached(thisValue).?; } - pub fn set_onTimeout(this: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) void { + pub fn set_onTimeout(_: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) void { TimeoutObject.js.callbackSetCached(thisValue, globalThis, value); - this.internals.flags.should_destroy_before_firing = !value.toBoolean(); } pub fn get_idleTimeout(_: *TimeoutObject, thisValue: JSValue, _: *JSGlobalObject) JSValue { return TimeoutObject.js.idleTimeoutGetCached(thisValue).?; } - pub fn set_idleTimeout(this: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) JSError!void { + pub fn set_idleTimeout(_: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) void { TimeoutObject.js.idleTimeoutSetCached(thisValue, globalThis, value); - if (value.isNumber()) { - const num = try value.toNumber(globalThis); + // if (value.isNumber()) { + // const num = try value.toNumber(globalThis); - // cancel if the value is exactly -1 - // https://github.com/nodejs/node/blob/4cd8e1914a503ece778d642e748020e675cf1060/lib/internal/timers.js#L612-L625 - this.internals.flags.should_reschedule_interval = num != -1; - } else { - this.internals.flags.should_reschedule_interval = true; - } + // // cancel if the value is exactly -1 + // // https://github.com/nodejs/node/blob/4cd8e1914a503ece778d642e748020e675cf1060/lib/internal/timers.js#L612-L625 + // this.internals.flags.should_reschedule_interval = num != -1; + // } else { + // this.internals.flags.should_reschedule_interval = true; + // } - this.internals.interval = try globalThis.bunVM().timer.jsValueToCountdown(globalThis, value, .one_ms, false); + // this.internals.interval = try globalThis.bunVM().timer.jsValueToCountdown(globalThis, value, .one_ms, false); } pub fn get_repeat(_: *TimeoutObject, thisValue: JSValue, _: *JSGlobalObject) JSValue { return TimeoutObject.js.repeatGetCached(thisValue).?; } - pub fn set_repeat(_: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) JSError!void { + pub fn set_repeat(_: *TimeoutObject, thisValue: JSValue, globalThis: *JSGlobalObject, value: JSValue) void { TimeoutObject.js.repeatSetCached(thisValue, globalThis, value); - - const num = try value.toNumber(globalThis); - _ = num; } pub fn dispose(this: *TimeoutObject, globalThis: *JSGlobalObject, _: *JSC.CallFrame) bun.JSError!JSValue { @@ -718,7 +716,7 @@ pub const ImmediateObject = struct { globalThis: *JSGlobalObject, id: i32, callback: JSValue, - arguments_array_or_zero: JSValue, + arguments: JSValue, ) JSValue { // internals are initialized by init() const immediate = bun.new(ImmediateObject, .{ .ref_count = .init(), .internals = undefined }); @@ -731,8 +729,18 @@ pub const ImmediateObject = struct { .setImmediate, 0, callback, - arguments_array_or_zero, + arguments, ); + + if (globalThis.bunVM().isInspectorEnabled()) { + Debugger.didScheduleAsyncCall( + globalThis, + .DOMTimer, + ID.asyncID(.{ .id = id, .kind = .setImmediate }), + true, + ); + } + return js_value; } @@ -789,7 +797,7 @@ pub const TimerObjectInternals = struct { strong_this: JSC.Strong.Optional = .empty, flags: Flags = .{}, - const Flags = packed struct(u34) { + const Flags = packed struct(u33) { /// Whenever a timer is inserted into the heap (which happen on creation or refresh), the global /// epoch is incremented and the new epoch is set on the timer. For timers created by /// JavaScript, the epoch is used to break ties between timers scheduled for the same @@ -816,10 +824,6 @@ pub const TimerObjectInternals = struct { // is set `false` when `_idleTimeout` is set to -1 // https://github.com/nodejs/node/blob/4cd8e1914a503ece778d642e748020e675cf1060/lib/internal/timers.js#L612-L626 should_reschedule_interval: bool = true, - - // is set `true` when `_onTimeout` is set to a falsy value - // https://github.com/nodejs/node/blob/4cd8e1914a503ece778d642e748020e675cf1060/lib/internal/timers.js#L578-L592 - should_destroy_before_firing: bool = false, }; fn eventLoopTimer(this: *TimerObjectInternals) *EventLoopTimer { @@ -851,7 +855,7 @@ pub const TimerObjectInternals = struct { } } - extern "c" fn Bun__JSTimeout__call(encodedTimeoutValue: JSValue, globalObject: *JSC.JSGlobalObject) void; + extern "c" fn Bun__JSTimeout__call(globalObject: *JSC.JSGlobalObject, timer: JSValue, callback: JSValue, arguments: JSValue) void; pub fn runImmediateTask(this: *TimerObjectInternals, vm: *VirtualMachine) void { if (this.flags.has_cleared_timer or @@ -863,7 +867,7 @@ pub const TimerObjectInternals = struct { return; } - const this_object = this.strong_this.get() orelse { + const timer = this.strong_this.get() orelse { if (Environment.isDebug) { @panic("TimerObjectInternals.runImmediateTask: this_object is null"); } @@ -879,7 +883,9 @@ pub const TimerObjectInternals = struct { this.ref(); defer this.deref(); - this.run(this_object, globalThis, this.asyncID(), vm); + const callback = ImmediateObject.js.callbackGetCached(timer).?; + const arguments = ImmediateObject.js.argumentsGetCached(timer).?; + this.run(globalThis, timer, callback, arguments, this.asyncID(), vm); if (this.eventLoopTimer().state == .FIRED) { this.deref(); @@ -895,17 +901,35 @@ pub const TimerObjectInternals = struct { pub fn fire(this: *TimerObjectInternals, _: *const timespec, vm: *JSC.VirtualMachine) EventLoopTimer.Arm { const id = this.id; const kind = this.flags.kind.big(); + const async_id: ID = .{ .id = id, .kind = kind }; const has_been_cleared = this.eventLoopTimer().state == .CANCELLED or this.flags.has_cleared_timer or vm.scriptExecutionStatus() != .running; this.eventLoopTimer().state = .FIRED; const globalThis = vm.global; + const this_object = this.strong_this.get().?; - if (has_been_cleared or this.flags.should_destroy_before_firing) { + const callback, const arguments, var idle_timeout, var repeat = switch (kind) { + .setImmediate => .{ + ImmediateObject.js.callbackGetCached(this_object).?, + ImmediateObject.js.argumentsGetCached(this_object).?, + + .undefined, + .undefined, + }, + .setTimeout, .setInterval => .{ + TimeoutObject.js.callbackGetCached(this_object).?, + TimeoutObject.js.argumentsGetCached(this_object).?, + TimeoutObject.js.idleTimeoutGetCached(this_object).?, + TimeoutObject.js.repeatGetCached(this_object).?, + }, + }; + + if (has_been_cleared or !callback.toBoolean()) { if (vm.isInspectorEnabled()) { - Debugger.didCancelAsyncCall(globalThis, .DOMTimer, ID.asyncID(.{ .id = id, .kind = kind })); + Debugger.didCancelAsyncCall(globalThis, .DOMTimer, ID.asyncID(async_id)); } - + this.setEnableKeepingEventLoopAlive(vm, false); this.flags.has_cleared_timer = true; this.strong_this.deinit(); this.deref(); @@ -913,7 +937,6 @@ pub const TimerObjectInternals = struct { return .disarm; } - const this_object = this.strong_this.get().?; var time_before_call: timespec = undefined; if (kind != .setInterval) { @@ -929,12 +952,20 @@ pub const TimerObjectInternals = struct { this.ref(); defer this.deref(); - this.run(this_object, globalThis, ID.asyncID(.{ .id = id, .kind = kind }), vm); + this.run(globalThis, this_object, callback, arguments, ID.asyncID(async_id), vm); + + switch (kind) { + .setTimeout, .setInterval => { + idle_timeout = TimeoutObject.js.idleTimeoutGetCached(this_object).?; + repeat = TimeoutObject.js.repeatGetCached(this_object).?; + }, + else => {}, + } const is_timer_done = is_timer_done: { // Node doesn't drain microtasks after each timer callback. if (kind == .setInterval) { - if (!this.flags.should_reschedule_interval) { + if (!this.shouldRescheduleTimer(repeat, idle_timeout)) { break :is_timer_done true; } switch (this.eventLoopTimer().state) { @@ -960,8 +991,19 @@ pub const TimerObjectInternals = struct { break :is_timer_done true; }, } - } else if (this.eventLoopTimer().state == .FIRED) { - break :is_timer_done true; + } else { + if (kind == .setTimeout and !repeat.isNull()) { + if (idle_timeout.getNumber()) |num| { + if (num != -1) { + this.toInterval(globalThis, this_object, repeat); + break :is_timer_done false; + } + } + } + + if (this.eventLoopTimer().state == .FIRED) { + break :is_timer_done true; + } } break :is_timer_done false; @@ -978,7 +1020,22 @@ pub const TimerObjectInternals = struct { return .disarm; } - pub fn run(this: *TimerObjectInternals, this_object: JSC.JSValue, globalThis: *JSC.JSGlobalObject, async_id: u64, vm: *JSC.VirtualMachine) void { + fn toInterval(this: *TimerObjectInternals, global: *JSGlobalObject, timer: JSValue, repeat: JSValue) void { + bun.debugAssert(this.flags.kind == .setTimeout); + + const vm = global.bunVM(); + + const new_interval: u31 = if (repeat.getNumber()) |num| if (num < 1 or num > std.math.maxInt(u31)) 1 else @intFromFloat(num) else 1; + + // https://github.com/nodejs/node/blob/a7cbb904745591c9a9d047a364c2c188e5470047/lib/internal/timers.js#L613 + TimeoutObject.js.idleTimeoutSetCached(timer, global, repeat); + this.strong_this.set(global, timer); + this.flags.kind = .setInterval; + this.interval = new_interval; + this.reschedule(timer, vm); + } + + pub fn run(this: *TimerObjectInternals, globalThis: *JSC.JSGlobalObject, timer: JSValue, callback: JSValue, arguments: JSValue, async_id: u64, vm: *JSC.VirtualMachine) void { if (vm.isInspectorEnabled()) { Debugger.willDispatchAsyncCall(globalThis, .DOMTimer, async_id); } @@ -992,20 +1049,20 @@ pub const TimerObjectInternals = struct { // Bun__JSTimeout__call handles exceptions. this.flags.in_callback = true; defer this.flags.in_callback = false; - Bun__JSTimeout__call(this_object, globalThis); + Bun__JSTimeout__call(globalThis, timer, callback, arguments); } pub fn init( this: *TimerObjectInternals, - timer_js: JSValue, - globalThis: *JSGlobalObject, + timer: JSValue, + global: *JSGlobalObject, id: i32, kind: Kind, interval: u31, callback: JSValue, arguments: JSValue, ) void { - const vm = globalThis.bunVM(); + const vm = global.bunVM(); this.* = .{ .id = id, .flags = .{ .kind = kind, .epoch = vm.timer.epoch }, @@ -1013,24 +1070,24 @@ pub const TimerObjectInternals = struct { }; if (kind == .setImmediate) { - if (arguments != .zero) - ImmediateObject.js.argumentsSetCached(timer_js, globalThis, arguments); - ImmediateObject.js.callbackSetCached(timer_js, globalThis, callback); + ImmediateObject.js.argumentsSetCached(timer, global, arguments); + ImmediateObject.js.callbackSetCached(timer, global, callback); const parent: *ImmediateObject = @fieldParentPtr("internals", this); vm.enqueueImmediateTask(parent); this.setEnableKeepingEventLoopAlive(vm, true); // ref'd by event loop parent.ref(); } else { - if (arguments != .zero) - TimeoutObject.js.argumentsSetCached(timer_js, globalThis, arguments); - TimeoutObject.js.callbackSetCached(timer_js, globalThis, callback); - TimeoutObject.js.idleTimeoutSetCached(timer_js, globalThis, JSC.jsNumber(interval)); + TimeoutObject.js.argumentsSetCached(timer, global, arguments); + TimeoutObject.js.callbackSetCached(timer, global, callback); + TimeoutObject.js.idleTimeoutSetCached(timer, global, JSC.jsNumber(interval)); + TimeoutObject.js.repeatSetCached(timer, global, if (kind == .setInterval) JSC.jsNumber(interval) else .null); + // this increments the refcount - this.reschedule(vm); + this.reschedule(timer, vm); } - this.strong_this.set(globalThis, timer_js); + this.strong_this.set(global, timer); } pub fn doRef(this: *TimerObjectInternals, _: *JSC.JSGlobalObject, this_value: JSValue) JSValue { @@ -1039,7 +1096,10 @@ pub const TimerObjectInternals = struct { const did_have_js_ref = this.flags.has_js_ref; this.flags.has_js_ref = true; - if (!did_have_js_ref) { + // https://github.com/nodejs/node/blob/a7cbb904745591c9a9d047a364c2c188e5470047/lib/internal/timers.js#L256 + // and + // https://github.com/nodejs/node/blob/a7cbb904745591c9a9d047a364c2c188e5470047/lib/internal/timers.js#L685-L687 + if (!did_have_js_ref and !this.flags.has_cleared_timer) { this.setEnableKeepingEventLoopAlive(JSC.VirtualMachine.get(), true); } @@ -1058,7 +1118,7 @@ pub const TimerObjectInternals = struct { } this.strong_this.set(globalObject, this_value); - this.reschedule(VirtualMachine.get()); + this.reschedule(this_value, VirtualMachine.get()); return this_value; } @@ -1093,9 +1153,22 @@ pub const TimerObjectInternals = struct { } } - pub fn reschedule(this: *TimerObjectInternals, vm: *VirtualMachine) void { + fn shouldRescheduleTimer(this: *TimerObjectInternals, repeat: JSValue, idle_timeout: JSValue) bool { + if (this.flags.kind == .setInterval and repeat.isNull()) return false; + if (idle_timeout.getNumber()) |num| { + if (num == -1) return false; + } + return true; + } + + pub fn reschedule(this: *TimerObjectInternals, timer: JSValue, vm: *VirtualMachine) void { if (this.flags.kind == .setImmediate) return; - if (!this.flags.should_reschedule_interval) return; + + const idle_timeout = TimeoutObject.js.idleTimeoutGetCached(timer).?; + const repeat = TimeoutObject.js.repeatGetCached(timer).?; + + // https://github.com/nodejs/node/blob/a7cbb904745591c9a9d047a364c2c188e5470047/lib/internal/timers.js#L612 + if (!this.shouldRescheduleTimer(repeat, idle_timeout)) return; const now = timespec.msFromNow(this.interval); const was_active = this.eventLoopTimer().state == .ACTIVE; diff --git a/src/bun.js/bindings/BunObject.cpp b/src/bun.js/bindings/BunObject.cpp index 59bd982702..af9a78fcfd 100644 --- a/src/bun.js/bindings/BunObject.cpp +++ b/src/bun.js/bindings/BunObject.cpp @@ -455,7 +455,7 @@ JSC_DEFINE_HOST_FUNCTION(functionBunSleep, } JSC::JSPromise* promise = JSC::JSPromise::create(vm, globalObject->promiseStructure()); - Bun__Timer__setTimeout(globalObject, JSValue::encode(promise), JSC::JSValue::encode(millisecondsValue), {}, Bun::CountdownOverflowBehavior::Clamp); + Bun__Timer__sleep(globalObject, JSValue::encode(promise), JSC::JSValue::encode(millisecondsValue)); return JSC::JSValue::encode(promise); } diff --git a/src/bun.js/bindings/NodeTimerObject.cpp b/src/bun.js/bindings/NodeTimerObject.cpp index 5b41011b73..300e526392 100644 --- a/src/bun.js/bindings/NodeTimerObject.cpp +++ b/src/bun.js/bindings/NodeTimerObject.cpp @@ -16,68 +16,45 @@ namespace Bun { using namespace JSC; -template -void callInternal(T* timeout, JSGlobalObject* globalObject) +static void call(JSGlobalObject* globalObject, JSValue timerObject, JSValue callbackValue, JSValue argumentsValue) { - static_assert(std::is_same_v || std::is_same_v, - "wrong type passed to callInternal"); - auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); - JSValue callbackValue = timeout->m_callback.get(); - JSCell* callbackCell = callbackValue.isCell() ? callbackValue.asCell() : nullptr; - if (!callbackCell) { - Bun__reportUnhandledError(globalObject, JSValue::encode(createNotAFunctionError(globalObject, callbackValue))); - return; - } - JSValue restoreAsyncContext {}; JSC::InternalFieldTuple* asyncContextData = nullptr; - if (auto* wrapper = jsDynamicCast(callbackCell)) { - callbackCell = wrapper->callback.get().asCell(); + if (auto* wrapper = jsDynamicCast(callbackValue)) { + callbackValue = wrapper->callback.get(); asyncContextData = globalObject->m_asyncContextData.get(); restoreAsyncContext = asyncContextData->getInternalField(0); asyncContextData->putInternalField(vm, 0, wrapper->context.get()); } - switch (callbackCell->type()) { - case JSC::JSPromiseType: { + if (auto* promise = jsDynamicCast(callbackValue)) { // This was a Bun.sleep() call - auto promise = jsCast(callbackCell); promise->resolve(globalObject, jsUndefined()); - break; - } - - default: { - auto callData = JSC::getCallData(callbackCell); + } else { + auto callData = JSC::getCallData(callbackValue); if (callData.type == CallData::Type::None) { Bun__reportUnhandledError(globalObject, JSValue::encode(createNotAFunctionError(globalObject, callbackValue))); return; } MarkedArgumentBuffer args; - if (timeout->m_arguments) { - JSValue argumentsValue = timeout->m_arguments.get(); - auto* butterfly = jsDynamicCast(argumentsValue); - + if (auto* butterfly = jsDynamicCast(argumentsValue)) { // If it's a JSImmutableButterfly, there is more than 1 argument. - if (butterfly) { - unsigned length = butterfly->length(); - args.ensureCapacity(length); - for (unsigned i = 0; i < length; ++i) { - args.append(butterfly->get(i)); - } - } else { - // Otherwise, it's a single argument. - args.append(argumentsValue); + unsigned length = butterfly->length(); + args.ensureCapacity(length); + for (unsigned i = 0; i < length; ++i) { + args.append(butterfly->get(i)); } + } else if (!argumentsValue.isUndefined()) { + // Otherwise, it's a single argument. + args.append(argumentsValue); } - JSC::profiledCall(globalObject, ProfilingReason::API, JSValue(callbackCell), JSC::getCallData(callbackCell), timeout, ArgList(args)); - break; - } + JSC::profiledCall(globalObject, ProfilingReason::API, callbackValue, callData, timerObject, args); } if (UNLIKELY(scope.exception())) { @@ -91,21 +68,14 @@ void callInternal(T* timeout, JSGlobalObject* globalObject) } } -extern "C" void Bun__JSTimeout__call(JSC::EncodedJSValue encodedTimeoutValue, JSC::JSGlobalObject* globalObject) +extern "C" void Bun__JSTimeout__call(JSGlobalObject* globalObject, EncodedJSValue timerObject, EncodedJSValue callbackValue, EncodedJSValue argumentsValue) { auto& vm = globalObject->vm(); if (UNLIKELY(vm.hasPendingTerminationException())) { return; } - JSValue timeoutValue = JSValue::decode(encodedTimeoutValue); - if (auto* timeout = jsDynamicCast(timeoutValue)) { - return callInternal(timeout, globalObject); - } else if (auto* immediate = jsDynamicCast(timeoutValue)) { - return callInternal(immediate, globalObject); - } - - ASSERT_NOT_REACHED_WITH_MESSAGE("Object passed to Bun__JSTimeout__call is not a JSTimeout or a JSImmediate"); + return call(globalObject, JSValue::decode(timerObject), JSValue::decode(callbackValue), JSValue::decode(argumentsValue)); } } diff --git a/src/bun.js/bindings/headers.h b/src/bun.js/bindings/headers.h index 9af6a67850..0714731d76 100644 --- a/src/bun.js/bindings/headers.h +++ b/src/bun.js/bindings/headers.h @@ -675,7 +675,7 @@ ZIG_DECL JSC::EncodedJSValue Bun__Timer__clearImmediate(JSC::JSGlobalObject* arg ZIG_DECL JSC::EncodedJSValue Bun__Timer__clearInterval(JSC::JSGlobalObject* arg0, JSC::EncodedJSValue JSValue1); ZIG_DECL JSC::EncodedJSValue Bun__Timer__clearTimeout(JSC::JSGlobalObject* arg0, JSC::EncodedJSValue JSValue1); ZIG_DECL int32_t Bun__Timer__getNextID(); -ZIG_DECL JSC::EncodedJSValue Bun__Timer__setInterval(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue callback, JSC::EncodedJSValue countdown, JSC::EncodedJSValue arguments); +ZIG_DECL JSC::EncodedJSValue Bun__Timer__setInterval(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue callback, JSC::EncodedJSValue arguments, JSC::EncodedJSValue countdown); namespace Bun { enum class CountdownOverflowBehavior : uint8_t { // If the countdown overflows the range of int32_t, use a countdown of 1ms instead. Behavior of `setTimeout` and friends. @@ -684,7 +684,8 @@ enum class CountdownOverflowBehavior : uint8_t { Clamp, }; } // namespace Bun -ZIG_DECL JSC::EncodedJSValue Bun__Timer__setTimeout(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue callback, JSC::EncodedJSValue countdown, JSC::EncodedJSValue arguments, Bun::CountdownOverflowBehavior behavior); +ZIG_DECL JSC::EncodedJSValue Bun__Timer__setTimeout(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue callback, JSC::EncodedJSValue arguments, JSC::EncodedJSValue countdown); +ZIG_DECL JSC::EncodedJSValue Bun__Timer__sleep(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue promise, JSC::EncodedJSValue countdown); ZIG_DECL JSC::EncodedJSValue Bun__Timer__setImmediate(JSC::JSGlobalObject* globalThis, JSC::EncodedJSValue callback, JSC::EncodedJSValue arguments); #endif diff --git a/src/bun.js/bindings/node/NodeTimers.cpp b/src/bun.js/bindings/node/NodeTimers.cpp index a2b62431f2..2bd0218a7b 100644 --- a/src/bun.js/bindings/node/NodeTimers.cpp +++ b/src/bun.js/bindings/node/NodeTimers.cpp @@ -13,7 +13,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetTimeout, auto& vm = JSC::getVM(globalObject); JSC::JSValue job = callFrame->argument(0); JSC::JSValue num = callFrame->argument(1); - JSC::JSValue arguments = {}; + JSC::JSValue arguments = jsUndefined(); size_t argumentCount = callFrame->argumentCount(); auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); switch (argumentCount) { @@ -59,7 +59,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetTimeout, } #endif - return Bun__Timer__setTimeout(globalObject, JSC::JSValue::encode(job), JSC::JSValue::encode(num), JSValue::encode(arguments), Bun::CountdownOverflowBehavior::OneMs); + return Bun__Timer__setTimeout(globalObject, JSC::JSValue::encode(job), JSC::JSValue::encode(arguments), JSValue::encode(num)); } JSC_DEFINE_HOST_FUNCTION(functionSetInterval, @@ -68,7 +68,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetInterval, auto& vm = JSC::getVM(globalObject); JSC::JSValue job = callFrame->argument(0); JSC::JSValue num = callFrame->argument(1); - JSC::JSValue arguments = {}; + JSC::JSValue arguments = jsUndefined(); size_t argumentCount = callFrame->argumentCount(); auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); @@ -77,10 +77,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetInterval, Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_ARG_TYPE, "setInterval requires 1 argument (a function)"_s); return {}; } - case 1: { - num = jsNumber(0); - break; - } + case 1: case 2: { break; } @@ -118,7 +115,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetInterval, } #endif - return Bun__Timer__setInterval(globalObject, JSC::JSValue::encode(job), JSC::JSValue::encode(num), JSValue::encode(arguments)); + return Bun__Timer__setInterval(globalObject, JSC::JSValue::encode(job), JSValue::encode(num), JSC::JSValue::encode(arguments)); } // https://developer.mozilla.org/en-US/docs/Web/API/Window/setImmediate @@ -141,7 +138,7 @@ JSC_DEFINE_HOST_FUNCTION(functionSetImmediate, return {}; } - JSC::JSValue arguments = {}; + JSC::JSValue arguments = jsUndefined(); switch (argCount) { case 0: case 1: { diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index b8c87c27b7..5ed707b3a8 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -1404,17 +1404,19 @@ pub const EventLoop = struct { } pub fn tickImmediateTasks(this: *EventLoop, virtual_machine: *VirtualMachine) void { - var global = this.global; - const global_vm = global.vm(); - var to_run_now = this.immediate_tasks; this.immediate_tasks = this.next_immediate_tasks; this.next_immediate_tasks = .{}; - for (to_run_now.items) |task| { - task.runImmediateTask(virtual_machine); - this.drainMicrotasksWithGlobal(global, global_vm); + if (to_run_now.items.len > 0) { + this.enter(); + + for (to_run_now.items) |task| { + task.runImmediateTask(virtual_machine); + } + + this.exit(); } if (this.next_immediate_tasks.capacity > 0) { diff --git a/src/bun.js/jsc/host_fn.zig b/src/bun.js/jsc/host_fn.zig index a4c90a579b..aedb7f867a 100644 --- a/src/bun.js/jsc/host_fn.zig +++ b/src/bun.js/jsc/host_fn.zig @@ -35,18 +35,14 @@ pub fn toJSHostFn(comptime functionToWrap: JSHostFnZig) JSHostFn { pub fn toJSHostFnWithContext(comptime ContextType: type, comptime Function: JSHostFnZigWithContext(ContextType)) JSHostFunctionTypeWithContext(ContextType) { return struct { pub fn function(ctx: *ContextType, globalThis: *JSGlobalObject, callframe: *CallFrame) callconv(jsc.conv) JSValue { - if (Environment.allow_assert and Environment.is_canary) { - const value = Function(ctx, globalThis, callframe) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => globalThis.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(globalThis, value, Function); - return value; - } - return @call(.always_inline, Function, .{ ctx, globalThis, callframe }) catch |err| switch (err) { + const value = Function(ctx, globalThis, callframe) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => globalThis.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(globalThis, value, Function); + } + return value; } }.function; } @@ -123,18 +119,14 @@ pub fn wrap1(comptime func: anytype) @"return": { const p = @typeInfo(@TypeOf(func)).@"fn".params; return struct { pub fn wrapped(arg0: p[0].type.?) callconv(.c) JSValue { - if (Environment.allow_assert and Environment.isDebug) { - const value = func(arg0) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => arg0.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(arg0, value, func); - return value; - } - return @call(.always_inline, func, .{arg0}) catch |err| switch (err) { + const value = func(arg0) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => arg0.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(arg0, value, func); + } + return value; } }.wrapped; } @@ -146,18 +138,14 @@ pub fn wrap2(comptime func: anytype) @"return": { const p = @typeInfo(@TypeOf(func)).@"fn".params; return struct { pub fn wrapped(arg0: p[0].type.?, arg1: p[1].type.?) callconv(.c) JSValue { - if (Environment.allow_assert and Environment.isDebug) { - const value = func(arg0, arg1) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => arg0.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(arg0, value, func); - return value; - } - return @call(.always_inline, func, .{ arg0, arg1 }) catch |err| switch (err) { + const value = func(arg0, arg1) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => arg0.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(arg0, value, func); + } + return value; } }.wrapped; } @@ -169,18 +157,14 @@ pub fn wrap3(comptime func: anytype) @"return": { const p = @typeInfo(@TypeOf(func)).@"fn".params; return struct { pub fn wrapped(arg0: p[0].type.?, arg1: p[1].type.?, arg2: p[2].type.?) callconv(.c) JSValue { - if (Environment.allow_assert and Environment.isDebug) { - const value = func(arg0, arg1, arg2) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => arg0.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(arg0, value, func); - return value; - } - return @call(.always_inline, func, .{ arg0, arg1, arg2 }) catch |err| switch (err) { + const value = func(arg0, arg1, arg2) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => arg0.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(arg0, value, func); + } + return value; } }.wrapped; } @@ -192,18 +176,14 @@ pub fn wrap4(comptime func: anytype) @"return": { const p = @typeInfo(@TypeOf(func)).@"fn".params; return struct { pub fn wrapped(arg0: p[0].type.?, arg1: p[1].type.?, arg2: p[2].type.?, arg3: p[3].type.?) callconv(.c) JSValue { - if (Environment.allow_assert and Environment.isDebug) { - const value = func(arg0, arg1, arg2, arg3) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => arg0.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(arg0, value, func); - return value; - } - return @call(.always_inline, func, .{ arg0, arg1, arg2, arg3 }) catch |err| switch (err) { + const value = func(arg0, arg1, arg2, arg3) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => arg0.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(arg0, value, func); + } + return value; } }.wrapped; } @@ -215,18 +195,14 @@ pub fn wrap5(comptime func: anytype) @"return": { const p = @typeInfo(@TypeOf(func)).@"fn".params; return struct { pub fn wrapped(arg0: p[0].type.?, arg1: p[1].type.?, arg2: p[2].type.?, arg3: p[3].type.?, arg4: p[4].type.?) callconv(.c) JSValue { - if (Environment.allow_assert and Environment.isDebug) { - const value = func(arg0, arg1, arg2, arg3, arg4) catch |err| switch (err) { - error.JSError => .zero, - error.OutOfMemory => arg0.throwOutOfMemoryValue(), - }; - debugExceptionAssertion(arg0, value, func); - return value; - } - return @call(.always_inline, func, .{ arg0, arg1, arg2, arg3, arg4 }) catch |err| switch (err) { + const value = func(arg0, arg1, arg2, arg3, arg4) catch |err| switch (err) { error.JSError => .zero, error.OutOfMemory => arg0.throwOutOfMemoryValue(), }; + if (Environment.allow_assert and Environment.is_canary) { + debugExceptionAssertion(arg0, value, func); + } + return value; } }.wrapped; } diff --git a/test/js/node/test/parallel/test-timers-immediate-queue-throw.js b/test/js/node/test/parallel/test-timers-immediate-queue-throw.js new file mode 100644 index 0000000000..cb16575006 --- /dev/null +++ b/test/js/node/test/parallel/test-timers-immediate-queue-throw.js @@ -0,0 +1,56 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); + +// setImmediate should run clear its queued cbs once per event loop turn +// but immediates queued while processing the current queue should happen +// on the next turn of the event loop. + +// In addition, if any setImmediate throws, the rest of the queue should +// be processed after all error handling is resolved, but that queue +// should not include any setImmediate calls scheduled after the +// processing of the queue started. + +let threw = false; +let stage = -1; + +const QUEUE = 10; + +const errObj = { + name: 'Error', + message: 'setImmediate Err' +}; + +process.once('uncaughtException', common.mustCall((err, errorOrigin) => { + assert.strictEqual(errorOrigin, 'uncaughtException'); + assert.strictEqual(stage, 0); + common.expectsError(errObj)(err); +})); + +// const d1 = domain.create(); +// d1.once('error', common.expectsError(errObj)); +// d1.once('error', () => assert.strictEqual(stage, 0)); + +const run = common.mustCall((callStage) => { + assert(callStage >= stage); + stage = callStage; + if (threw) + return; + + setImmediate(run, 2); +}, QUEUE * 3); + +for (let i = 0; i < QUEUE; i++) + setImmediate(run, 0); +setImmediate(() => { + threw = true; + process.nextTick(() => assert.strictEqual(stage, 1)); + throw new Error('setImmediate Err'); +}); +// d1.run(() => setImmediate(() => { +// throw new Error('setImmediate Err'); +// })); +for (let i = 0; i < QUEUE; i++) + setImmediate(run, 1); diff --git a/test/js/node/test/parallel/test-timers-timeout-to-interval.js b/test/js/node/test/parallel/test-timers-timeout-to-interval.js new file mode 100644 index 0000000000..6b83e2e09e --- /dev/null +++ b/test/js/node/test/parallel/test-timers-timeout-to-interval.js @@ -0,0 +1,12 @@ +'use strict'; +const common = require('../common'); + +// This isn't officially supported but nonetheless is something that is +// currently possible and as such it shouldn't cause the process to crash + +const t = setTimeout(common.mustCall(() => { + if (t._repeat) { + clearInterval(t); + } + t._repeat = 1; +}, 2), 1);