fix: BackpressureMonitor.Dispose() no longer deadlocks on single-threaded targets#5330
Open
jamescrosswell wants to merge 7 commits into
Open
fix: BackpressureMonitor.Dispose() no longer deadlocks on single-threaded targets#5330jamescrosswell wants to merge 7 commits into
jamescrosswell wants to merge 7 commits into
Conversation
…aded targets On single-threaded runtimes (Unity WebGL / browser-wasm) there is no separate thread pool: Task.Run and Task.Delay continuations all run on the one main thread. BackpressureMonitor.Dispose() called _cts.Cancel() followed by a synchronous _workerTask.Wait(), which blocked the only thread that could run the worker's cancellation continuation - a deadlock. On Unity WebGL this wedged unityInstance.Quit() indefinitely (#5237). Dispose() now only requests cancellation and returns; the worker observes the token and unwinds on its own. The monitor's public methods (DownsampleFactor / RecordQueueOverflow / RecordRateLimitHit) never touch the cancellation token source, so it is now safe for Hub.Dispose() to dispose the monitor again rather than leaking the worker for the lifetime of the process. Adds a regression test that runs the worker on a scheduler that never completes and asserts Dispose() returns without blocking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5330 +/- ##
==========================================
+ Coverage 74.16% 74.20% +0.04%
==========================================
Files 508 508
Lines 18353 18363 +10
Branches 3586 3589 +3
==========================================
+ Hits 13612 13627 +15
+ Misses 3869 3864 -5
Partials 872 872 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jamescrosswell
commented
Jun 29, 2026
jamescrosswell
commented
Jun 29, 2026
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
…rker stops Removing the blocking _workerTask.Wait() from Dispose() (to avoid the single-threaded deadlock) reintroduced a race: the inline _cts.Dispose() could run while the worker was still inside Task.Delay(..., token) registering its continuation. On platforms where registering on a disposed source throws (e.g. .NET Framework) the worker would observe an ObjectDisposedException - which it doesn't catch - and fault with an unobserved task exception. Dispose the CancellationTokenSource from a non-blocking continuation on the worker task instead, so it is only disposed once the worker has stopped using the token. Adds a test asserting the worker runs to completion without faulting after Dispose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…adlock' into fix/5237-backpressure-dispose-deadlock
jamescrosswell
commented
Jun 30, 2026
A second Dispose() call would hit the already-disposed _cts and log a spurious ObjectDisposedException warning (and schedule redundant cleanup). Guard with a thread-safe Interlocked flag so only the first caller runs the disposal logic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…etsentry/sentry-dotnet into fix/5237-backpressure-dispose-deadlock
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
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.
Fixes #5237
Problem
On single-threaded runtimes (Unity WebGL / browser-wasm) there is no separate thread pool —
Task.RunandTask.Delaycontinuations all run on the one main thread.BackpressureMonitor.Dispose()did:_workerTask.Wait()synchronously blocks the calling thread waiting for the worker's post-cancellation continuation, which on a single-threaded runtime can only be scheduled on that same blocked thread (e.g. callingunityInstance.Quit()in Unity WebGL).ConfigureAwait(false)does not help here. On a desktop host the cancellation continuation escapes to a real thread-pool thread. On single-threaded targets the "thread pool" is the main thread, so the continuation has nowhere to run except the thread that is blocked inWait().Fix
Dispose()now only requests cancellation and returns — it does not block on the worker. The worker observes the token, exits itsTask.Delayloop and unwinds on its own; it produces no result anyone needs to await.Because the monitor's public methods (
DownsampleFactor/RecordQueueOverflow/RecordRateLimitHit) never touch_cts, it is now safe to dispose the monitor even while the client keeps draining envelopes. This letsHub.Dispose()dispose the monitor again rather than leaking the worker for the process lifetime.Hub.Dispose()The call to
_backpressureMonitor?.Dispose()was removed in #4613 with the comment:I've restored it because, with a non-blocking
Dispose()that only cancels/disposes the CTS, none of the monitor's post-disposal call sites touch the disposed token source — so the originalObjectDisposedExceptionshould not recur.Warning
TODO: Confirm this matches the scenario #4613 was guarding against. We can always reinstate the fix from #4613 if we're concerned about this.
Affected versions
The live deadlock path (
Hub.Dispose()→_backpressureMonitor?.Dispose()) exists in 4.x/5.x (and the Sentry Unity 4.x that reported this). It was incidentally removed onmain/6.0.0+ by #4613, so 6.x no longer reaches the deadlock - but at the cost of leaking the worker.Dispose()itself remained latently broken. This PR fixes the root cause so the call is safe again. Users on ≤5.x operating in a single threaded environment should setEnableBackpressureHandling = falseas a workaround instead.🤖 Generated with Claude Code