Conversation
onAddPendingWork takes the keep-alive of an ImminentlyScheduled ticket (WebAssembly.compile, a FinalizationRegistry cleanup) on the event loop that is current, and records that loop in the ticket's PendingWork. Every release queued the -1 on the regular loop. When the ticket was registered while a macro ran, the +1 was on the macro loop. A macro VM on a bundler or transpiler thread never ticks its regular loop, so the -1 was never applied and the thread's native loop stayed active. Release on the recorded loop at all four release sites.
|
Status: ready for review. CI is green on 1860c3b (build 118219). How I reproduced it. A macro reads Relation to #40769. With #40769 alone, |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. WalkthroughThe task scheduler now tracks the loop associated with each keep-alive ticket and uses it during release. Macro tests cover awaited ChangesLoop-specific keep-alive release
Suggested reviewers: Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/jsc/bindings/JSCTaskScheduler.cpp— A macro on a transpiler pool thread that starts WebAssembly.compile() without awaiting it leaves that thread's native loop active until the next macro runs on the same thread; if none does, forever, exactly as the PR says it fixes. Line 76 folds the +1 into the native loop at once via Bun__eventLoop__refKeepAlive. The completion is posted to the macro loop, which on a pool VM only ticks inside a later macro; the -1 at line 132 then waits there too. Fix: after a macro returns on a VM whose regular loop never ticks, drain the macro loop's adopted work and fold its delta, or document that un-awaited macro work is unsupported on pool VMs.Extended reasoning...
lib.ts (a non-entry import) is transpiled on a RuntimeTranspilerStore pool thread whose VM runs macros. The macro calls compile() and returns 0 without awaiting. onAddPendingWork line 63 records Macro; line 76 ref_keep_alive (event_loop.rs:1179) folds +1 into the shared native loop. Later the JSC helper thread finishes and onScheduleWorkSoon line 107 posts the job to the macro loop. macro_loop_if_not_running (event_loop.rs:688) only helps the regular loop, and a pool VM's regular loop never ticks. So the job and its -1 (line 132) run only if another macro on this thread ticks the macro loop. The dismissal says the base failed the same way; on the base the -1 targeted Regular and was stranded forever, here it is stranded until the next macro, so the exposure differs and the PR text claims balance. Population: every un-awaited macro wasm compile on pool threads; the residue is visible to getEventLoopStats and is_event_loop_alive on that VM. Remedy: have the macro runner tick the macro loop once more after the macro returns when the VM has no ticking regular loop.
Verification: pre-existing — triggered when a macro running on a pool-thread VM (RuntimeTranspilerStore transpiling a non-entry import, or a Bun.build() worker) calls WebAssembly.compile() without awaiting it and no later async macro runs on that same thread. Mechanism verified: /home/claude/bun/src/jsc/bindings/JSCTaskScheduler.cpp:63 records
Bun__VM__currentLoopKind(Macro while the macro runs, since…
|
On the finding about work that a macro starts and does not await on a bundler or transpiler thread: the description is correct, and this PR does not change that behavior. I did not change the code for it. The PR notes now state it as a known limit.
|
|
Updated 1:43 AM PT - Sep 19th, 2026
✅ @robobun, your commit 1860c3b3afa287db537420173b4c46e24035dee1 passed in 🧪 To try this PR locally: bunx bun-pr 43415That installs a local version of the PR into your bun-43415 --bun |
Problem
WebAssembly.compile(), the VM's native event loop stays active.getEventLoopStats().numPollsstays 1 higher, forever on a bundler or transpiler thread.onAddPendingWork(src/jsc/bindings/JSCTaskScheduler.cpp:71) takes the keep-alive on the current loop, here the macro loop. The four release sites (lines 92, 114, 126, 188) queue the -1 on the regular loop. Such a VM never ticks that loop. Fix hang when a macro awaits crypto.subtle.digest #39905 introduced this.Fix
onAddPendingWorkrecorded in the ticket'sPendingWork.macro_loop_if_not_running).Ticket::unref_keep_alivefollows the same rule.test/bundler/transpiler/macro-test.test.ts, three new cases. Main fails all three withleaked 1. Alsotest/regression/issue/39900.test.ts,atomics.test.ts,wasm-streaming.test.ts.Background
vm.event_looppoints at the macro loop while a macro runs. Both use one native loop (uSockets or libuv).Bun__VmHandle__refKeepAlivequeues it on one loop, which applies it on its next tick.DeferredWorkTimeris the JSC hook for work such as a wasm compile. Bun keeps the loop alive from the registration of the work until its job runs.Notes
Origin: a review bot on macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769 found this by a read of the code. There is no user report. On main, the only effect is the count that
getEventLoopStats()reads. Build results, exit codes and output do not change. Fix Bun.spawnSync + GC unbalancing the event loop's keep-alive count #40508 fixed another keep-alive imbalance from the same change in Fix hang when a macro awaits crypto.subtle.digest #39905 (a GC insideBun.spawnSync).Repro on main, run with
bun build.ts.bunEnvfrom the test harness makesbun:internal-for-testingavailable.Main prints
before = 1,after = 2. This branch prints 1 and 1.The new cases run the same sequence in three VMs: the main VM (entry file), the VM of a transpiler thread (imported module), and the VM of a
Bun.build()worker. Each prints thenumPollsdelta andBun.isMainThread, so a case cannot pass on the wrong VM. All three fail on main, on Linux and on Windows, and pass on this branch on both.CI ran the file with this commit. I read the logs of build 118219 for macOS arm64, macOS x64, Windows x64, Windows arm64 and Linux x64 ASAN: 30 pass, 0 fail on each.
Check with macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769: I merged this change into that PR branch on a Windows debug build. The repro from that PR (
export const a = compiles(); export const b = never();) hangs with macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769 alone. With both changes,bun build index.tsexits 1 withmacro returned a promise that never settles. This removes one known limit of macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769. One more limit stays, see Bun.build: FinalizationRegistry cleanup scheduled by the worker-teardown GC never runs, and its keep-alive stays on the worker thread's event loop #43458 below.The un-awaited case in the main VM stays balanced.
macro-test.test.ts"a WebAssembly.compile() inside the macro that it does not await still lets the process exit" passes. The regular loop adopts the job of the macro loop after the macro returns, and its nexttick_concurrent_with_countapplies the -1 that the job queued on the macro loop.Known limit that this PR does not change: on a bundler or transpiler thread, nothing ticks the macro loop after a macro returns. Work that a macro starts and does not await runs only when a later macro on that thread waits on a promise, and its keep-alive stays until then. Before this PR it stayed forever. The idle check of macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769 still works in that case. It runs after a tick of the macro loop, and that tick runs the job and applies the release. If the job is not posted yet, the wait continues until it arrives. Checked on the same Windows debug build with both changes: a macro that starts a compile and does not await it, then
never(), exits 1 with the same error in 5 of 5 runs.Known limit that this PR does not change, tracked in Bun.build: FinalizationRegistry cleanup scheduled by the worker-teardown GC never runs, and its keep-alive stays on the worker thread's event loop #43458: a ticket that is registered while no macro runs records
Regular. Its job and its release go to the regular loop, as before. That is correct for the main VM and for aWorker, which tick their regular loop. A macro VM on a bundler or transpiler thread never ticks it, so the job never runs and the keep-alive stays. The GC at the end of eachBun.build()(collect_macro_vm_garbage) registers such a ticket when a macro left deadFinalizationRegistryregistrations. With macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769 and this PR, a later build on that thread still hangs on a macro whose promise never settles. The repro is in Bun.build: FinalizationRegistry cleanup scheduled by the worker-teardown GC never runs, and its keep-alive stays on the worker thread's event loop #43458.Optional hardening that is not in this PR: worker teardown (phase D of
VirtualMachine::teardown) applies the queued delta of the current loop only. A -1 that a macro ticket queues on the macro loop during teardown stays inconcurrent_ref. The loop is freed next, anduv_loop_closeandus_loop_freedo not read the count.Ticket::unref_keep_aliveon a macro ticket has the same property today. Phase D could apply the delta of both loops. It has no effect that a test can see, so I kept this PR to one concern.Rejected from the self-review: a per-test timeout for the new cases. They took 4.7 to 5.0 s on a debug ASAN build under heavy machine load, against the 5 s default.
test/CLAUDE.mddoes not allow per-test timeouts, and the CI runner sets its own. On an idle machine the same build runs each case in 1.4 to 1.9 s.Related open PRs. Let JSC's DeferredWorkTimer own the deferred-work tickets so a collection cancels a dead realm's work (WebKit bump) #39994 rewrites this file for a WebKit change and also releases on the recorded loop from other threads. It has conflicts and waits on a WebKit PR. macros: run every macro on one dedicated VM thread #40059 moves all macros to one dedicated VM thread. It removes the macro loop and
BunLoopKind, so it would replace this change and the VM expectations of the new cases. It has conflicts, and its last commit is from 2026-08-25. Reach the VM of a deferred-work job through its client data, not the ticket's realm #43085 and Fix O(N^2) worker teardown with pending Atomics.waitAsync waiters #37280 touch the same functions and need a small rebase against this change.Local runs that are not related to this diff:
spawnsync-isolated-event-loop.test.ts(twocollectContinuouslycases) and oneworker-late-completioncase go over the 5 s default timeout on my debug ASAN build. The fixtures printOKwith this change when I run them directly, and theworker-late-completioncase also times out without this change.