From 617d6033e82f258236feab749941438232e1cd31 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Tue, 22 Apr 2025 14:28:31 -0700 Subject: [PATCH] fix: `globalObject.reload()` should drain nextTick queue --- src/bun.js/bindings/JSGlobalObject.zig | 29 ++++++++++++++++++++++++- src/bun.js/bindings/VM.zig | 5 +++++ src/bun.js/bindings/ZigGlobalObject.cpp | 10 +++++++++ src/bun.js/event_loop.zig | 2 +- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index e293eee25b..ea1dd1b5c7 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -216,9 +216,12 @@ pub const JSGlobalObject = opaque { extern fn JSC__JSGlobalObject__reload(JSC__JSGlobalObject__ptr: *JSGlobalObject) void; pub fn reload(this: *JSC.JSGlobalObject) void { - this.vm().drainMicrotasks(); + this.drainMicrotasks(); this.vm().collectAsync(); + // NOTE(@DonIsaac): This triggers a synchronous GC every other reload. + // sync collections wait for pending async ones to complete. Maybe we + // should move the above collectAsync call into the reload function. JSC__JSGlobalObject__reload(this); } @@ -384,6 +387,30 @@ pub const JSGlobalObject = opaque { ); } + /// Drain the microtask queue and, if active, the nextTick queue. + /// + /// Prefer `EventLoop.drainMicrotasks` over this function, since that also + /// drains deferred tasks. + /// + /// The microtask queue will be drained for as long as there are microtasks + /// in it. This means that if a microtask queues more microtasks, those will + /// be drained as well. + /// + /// The `nextTick` queue only gets activated when a user calls + /// `process.nextTick`. When active, `nextTick` tasks get drained _after_ + /// microtasks. + /// + /// Draining will stop early if a microtask or nextTick task throws a + /// termination exception (e.g. a worker calls `process.exit()` within an + /// event listener run on next tick). + /// + /// ## Safety + /// - Caller thread must hold the JSC API lock + pub fn drainMicrotasks(this: *JSGlobalObject) void { + JSC__JSGlobalObject__drainMicrotasks(this); + } + extern fn JSC__JSGlobalObject__drainMicrotasks(*JSC.JSGlobalObject) void; + extern fn Bun__Process__emitWarning(globalObject: *JSGlobalObject, warning: JSValue, @"type": JSValue, code: JSValue, ctor: JSValue) void; pub fn emitWarning(globalObject: *JSGlobalObject, warning: JSValue, @"type": JSValue, code: JSValue, ctor: JSValue) JSError!void { Bun__Process__emitWarning(globalObject, warning, @"type", code, ctor); diff --git a/src/bun.js/bindings/VM.zig b/src/bun.js/bindings/VM.zig index 3e4e191bef..679cab0dce 100644 --- a/src/bun.js/bindings/VM.zig +++ b/src/bun.js/bindings/VM.zig @@ -167,9 +167,14 @@ pub const VM = opaque { } extern fn JSC__VM__drainMicrotasks(vm: *VM) void; + /// Drain all microtasks from the microtask queue. + /// + /// This does _not_ drain the nextTick queue. Use + /// `JSGlobalObject.drainMicrotasks` for that. pub fn drainMicrotasks( vm: *VM, ) void { + JSC.markMemberBinding("VM", @src()); return JSC__VM__drainMicrotasks(vm); } diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index ec2841f953..60cd7f3e1e 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -3948,6 +3948,16 @@ void GlobalObject::drainMicrotasks() if (auto nextTickQueue = this->m_nextTickQueue.get()) { Bun::JSNextTickQueue* queue = jsCast(nextTickQueue); queue->drain(vm, this); +#if BUN_DEBUG + // microtask queue draining may stop early when a + // termination exception is thrown within a microtask + if (!vm.hasPendingTerminationException()) { + vm.m_microtaskQueues.forEach([](JSC::MicrotaskQueue* queue) { + ASSERT(queue->isEmpty()); + }); + } +#endif + return; } vm.drainMicrotasks(); diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index 306ec37124..0ceb5f8c0e 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -922,7 +922,7 @@ pub const EventLoop = struct { pub fn drainMicrotasksWithGlobal(this: *EventLoop, globalObject: *JSC.JSGlobalObject, _: *JSC.VM) void { JSC.markBinding(@src()); - JSC__JSGlobalObject__drainMicrotasks(globalObject); + globalObject.drainMicrotasks(); this.virtual_machine.is_inside_deferred_task_queue = true; this.deferred_tasks.run();