dotnet / dotnet/SqlClient

Sync ExecuteReader on an STA thread deadlocks against a StateChange handler that marshals to the UI thread (OpenAsyncRetry path)

Open Beginner friendly
#4,534 1 comment 0 reactions 0 assignees View on GitHub
External :link:
Dominant language
C#
Stars
989
Forks
340
Avg merge
4d 19h
Merged PRs (30d)
72

Description

### Summary

This is not a claim that SqlClient is at fault. The blocking call is in a closed-source consumer (SSMS). I am raising it here because the deadlock is only reachable through two SqlClient behaviours, one of which was explicitly flagged as a residual risk in #1213, and because a note in the docs would probably prevent the next occurrence of it.

Two SqlClient behaviours combine:

1. Synchronous `SqlCommand.ExecuteReader` funnels into `AsyncHelper.WaitForCompletion` → `Task.Wait`. On an STA thread that becomes a COM pumping wait, which dispatches COM calls and *sent* window messages but not *posted* ones.
2. `SqlConnection.OpenAsyncRetry.Retry` raises `DbConnection.StateChange` **on a thread-pool thread**, not on the thread that initiated the open.

If a consumer's `StateChange` handler marshals to the UI thread with a blocking call (`Control.Invoke`, `Dispatcher.Invoke`), and that UI thread is simultaneously inside a synchronous SqlClient call, the two deadlock permanently. Neither behaviour is a bug on its own.

### Versions

- `Microsoft.Data.SqlClient` 6.1.5 (file 6.15.26114.3)
- .NET Framework 4.8.9324.0, x64, STA
- Observed in SQL Server Management Studio 22.8.12023.21 against Azure SQL Database with Microsoft Entra authentication

### Observed stacks

Captured with ClrMD against the live wedged process.

UI thread (STA):

```
Microsoft.Data.SqlClient.SqlCommand.ExecuteReader
Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds
Microsoft.Data.SqlClient.AsyncHelper.WaitForCompletion
System.Threading.Tasks.Task.Wait
System.Threading.ManualResetEventSlim.Wait
System.Threading.Monitor.ObjWait
System.Threading.SynchronizationContext.WaitHelper
```

Thread-pool worker:

```
ThreadPoolWorkQueue.Dispatch
Task.Execute
Microsoft.Data.SqlClient.SqlConnection+OpenAsyncRetry.Retry
Microsoft.Data.SqlClient.SqlConnection.TryOpen
Microsoft.Data.SqlClient.SqlConnection.TryOpenInner
Microsoft.Data.ProviderBase.DbConnectionClosedConnecting.TryOpenConnection
Microsoft.Data.SqlClient.SqlConnectionFactory.SetInnerConnectionEvent
System.Data.Common.DbConnection.OnStateChange

System.Windows.Forms.Control.Invoke <-- blocking marshal to the UI thread
System.Windows.Forms.Control.WaitForWaitHandle
System.Threading.WaitHandle.WaitOne
```

The query is never sent. Server-side the sessions show `cpu_time` 0 and `last_request_end_time` equal to `last_request_start_time`.

### Minimal repro

.NET Framework 4.8 WinForms, `[STAThread]`, `Microsoft.Data.SqlClient` 6.1.5. On a button click:

```csharp
var completion = new TaskCompletionSource();

Task.Run(() =>
{
Thread.Sleep(1500); // let the UI thread enter its wait first
Invoke((Action)(() => { })); // blocking marshal, as a StateChange handler would do
completion.SetResult(true);
});

completion.Task.Wait(TimeSpan.FromMinutes(3)); // as AsyncHelper.WaitForCompletion does
```

Wedges every time and stays wedged. Swapping the single `Invoke` for `BeginInvoke` completes in ~1.5s with no other change and no external stimulus.

Worth noting for anyone debugging something similar: while wedged, `IsHungAppWindow` returns **false** and the process sits near 0% CPU, because the COM wait is still dispatching. It does not look like a hang from outside.

### Relationship to #1213

#1213 fixed the token-acquisition leg of this same family by wrapping `AcquireTokenAsync` in
`Task.Run` to escape the WinForms `SynchronizationContext`. In review, @roji noted:

> Changing Task.Run should indeed resolve the Winforms deadlock, since the code runs without the Winforms SynchronizationContext.

and also:

> This is still sync-over-async and therefore strongly discouraged, since it can cause various starvation/pseudo-deadlock effects

This looks like a concrete instance of that residual risk in the wild, four majors later, reached through the `StateChange` callback rather than through token acquisition.

### What I am actually asking for

Not necessarily a code change. In rough order of value:

1. **Document the hazard.** `DbConnection.StateChange` being raised on a thread-pool thread during the async-retry path is not obvious, and it means handlers must never block. A note on `SqlConnection.StateChange` or in the connection-pooling docs would be cheap and would have saved this investigation.
2. **Route it internally if you can.** The actual blocking call is in `Microsoft.SqlServer.Management.DataTools` (SSMS, closed source), which is in the same org. It is also reported at Developer Community 10857872, "SSMS Studio is busy after timeout", where it has been open and under investigation for a couple of years. The fix looks like one word: `Invoke` → `BeginInvoke`.
3. **Optional, and your call entirely:** whether `RunExecuteReaderTds` should be able to reach `WaitForCompletion` at all when called synchronously on an STA thread, given the known hazard.

Happy to attach the full repro project and raw stack dumps.

Contributor guide

Open the contributing guide

Research direction

Locate the SqlConnection.StateChange documentation and the connection-pooling documentation mentioned in the issue; use the OpenAsyncRetry.Retry behavior and the minimal WinForms repro as context. Done means the documentation clearly warns that StateChange handlers may run on a thread-pool thread during async retry and must not block while marshaling to a UI thread.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, sql
Domain
databases, documentation
Issue type
Documentation
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.