getsentry / getsentry/sentry-dotnet

BackgroundWorker.Dispose() uses the same unbounded WorkerTask.Wait() — latent single-threaded deadlock hazard

Open
#5,332 2 comments 0 reactions 0 assignees View on GitHub
.NET
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.