getsentry / getsentry/sentry-dotnet
BackgroundWorker.Dispose() uses the same unbounded WorkerTask.Wait() — latent single-threaded deadlock hazard
- Dominant language
- C#
- Stars
- 770
- Forks
- 248
- Avg merge
- 3d 4h
- Merged PRs (30d)
- 49
Description
### Context
Spun off from getsentry/sentry-dotnet#5237 / getsentry/sentry-dotnet#5330. While fixing the `BackpressureMonitor.Dispose()` deadlock on single-threaded targets (Unity WebGL / browser-wasm), we noticed `BackgroundWorker.Dispose()` uses the **same unbounded** `WorkerTask.Wait()` **pattern** and is therefore a candidate for the same hazard.
### The pattern
[`BackgroundWorker`]() starts its worker via `Task.Run` and parks it on an awaited semaphore wait:
```csharp
// ctor (line 43)
WorkerTask = Task.Run(DoWorkAsync);
// DoWorkAsync (line 120)
await _queuedEnvelopeSemaphore.WaitAsync(_shutdownSource.Token).ConfigureAwait(false);
```
and `Dispose()` (lines 328-355) does:
```csharp
_shutdownSource.Cancel();
// ...
WorkerTask.Wait(); // intentionally unbounded - see the comment in the code
```
This is structurally identical to the `BackpressureMonitor` deadlock in getsentry/sentry-dotnet#5237: on a runtime where the thread pool **is** the single main thread (Unity WebGL / browser-wasm), `WorkerTask.Wait()` blocks the only thread that could ever run the worker's post-cancellation continuation, so the task can never complete → deadlock. `ConfigureAwait(false)` does not help, because there is no separate thread for the continuation to escape to.
Note the deliberate comment in `Dispose()` explaining why no timeout/token is passed to `Wait()` ("we are waiting for the *continuation* of the method, not its *execution*"), so any fix needs to preserve the queue-draining/flush semantics.
### Reachability — why this is latent, not the cause of getsentry/sentry-dotnet#5237
In the **current** SDK, `BackgroundWorker.Dispose()` does **not** appear to be reached by the normal `SentrySdk.Close()` / `Hub.Dispose()` shutdown path:
* `Hub.Dispose()` flushes via `CurrentClient.FlushAsync(_options.ShutdownTimeout)` — **bounded** by `ShutdownTimeout` — and never disposes the client or the worker.
* `SentryClient.Dispose()` also only calls `Worker.FlushAsync(...)`; it never calls `Worker.Dispose()`.
* `SentryClient.Worker` is typed as the public `IBackgroundWorker`, which does **not** extend `IDisposable`; only the internal concrete `BackgroundWorker` does.
So the unbounded `WorkerTask.Wait()` seems only reachable from tests or from code that constructs/disposes a `BackgroundWorker` directly. This is consistent with the getsentry/sentry-dotnet#5237 reporter's ablation: setting `EnableBackpressureHandling = false` **fully** resolved the Unity WebGL freeze, which implies the only unbounded `Wait()` actually hit during their shutdown was `BackpressureMonitor.Dispose()` — not the background worker.
### Why file it anyway
getsentry/sentry-dotnet#5237 showed how easily this bites: the monitor's deadlock was live in 4.x/5.x and only *incidentally* stopped being reachable in 6.0.0 (when an unrelated change removed the `Hub.Dispose()` → `_backpressureMonitor?.Dispose()` call). The same could happen here in reverse — a future change that disposes the worker during shutdown would silently reintroduce a single-threaded deadlock.
### Proposed action
1. Confirm whether `BackgroundWorker.Dispose()` can be reached on single-threaded targets (Unity WebGL, Blazor WASM) in any supported configuration.
2. If so (or defensively regardless), make `Dispose()` not block the calling thread on `WorkerTask` on single-threaded runtimes — applying the same non-blocking approach used for `BackpressureMonitor` in getsentry/sentry-dotnet#5330, while preserving the flush/drain semantics the current unbounded `Wait()` guarantees.
### Repro / reference
* Issue: getsentry/sentry-dotnet#5237
* Fix PR for the monitor: getsentry/sentry-dotnet#5330
Contributor guide
Research direction
Start with src/Sentry/Internal/BackgroundWorker.cs, especially Dispose(), DoWorkAsync(), and the WorkerTask creation, then trace whether supported Unity WebGL or browser-WASM shutdown paths can reach Dispose(). Compare the non-blocking approach used for BackpressureMonitor in getsentry/sentry-dotnet#5330. Done means the reachability is confirmed or ruled out and any change preserves queue draining and flush semantics without blocking a single-threaded runtime.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100