diff options
| -rw-r--r-- | src/bun.js/api/bun.zig | 143 | ||||
| -rw-r--r-- | src/deps/uws.zig | 16 | ||||
| -rw-r--r-- | test/bun.js/setTimeout.test.js | 4 |
3 files changed, 103 insertions, 60 deletions
diff --git a/src/bun.js/api/bun.zig b/src/bun.js/api/bun.zig index ad0bcd00c..c173391cf 100644 --- a/src/bun.js/api/bun.zig +++ b/src/bun.js/api/bun.zig @@ -2199,7 +2199,7 @@ pub const TOML = struct { }; pub const Timer = struct { - last_id: i32 = 0, + last_id: i32 = 1, warned: bool = false, active: u32 = 0, timeouts: TimeoutMap = TimeoutMap{}, @@ -2207,7 +2207,7 @@ pub const Timer = struct { const TimeoutMap = std.AutoArrayHashMapUnmanaged(i32, *Timeout); pub fn getNextID() callconv(.C) i32 { - VirtualMachine.vm.timer.last_id += 1; + VirtualMachine.vm.timer.last_id +%= 1; return VirtualMachine.vm.timer.last_id; } @@ -2217,77 +2217,101 @@ pub const Timer = struct { callback: JSC.Strong = .{}, interval: i32 = 0, repeat: bool = false, - cancelled: bool = false, globalThis: *JSC.JSGlobalObject, timer: *uws.Timer, poll_ref: JSC.PollRef = JSC.PollRef.init(), - task_ref: JSC.Ref = JSC.Ref.init(), - // When deleting a timeout that is currently being called, we delay - in_progress: bool = false, - reschedule: bool = false, - task: JSC.AnyTask = undefined, + pub var invalid_timer_ref: *Timeout = undefined; - pub fn ref(this: *Timeout) void { - this.task_ref.ref(this.globalThis.bunVM()); - } + pub fn run(timer: *uws.Timer) callconv(.C) void { + const id: i32 = timer.as(i32); - pub fn unref(this: *Timeout) void { - this.task_ref.unref(this.globalThis.bunVM()); - } + // use the threadlocal despite being slow on macOS + // to handle the timeout being cancelled after already enqueued + var vm = JSC.VirtualMachine.vm; - pub fn run(timer: *uws.Timer) callconv(.C) void { - timer.ext(Timeout).?.then(); - } + var this_entry = vm.timer.timeouts.getEntry( + id, + ) orelse { + // this timer was cancelled after the event loop callback was queued + return; + }; - pub fn perform(this: *Timeout) void { - var vm = this.globalThis.bunVM(); - const callback = this.callback.get() orelse @panic("Expected callback in timer"); - const result = callback.call(this.globalThis, &.{}); + var this: *Timeout = this_entry.value_ptr.*; - if (result.isAnyError(this.globalThis)) { - vm.runErrorHandler(result, null); - } - this.in_progress = false; - if (!this.repeat) this.cancelled = true; + std.debug.assert(this != invalid_timer_ref); - if (this.reschedule) { - this.in_progress = true; - } + var cb: CallbackJob = .{ + .callback = if (this.repeat) + JSC.Strong.create(this.callback.get() orelse return, this.globalThis) + else + this.callback, + .globalThis = this.globalThis, + .id = this.id, + }; - if (this.cancelled and !this.in_progress) this.deinit(); - } + var job = vm.allocator.create(CallbackJob) catch @panic( + "Out of memory while allocating Timeout", + ); - pub fn schedule(this: *Timeout) void { - this.in_progress = true; - this.task = JSC.AnyTask.New(Timeout, perform).init(this); - this.globalThis.bunVM().eventLoop().enqueueTask(JSC.Task.init(&this.task)); - this.reschedule = false; - } + job.* = cb; + job.task = CallbackJob.Task.init(job); + job.ref.ref(vm); - pub fn then(this: *Timeout) void { - if (comptime JSC.is_bindgen) - unreachable; + vm.enqueueTask(JSC.Task.init(&job.task)); - if (!this.cancelled and !this.in_progress) { - this.schedule(); - } else if (!this.cancelled and this.in_progress) { - this.reschedule = true; + // This allows us to: + // - free the memory before the job is run + // - reuse the JSC.Strong + if (!this.repeat) { + this.callback = .{}; + this_entry.value_ptr.* = invalid_timer_ref; + this.deinit(); } - - if (this.cancelled and !this.in_progress) this.deinit(); } + // TODO: reference count to avoid multiple Strong references to the same + // object in setInterval + const CallbackJob = struct { + id: i32 = 0, + task: JSC.AnyTask = undefined, + ref: JSC.Ref = JSC.Ref.init(), + globalThis: *JSC.JSGlobalObject, + callback: JSC.Strong = .{}, + + pub const Task = JSC.AnyTask.New(CallbackJob, perform); + + pub fn perform(this: *CallbackJob) void { + defer { + this.callback.deinit(); + this.ref.unref(this.globalThis.bunVM()); + bun.default_allocator.destroy(this); + } + var vm = this.globalThis.bunVM(); + + if (!vm.timer.timeouts.contains(this.id)) { + // we didn't find the timeout, so it was already cleared + // that means this job shouldn't run. + return; + } + + const callback = this.callback.get() orelse @panic("Expected callback in timer"); + + const result = callback.call(this.globalThis, &.{}); + + if (result.isAnyError(this.globalThis)) { + vm.runErrorHandler(result, null); + } + } + }; + pub fn deinit(this: *Timeout) void { if (comptime JSC.is_bindgen) unreachable; var vm = this.globalThis.bunVM(); - this.cancelled = true; this.poll_ref.unref(vm); - _ = vm.timer.timeouts.swapRemove(this.id); this.timer.deinit(); - this.unref(); this.callback.deinit(); vm.allocator.destroy(this); } @@ -2311,9 +2335,13 @@ pub const Timer = struct { .globalThis = globalThis, .timer = uws.Timer.create(vm.uws_event_loop.?, false, timeout), }; - timeout.timer.set(timeout, Timeout.run, timeout.interval, @as(i32, @boolToInt(repeat)) * timeout.interval); + timeout.timer.set( + id, + Timeout.run, + timeout.interval, + @as(i32, @boolToInt(repeat)) * timeout.interval, + ); timeout.poll_ref.ref(vm); - timeout.ref(); vm.timer.timeouts.put(vm.allocator, id, timeout) catch unreachable; } @@ -2348,12 +2376,15 @@ pub const Timer = struct { pub fn clearTimer(id: JSValue, _: *JSGlobalObject) void { if (comptime is_bindgen) unreachable; - var timer: *Timeout = VirtualMachine.vm.timer.timeouts.get(id.toInt32()) orelse return; - if (timer.in_progress) { - timer.cancelled = true; - } else { - timer.deinit(); + + var timer = VirtualMachine.vm.timer.timeouts.fetchSwapRemove(id.toInt32()) orelse return; + if (timer.value == Timeout.invalid_timer_ref) { + // this timer was scheduled to run but was cancelled before it was run + // so long as the callback isn't already in progress, fetchSwapRemove will handle invalidating it + return; } + + timer.value.deinit(); } pub fn clearTimeout( diff --git a/src/deps/uws.zig b/src/deps/uws.zig index ee44db0d2..4d6fa05e2 100644 --- a/src/deps/uws.zig +++ b/src/deps/uws.zig @@ -261,13 +261,16 @@ pub const SocketTCP = NewSocketHandler(false); pub const SocketTLS = NewSocketHandler(true); pub const Timer = opaque { - pub fn create(loop: *Loop, falltrhough: bool, ptr: ?*anyopaque) *Timer { - return us_create_timer(loop, @as(i32, @boolToInt(falltrhough)), if (ptr != null) 8 else 0); + pub fn create(loop: *Loop, falltrhough: bool, ptr: anytype) *Timer { + const Type = @TypeOf(ptr); + return us_create_timer(loop, @as(i32, @boolToInt(falltrhough)), @sizeOf(Type)); } - pub fn set(this: *Timer, ptr: ?*anyopaque, cb: ?fn (*Timer) callconv(.C) void, ms: i32, repeat_ms: i32) void { + pub fn set(this: *Timer, ptr: anytype, cb: ?fn (*Timer) callconv(.C) void, ms: i32, repeat_ms: i32) void { us_timer_set(this, cb, ms, repeat_ms); - us_timer_ext(this).* = ptr.?; + var value_ptr = us_timer_ext(this); + @setRuntimeSafety(false); + @ptrCast(*@TypeOf(ptr), @alignCast(@alignOf(*@TypeOf(ptr)), value_ptr)).* = ptr; } pub fn deinit(this: *Timer) void { @@ -277,6 +280,11 @@ pub const Timer = opaque { pub fn ext(this: *Timer, comptime Type: type) ?*Type { return @ptrCast(*Type, @alignCast(@alignOf(Type), us_timer_ext(this).*.?)); } + + pub fn as(this: *Timer, comptime Type: type) Type { + @setRuntimeSafety(false); + return @ptrCast(*?Type, @alignCast(@alignOf(Type), us_timer_ext(this))).*.?; + } }; pub const SocketContext = opaque { pub fn getNativeHandle(this: *SocketContext, comptime ssl: bool) *anyopaque { diff --git a/test/bun.js/setTimeout.test.js b/test/bun.js/setTimeout.test.js index 55f71712c..79039a568 100644 --- a/test/bun.js/setTimeout.test.js +++ b/test/bun.js/setTimeout.test.js @@ -30,6 +30,10 @@ it("clearTimeout", async () => { expect(false).toBe(true); }, 1); clearTimeout(id); + + // assert it doesn't crash if you call clearTimeout twice + clearTimeout(id); + await new Promise((resolve, reject) => { setTimeout(() => { resolve(); |