[cheenamalhotra]

Description

Fixes #4424
Fixes #44
Fixes #4691

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

Also remove lock(_stateObj) from CreateLocalCompletionTask (the internal-end/retry path used when column encryption is enabled and parameter metadata came from the cache).

An earlier revision of this PR kept that lock and justified it on the grounds that the continuation runs only after the response is buffered. That was wrong. localCompletion is completed from BeginExecuteReaderInternalReadStage -> _stateObj.ReadSni(completion), which resolves as soon as a packet arrives. And FinishExecuteReader only early-returns on the _internalEndExecuteInitiated fast path when !isInternal, so the internal call falls through to a blocking TryRun(RunBehavior.UntilDone, ...). Holding the monitor across that read starves TdsParserStateObject.Cancel(), which polls Monitor.TryEnter for the same monitor to send attention.

Commit ac8acd72 had reverted this because AE TestCancellationToken failed. The exception on the unlocked path is a SqlException with Number == 0 and two errors (A severe error occurred on the current command. then Operation cancelled by user.) — a correct cancellation, where the first error is what TryRun adds on consuming the attention ack. The test asserted an exact whole-message match, so that assertion is relaxed to Assert.Contains rather than reverting the fix.

Testing

Which tests fail without the source fix?

Measured per-test against a live SQL Server, not inferred:

Test	Fails on main?	Evidence
CancellationSendsAttention_WhenPartialResultsReceived	Yes	RAISERROR ... WITH NOWAIT flushes, WAITFOR blocks; reader returned instead of cancelling
CancellationOfInfiniteWhileLoop_DoesNotHang	Yes	loop streams DONE tokens (~30k server writes in 6s), so the End path runs and blocks; cts.Cancel() never returns
CancellationOnInternalEndExecutePath_SendsAttention	Yes	reader returned 60005ms after cancellation was requested
CancellationDuringExecuteXmlReaderAsync_SendsAttention	Yes	reader returned 60014ms after cancellation was requested
CancellationDuringExecuteReaderAsync_SendsAttention	No	WAITFOR first, nothing flushed, End path never runs — genuine guard test

DataReaderCancellationTest totals: 4 failed / 3 passed on main; 7 passed in 1s with the fix.

Note on the infinite WHILE case: it is not merely slow on main. The token callback runs SqlCommand.Cancel() synchronously on the cancelling thread, so CancellationTokenSource.Cancel() itself blocks indefinitely (observed >20 minutes). With the fix it returns in 4ms.

Suggested release note entry

Release notes in this repo are batched into dedicated release PRs rather than added per bugfix, so no file is added here. Suggested entry:

Fixed — Fixed asynchronous cancellation failing to send a TDS attention signal when the server had already flushed part of a response, which caused CancellationToken cancellation of ExecuteReaderAsync, ExecuteNonQueryAsync and ExecuteXmlReaderAsync to be ignored until the command completed on its own. In the worst case CancellationTokenSource.Cancel() itself would block indefinitely. (#4424, #4435)

• Added manual test for partial results scenario (severity 10 RAISERROR WITH NOWAIT + WAITFOR + CancellationToken) — fails pre-fix
• Added manual test for cancellation during ExecuteReaderAsync (WAITFOR as first statement)
• Added manual test for the infinite WHILE loop repro from Canceling SQL Server query with while loop hangs forever #44 — fails pre-fix
• Added AE manual test exercising EndExecuteReaderAsync with AE-enabled commands (cache warmup + parameterized WAITFOR delay)
• Added CancellationOnInternalEndExecutePath_SendsAttention (forces the internal-end path via the DEBUG _forceInternalEndQuery hook, so it runs without an AE setup and skips against a Release driver) — fails pre-fix
• Added CancellationDuringExecuteXmlReaderAsync_SendsAttention, closing the ExecuteXmlReaderAsync coverage gap (FOR XML RAW; FOR XML AUTO fails at compile time before RAISERROR runs) — fails pre-fix
• Relaxed AE TestCancellationToken exact-message assertion to Assert.Contains (attention ack adds a leading error)
• Observed the abandoned execTask on the watchdog branch to avoid unobserved task exceptions leaking into unrelated tests
• Cancellation in the partial-results test is synchronized on InfoMessage arrival (asserted), not a fixed timer
• 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
SqlCommand.cs	Removed lock(_stateObj) in CreateLocalCompletionTask (internal-end path)
DataReaderCancellationTest.cs	Added CancellationSendsAttention_WhenPartialResultsReceived, CancellationDuringExecuteReaderAsync_SendsAttention, CancellationOfInfiniteWhileLoop_DoesNotHang, CancellationOnInternalEndExecutePath_SendsAttention and CancellationDuringExecuteXmlReaderAsync_SendsAttention tests
CommandHelper.cs / DataTestUtility.cs	Exposed the DEBUG _forceInternalEndQuery hook so the internal-end path can be tested without an AE setup
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]

🟡 Changes recommended

Approval is blocked by active moderate findings: several cancellation tests may cancel before reaching the server, and multiple connection setups can fail under Azure SQL authentication.

2 open issues

• Use the shared connection factory to attach Azure SQL credentials · New
• Wait for the infinite-loop query to reach the server before cancelling · New

_{Review effort: Lite}

[Copilot]

🔵 Needs a closer look

A moderate finding remains because the test bypasses the shared connection factory, which can cause supported Azure SQL configurations to fail before exercising the cancellation regression.

2 open issues

• Wait for the infinite-loop query to reach the server before cancelling
• Use the shared connection factory to attach Azure SQL credentials

_{Review effort: Lite}