Exit unsettled top-level await instead of hanging / busy-looping

src/jsc/VirtualMachine.zig

@@ -720,7 +720,33 @@
     var promise = this.pending_internal_promise orelse return;
 
     switch (promise.status()) {
-        .pending => return,
+        .pending => {
+            // A prior `reload()` deferred because a ref'd await was in
+            // flight, and the await has now resolved into another
+            // pending state that the loop can no longer make progress
+            // on (e.g. `await Bun.sleep(N).then(() => new Promise(() =>
+            // {}))`). In that case `loadEntryPoint`'s watcher loop has
+            // already bailed out with the promise still `.pending` and
+            // no work to settle it — `hot_reload_deferred` would remain
+            // stranded and the deferred save silently dropped. Consume
+            // the flag here and let `reload()` re-evaluate (it'll
+            // observe the abandoned state and proceed). Guarded on the
+            // same "nothing ref'd but forever_timer" check `reload()`
+            // itself uses so we don't interrupt live ref'd work.
+            //
+            // Tail-recurse after the reload so the fresh
+            // `pending_internal_promise` is re-read from the caller
+            // rather than the stale local `promise` above. Without
+            // this, a new module body that synchronously rejects
+            // (top-level `throw`, transpile error) would leave the
+            // fresh `.rejected` unreported until the next loop wakeup
+            // — `tickPossiblyForever` blocks in `loop.tick()` first.
+            if (this.hot_reload_deferred and !this.eventLoop().hasAnyHandleWorkIgnoringForeverTimer()) {
+                this.reload(null);
+                return this.reportExceptionInHotReloadedModuleIfNeeded();
+            }
+            return;
+        },
         .rejected => if (this.pending_internal_promise_reported_at != this.hot_reload_counter) {
             this.pending_internal_promise_reported_at = this.hot_reload_counter;
             this.unhandledRejection(this.global, promise.result(this.global.vm()), promise.toJS());
@@ -782,8 +808,49 @@
     if (this.pending_internal_promise) |p| {
         switch (p.status()) {
             .pending => {
-                this.hot_reload_deferred = true;
-                return;
+                // Normally defer. A watcher event arrived while either:
+                //   (a) the C++ module loader / transpile chain is still
+                //       draining on the microtask queue,
+                //   (b) a user ref'd await (timer / network / fs) is
+                //       pending and will settle the promise.
+                // Both cases need the current continuation to run first;
+                // tearing down the module registry now would let the two
+                // chains interleave through one registry, or leave a
+                // zombie continuation firing against the freshly-reset
+                // global.
+                //
+                // The defer is consumed by
+                // `reportExceptionInHotReloadedModuleIfNeeded` once the
+                // promise settles. But `reportException…` early-returns
+                // on `.pending`, so if the promise NEVER settles (an
+                // abandoned top-level await — `await new Promise(() =>
+                // {})` or an unref'd `AbortSignal.timeout`) the deferral
+                // would be permanent and --hot would go silently dead.
+                //
+                // Drain microtasks first to collapse case (a): if a
+                // loader chain microtask flips the status to settled,
+                // one of the other arms handles it. If status is still
+                // `.pending` after the drain, check whether any source
+                // of work could advance it — using the
+                // `hasAnyHandleWorkIgnoringForeverTimer` variant because
+                // the --hot main loop holds a ref'd `forever_timer` on
+                // Windows that would otherwise make plain
+                // `hasAnyHandleWork`/`isActive` permanently true.
+                this.eventLoop().drainMicrotasksWithGlobal(this.global, this.jsc_vm) catch {};
+                switch (p.status()) {
+                    .pending => if (this.eventLoop().hasAnyHandleWorkIgnoringForeverTimer()) {
+                        // case (b): ref'd I/O will settle the promise.
+                        this.hot_reload_deferred = true;
+                        return;
+                    },
+                    // Fell through case (a) during the drain — let the
+                    // matching arm decide.
+                    .rejected => if (this.pending_internal_promise_reported_at != this.hot_reload_counter) {
+                        this.hot_reload_deferred = true;
+                        return;
+                    },
+                    .fulfilled => {},
+                }
             },
             .rejected => if (this.pending_internal_promise_reported_at != this.hot_reload_counter) {
                 this.hot_reload_deferred = true;
@@ -821,6 +888,22 @@
     }
     // reloadEntryPoint() stores into pending_internal_promise on every return path.
     _ = this.reloadEntryPoint(this.main) catch @panic("Failed to reload");
+
+    // `reloadEntryPoint` returns while the fetch/link/evaluate chain is
+    // still queued as JSC microtasks. When this `reload()` is invoked
+    // from inside `tick()` (the common single-save path), the
+    // `HotReloadTask` handler returns → `tick()`'s else-branch drains.
+    // But out-of-`tick()` callers — `reportExceptionInHotReloadedModule
+    // IfNeeded` (the stranded-flag consumer, see field doc on
+    // `hot_reload_deferred`) and the pre-existing `bun.js.zig` dispatch
+    // after an initial-load rejection — would return to the main loop
+    // with the new module body still unexecuted. `tickPossiblyForever`
+    // then blocks in `loop.tick()` before `this.tick()` ever drains the
+    // microtask queue, so the new module doesn't evaluate until the
+    // next loop wakeup (another save, or `forever_timer` firing in up
+    // to 4 minutes). Drain here to guarantee the reload takes effect
+    // before `reload()` returns, covering every call site.
+    this.eventLoop().drainMicrotasksWithGlobal(this.global, this.jsc_vm) catch {};
 }
 
 pub inline fn nodeFS(this: *VirtualMachine) *Node.fs.NodeFS {
@@ -864,6 +947,16 @@
 
 pub fn onBeforeExit(this: *VirtualMachine) void {
     this.exit_handler.dispatchOnBeforeExit();
+    // A `process.on('beforeExit', …)` handler may queue JSC microtasks
+    // (e.g. by resolving a TLA that `waitForPromiseOrLoopExit` bailed
+    // out on). `isEventLoopAlive()` can't see those — it checks handles,
+    // tasks, and refs, not the microtask queue — so without this drain
+    // the inner `while` below would skip and the continuation would be
+    // silently dropped before process exit. The analogous gap for
+    // `unhandledRejection` handlers is covered by the drain at the tail
+    // of `waitForPromiseOrLoopExit` (see event_loop.zig); this is the
+    // same pattern one frame later.
+    this.eventLoop().drainMicrotasksWithGlobal(this.global, this.jsc_vm) catch {};
     var dispatch = false;
     while (true) {
         while (this.isEventLoopAlive()) : (dispatch = true) {
@@ -873,6 +966,7 @@
 
         if (dispatch) {
             this.exit_handler.dispatchOnBeforeExit();
+            this.eventLoop().drainMicrotasksWithGlobal(this.global, this.jsc_vm) catch {};
             dispatch = false;
 
             if (this.isEventLoopAlive()) continue;
@@ -2467,6 +2561,24 @@
                     if (this.pending_internal_promise.?.status() == .pending) {
                         this.eventLoop().autoTick();
                     }
+
+                    // Top-level await with no ref'd handle to resolve it:
+                    // bail so POSIX doesn't burn 100% CPU and Windows doesn't
+                    // hang on `uv_run(NOWAIT)` skipping its loop body. See
+                    // EventLoop.waitForPromiseOrLoopExit for the full rationale
+                    // (and for why this deliberately avoids `isEventLoopAlive`,
+                    // which short-circuits on `unhandled_error_counter != 0`).
+                    if (this.pending_internal_promise.?.status() == .pending and !this.eventLoop().hasAnyHandleWork()) {
+                        // Drain microtasks a user `process.on('unhandledRejection',
+                        // …)` handler may have queued inside `autoTick`'s
+                        // `handleRejectedPromises()` — see waitForPromiseOrLoopExit.
+                        // If the drain runs the continuation that settles the promise,
+                        // re-check and fall through to the loop condition.
+                        this.eventLoop().drainMicrotasksWithGlobal(this.global, this.jsc_vm) catch break;
+                        if (this.pending_internal_promise.?.status() == .pending and !this.eventLoop().hasAnyHandleWork()) {
+                            break;
+                        }
+                    }
                 }
             },
             else => {},
@@ -2477,7 +2589,7 @@
         }
 
         this.eventLoop().performGC();
-        this.waitForPromise(.{ .internal = promise });
+        this.eventLoop().waitForPromiseOrLoopExit(.{ .internal = promise });
     }
 
     return this.pending_internal_promise.?;

src/jsc/event_loop.zig

@@ -575,6 +575,134 @@
     }
 }
 
+/// Whether the event loop has any source of work that could still resolve a
+/// pending promise — active uv/uws handles, queued tasks (main or
+/// concurrent), concurrent refs, or immediates.
+///
+/// This is intentionally NOT `VirtualMachine.isEventLoopAlive()`:
+/// `isEventLoopAlive()` short-circuits on `unhandled_error_counter != 0`
+/// (it is the "should the main event loop keep running" predicate, and a
+/// fatal error stops it). For the TLA wait path we only care about work
+/// that could still wake the loop — a side-path unhandled rejection must
+/// NOT abandon an in-flight fetch whose resolution will complete the TLA.
+///
+/// `concurrent_tasks.isEmpty()` is a single atomic acquire-load; it's safe
+/// from the JS thread without locking. Including it closes a narrow race
+/// where a thread (e.g. the hot-reloader watcher at `hot_reloader.zig:278`)
+/// pushes to the concurrent queue without bumping `concurrent_ref` and the
+/// other liveness signals happen to be zero between the tick drain and the
+/// check.
+pub fn hasAnyHandleWork(this: *EventLoop) bool {
+    const vm = this.virtual_machine;
+    return vm.event_loop_handle.?.isActive() or
+        vm.active_tasks > 0 or
+        this.tasks.count > 0 or
+        this.hasPendingRefs() or
+        !this.concurrent_tasks.isEmpty() or
+        this.immediate_tasks.items.len > 0 or
+        this.next_immediate_tasks.items.len > 0;
+}
+
+/// Like `hasAnyHandleWork`, but ignores the outer `--hot` main-loop's
+/// `forever_timer` when counting active uv handles.
+///
+/// On Windows, `tickPossiblyForever` creates `forever_timer` as a ref'd
+/// `uv_timer_t` specifically so `uv_run(UV_RUN_ONCE)` blocks on file-
+/// watcher wakeups without immediately returning on a dead loop. Because
+/// the handle is ref'd, `uv_loop_alive()` (and therefore
+/// `event_loop_handle.isActive()`) reports the loop as "alive" even when
+/// there's no real work to settle a pending promise. Callers that need
+/// to distinguish "only forever_timer is holding the loop open" from
+/// "a user-held ref'd handle is holding the loop open" should use this
+/// variant. On POSIX, uws timers don't bump the uws `active` counter, so
+/// the forever_timer is invisible to `isActive()` and this is equivalent
+/// to `hasAnyHandleWork`.
+///
+/// Windows parity with `uv_loop_alive()`: also consider
+/// `active_reqs.count`, `pending_reqs_tail`, and `endgame_handles`. A
+/// native addon submitting a raw uv request via `napi_get_uv_event_loop`
+/// bumps `active_reqs.count` without touching `active_handles` or any
+/// Bun-side liveness signal, and we must not drop such work on the floor
+/// (the false-negative direction is unrecoverable: we'd tear down the
+/// module registry while the addon's uv_after_work_cb was still pending).
+pub fn hasAnyHandleWorkIgnoringForeverTimer(this: *EventLoop) bool {
+    if (comptime Environment.isWindows) {
+        const vm = this.virtual_machine;
+        // Subtract the forever_timer's contribution from
+        // `active_handles` — it's always 1 ref'd handle when present.
+        // Note this is a heuristic: another handle closing as we sample
+        // would underreport, but the fall-back on a false positive is
+        // simply "defer the reload once more", which is recoverable.
+        const uv_loop = vm.event_loop_handle.?;
+        const active = uv_loop.active_handles;
+        const forever_timer_contribution: u32 = if (this.forever_timer != null) 1 else 0;
+        const effective_active = if (active > forever_timer_contribution) active - forever_timer_contribution else 0;
+        return effective_active > 0 or
+            uv_loop.active_reqs.count > 0 or
+            uv_loop.pending_reqs_tail != null or
+            uv_loop.endgame_handles != null or
+            vm.active_tasks > 0 or
+            this.tasks.count > 0 or
+            this.hasPendingRefs() or
+            !this.concurrent_tasks.isEmpty() or
+            this.immediate_tasks.items.len > 0 or
+            this.next_immediate_tasks.items.len > 0;
+    }
+    return this.hasAnyHandleWork();
+}
+
+/// Like `waitForPromise`, but returns early when the event loop has nothing
+/// left that could resolve the promise (see `hasAnyHandleWork`). Used by the
+/// top-level-await entry points where the promise may be "unsettled" — e.g.
+/// awaiting an abort event whose only source is an unref'd
+/// `AbortSignal.timeout()` timer.
+///
+/// Without this, POSIX busy-loops at 100% CPU until the unref'd timer fires
+/// and Windows hangs forever (`uv_run(UV_RUN_NOWAIT)` early-returns when
+/// `uv__loop_alive()` is false, so unref'd Bun timers never fire via the uv
+/// scheduler). Matches Node.js, which also exits an unsettled top-level
+/// await without waiting on unref'd handles.
+///
+/// Callers that require a resolved promise on return should keep using
+/// `waitForPromise` — this variant is specifically for the top-level-entry
+/// path, which is prepared to observe a still-pending promise.
+pub fn waitForPromiseOrLoopExit(this: *EventLoop, promise: jsc.AnyPromise) void {
+    const jsc_vm = this.virtual_machine.jsc_vm;
+    switch (promise.status()) {
+        .pending => {
+            while (promise.status() == .pending) {
+                if (jsc_vm.executionForbidden()) {
+                    break;
+                }
+                this.tick();
+
+                if (promise.status() == .pending) {
+                    this.autoTick();
+                }
+
+                if (promise.status() == .pending and !this.hasAnyHandleWork()) {
+                    // About to break because nothing holds the loop alive.
+                    // `autoTick` just called `handleRejectedPromises()`, which
+                    // may have run a user `process.on('unhandledRejection',
+                    // …)` handler that resolved the awaited promise via a
+                    // JSC microtask. In `.bun` mode with a registered
+                    // handler, that path returns WITHOUT a
+                    // `defer drainMicrotasks`, so the continuation hasn't
+                    // run yet and `hasAnyHandleWork` can't see it. Drain
+                    // once and re-check the status — if the microtask
+                    // settled the promise, fall through to the loop
+                    // condition and exit normally instead of breaking.
+                    this.drainMicrotasksWithGlobal(this.global, jsc_vm) catch return;
+                    if (promise.status() == .pending and !this.hasAnyHandleWork()) {
+                        break;
+                    }
+                }
+            }
+        },
+        else => {},
+    }
+}
+
 pub fn waitForPromiseWithTermination(this: *EventLoop, promise: jsc.AnyPromise) void {
     const worker = this.virtual_machine.worker orelse @panic("EventLoop.waitForPromiseWithTermination: worker is not initialized");
     switch (promise.status()) {