Fix async cancellation failing to send TDS attention signal

[cheenamalhotra]

Description

Fixes #4424
Fixes #44

When using CancellationToken with async operations like ExecuteReaderAsync/ExecuteNonQueryAsync, cancellation fails to send a TDS attention signal to SQL Server if the server is blocked (e.g., infinite WHILE loop, WAITFOR DELAY, or partial results from RAISERROR WITH NOWAIT followed by a blocking operation). The cancellation hangs until the query completes naturally — which may be forever for infinite loops.

Root Cause

EndExecuteReaderAsync (and the equivalent NonQuery/XmlReader methods) held lock(_stateObj) while calling into EndExecute*Internal. When the server hadn't yet sent result metadata (blocked on a long-running or infinite query), TryConsumeMetaData performed a synchronous network read (_syncOverAsync = true), blocking the thread while holding the monitor lock.

Meanwhile, stateObj.Cancel() uses Monitor.TryEnter(this, 100ms) in a polling loop on the same stateObj instance. Since the lock was held by EndExecute*Async for the entire duration of the server-side wait, Cancel() could never acquire the monitor and never sent the attention signal.

Fix

Remove lock(_stateObj) from the user-facing async end methods:

• EndExecuteReaderAsync
• EndExecuteNonQueryAsync
• EndExecuteXmlReaderAsync

The lock is unnecessary in these paths because concurrent access is already handled by:

1. Cancel() — uses polling Monitor.TryEnter with parser state guards in its loop
2. Connection close — detected by TryRun checking parser Broken/Closed state
3. StateObj internals — handle their own synchronization for read/write operations

Note: The lock(_stateObj) in CreateLocalCompletionTask (AE internal-end/retry path) is intentionally preserved — that path handles cancellation via the task's IsCanceled state rather than via stateObj.Cancel().

Testing

• Added manual test for partial results scenario (severity 10 RAISERROR WITH NOWAIT + WAITFOR + CancellationToken)
• Added manual test for cancellation during ExecuteReaderAsync (WAITFOR as first statement — also covers infinite loop scenario from Canceling SQL Server query with while loop hangs forever #44)
• Added AE manual test exercising EndExecuteReaderAsync with AE-enabled commands (cache warmup + parameterized WAITFOR delay)
• All existing unit tests pass (904/908; 4 pre-existing failures unrelated to this change)
• Existing AE cancellation tests (TestSqlCommandCancellationToken) pass
• Public API unchanged — no ref assembly updates needed
• Verified against customer repro

Changes

File	Change
SqlCommand.Reader.cs	Removed lock(_stateObj) in EndExecuteReaderAsync
SqlCommand.NonQuery.cs	Removed lock(_stateObj) in EndExecuteNonQueryAsync
SqlCommand.Xml.cs	Removed lock(_stateObj) in EndExecuteXmlReaderAsync
DataReaderCancellationTest.cs	Added CancellationSendsAttention_WhenPartialResultsReceived and CancellationDuringExecuteReaderAsync_SendsAttention tests
ApiShould.cs	Added TestAsyncCancellationSendsAttention_WithAlwaysEncryptedCommand test

[Copilot]

Pull request overview

This PR fixes a deadlock/hang in async cancellation by removing lock(_stateObj) from the async EndExecute* paths so that TdsParserStateObject.Cancel() can acquire the monitor and send a TDS attention signal even when the async end-path is blocked on synchronous network reads (e.g., partial results + WAITFOR).

Changes:

• Removed lock(_stateObj) from EndExecuteReaderAsync, EndExecuteNonQueryAsync, and EndExecuteXmlReaderAsync to avoid blocking cancellation’s attention-send path.
• Added a new manual regression test intended to reproduce the “partial results + server blocked” cancellation hang scenario from #4424.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs	Removes lock(_stateObj) from EndExecuteReaderAsync and documents rationale tied to #4424.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs	Removes lock(_stateObj) from EndExecuteNonQueryAsync to keep cancellation from being blocked by the monitor.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs	Removes lock(_stateObj) from EndExecuteXmlReaderAsync consistent with reader/nonquery.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderCancellationTest.cs	Adds a manual regression test for cancellation sending attention after partial results are received.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs:2465

• The retained lock(_stateObj) in CreateLocalCompletionTask can still recreate the #4424 cancellation hang in the Always Encrypted internal-end path when the server sends any early packet (e.g., RAISERROR ... WITH NOWAIT) before blocking on metadata. In that case localCompletion can complete after the first packet (BeginExecuteInternalReadStage uses _stateObj.ReadSni), this continuation can then enter lock(_stateObj) and block inside InternalEndExecute while waiting synchronously for metadata—preventing TdsParserStateObject.Cancel() from acquiring the same monitor to send ATTENTION.

                        // Lock on _stateObj serializes this internal-end path with
                        // close/cancel. Note: stateObj.Cancel() also acquires this monitor,
                        // so this lock CAN block Cancel() while endFunc is executing.
                        // This is retained here (unlike the user-facing EndExecute* methods)
                        // because this continuation runs after the initial async I/O has
                        // already completed — the blocking metadata read that caused #4424
                        // in the user-facing path does not apply here since the data is
                        // already buffered by the time this continuation fires.
                        lock (_stateObj)
                        {
                            endFunc(this, task, /*isInternal:*/ true, endMethod);
                        }

[codecov]

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.91%. Comparing base (fdebcd2) to head (22528a8).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...lClient/src/Microsoft/Data/SqlClient/SqlCommand.cs	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4435      +/-   ##
==========================================
- Coverage   65.83%   63.91%   -1.93%     
==========================================
  Files         287      282       -5     
  Lines       43763    66650   +22887     
==========================================
+ Hits        28812    42599   +13787     
- Misses      14951    24051    +9100     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	63.91% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.