fix(turbo-tasks): eliminate ABBA deadlock in the compilation event queue - #96939
Draft
marcoshernanz wants to merge 1 commit into
Draft
fix(turbo-tasks): eliminate ABBA deadlock in the compilation event queue#96939marcoshernanz wants to merge 1 commit into
marcoshernanz wants to merge 1 commit into
Conversation
send() held event_history.lock() for the whole task, then acquired DashMap shard write guards and awaited channel sends, while subscribe(None) acquired the Global shard write guard and then awaited event_history.lock() plus replayed history while still holding the shard guard. Lock orders: send = history -> shard, subscribe = shard -> history — a two-worker interleave deadlocks (the parking_lot shard wait is a synchronous non-yielding block), and every subsequent send then blocks another worker thread, progressively stalling the runtime. Restructure around one serialization point (the history lock) with no lock ever held across an await: - subscribe: [lock] register the sender + replay history with try_send [unlock] — synchronous because the fresh channel's capacity (MAX_QUEUE_SIZE) is always >= the history's capacity, so it can never block. - send: [lock] append to history + snapshot the subscriber lists [unlock], then deliver guard-free. That yields exactly-once at subscribe boundaries (an event is in the replay snapshot xor in delivery snapshots) and strict ordering (replays fully enqueue before any later live delivery), while removing the deadlock and the global stall a full/slow subscriber channel used to cause. Delivery is concurrent across subscribers (join_all), and failed subscribers are removed afterwards under fresh guards, matched by channel identity so concurrent additions are kept.
Contributor
Tests PassedCommit: 18e0fa4 |
Contributor
Stats from current PR🟢 2 improvements
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (29 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diffapp-route-ex..ntime.dev.jsDiff too large to display app-route-ex..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route.runtime.dev.jsDiff too large to display app-route.ru..time.prod.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display pages-api-tu..ntime.dev.jsDiff too large to display pages-api-tu..time.prod.jsDiff too large to display pages-api.runtime.dev.jsDiff too large to display pages-api.ru..time.prod.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display pages-turbo...time.prod.jsDiff too large to display pages.runtime.dev.jsDiff too large to display pages.runtime.prod.jsDiff too large to display server.runtime.prod.jsDiff too large to display 📎 Tarball URLCommit: 18e0fa4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
CompilationEventQueue(turbo-tasks message_queue.rs — the fan-out for compilation/timing/trace/diagnostic events consumed by the napi layer,backgroundLogCompilationEvents/projectCompilationEventsSubscribe) had an ABBA deadlock:send()spawned a task that acquiredevent_history.lock()and held it for the task's whole life, then acquired DashMap shard write guards (get_mut) and awaited channel sends while holding both.subscribe(None)acquired the Global shard write guard viaentry().or_default(), then awaitedevent_history.lock()and replayed history while still holding the shard guard.Lock orders: send = history → shard, subscribe = shard → history. A two-worker interleave deadlocks (the parking_lot shard wait is a synchronous, non-yielding block), and every subsequent
sendthen blocks another worker thread — progressively stalling the whole runtime (next dev/next buildhangs with no error). Secondarily, one slow/full subscriber channel (capacity 256) stalled all history recording and global delivery.Fix
Restructure around one serialization point (the history lock), with no lock ever held across an
.await:[lock]register the sender + replay history withtry_send[unlock]— synchronous, because a fresh subscriber channel's capacity (MAX_QUEUE_SIZE= 256) is always ≥ the history's capacity (also 256), so it can never block.[lock]append to history + snapshot the subscriber lists[unlock], then deliver guard-free.Consequences, beyond eliminating the deadlock:
join_all), and failed subscribers are removed afterwards under freshly-taken guards, matched bysame_channelidentity so concurrently-added subscribers are kept.Tests
subscribe(None)replay + live send, 10s collect deadline) with duplicate detection after the expected count.TimingEventtests pass;cargo check/clippy/fmtgreen.Honest caveat: the ABBA deadlock is timing-dependent and my stress runs did not reproduce it on the old code (the lock-order inversion is proven statically from the lock acquisition order, not empirically reproduced).
Note: commits on this branch are currently unsigned; happy to re-sign and force-push if needed before review.