-
Notifications
You must be signed in to change notification settings - Fork 331
Fix async cancellation failing to send TDS attention signal #4435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
564e0db
71d63e3
5ca7e0f
89b51a9
7590ffe
b30baf4
c4ab8ec
ac8acd7
9431172
22528a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2276,6 +2276,107 @@ public void TestSqlCommandCancellationToken(string connection, int initalValue, | |
| } | ||
|
|
||
|
|
||
| /// <summary> | ||
| /// Validates that async cancellation via CancellationToken sends a TDS attention signal | ||
| /// when an AE-enabled command is blocked on a server-side wait (e.g., WAITFOR DELAY). | ||
| /// This exercises the EndExecuteReaderAsync path where the lock on _stateObj previously | ||
| /// prevented Cancel() from sending attention. See GitHub issue #4424. | ||
| /// Synapse: Incompatible query. | ||
| /// </summary> | ||
| [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsTargetReadyForAeWithKeyStore))] | ||
| [ClassData(typeof(AEConnectionStringProvider))] | ||
| public async Task TestAsyncCancellationSendsAttention_WithAlwaysEncryptedCommand(string connection) | ||
| { | ||
| CleanUpTable(connection, _tableName); | ||
|
|
||
| IList<object> values = GetValues(dataHint: 60); | ||
| int numberOfRows = 10; | ||
| int rowsAffected = InsertRows(tableName: _tableName, numberofRows: numberOfRows, values: values, connection: connection); | ||
| Assert.True(rowsAffected == numberOfRows, "number of rows affected is unexpected."); | ||
|
|
||
| using (SqlConnection sqlConnection = new SqlConnection(connection)) | ||
| { | ||
| await sqlConnection.OpenAsync(); | ||
|
|
||
| // Use the same CommandText for both warmup and cancellation test so the | ||
| // query metadata cache key matches and the second execution goes through | ||
| // the CreateLocalCompletionTask internal-end path. | ||
| // WAITFOR is placed BEFORE SELECT so that EndExecuteReaderInternal blocks | ||
| // waiting for result-set metadata — this is the exact CreateLocalCompletionTask | ||
| // code path we need to exercise. | ||
| string commandText = $"WAITFOR DELAY @Delay; SELECT CustomerId, FirstName, LastName FROM [{_tableName}] WHERE FirstName = @FirstName AND CustomerId = @CustomerId;"; | ||
|
|
||
|
cheenamalhotra marked this conversation as resolved.
|
||
| // Warmup: execute with zero delay to populate the metadata cache. | ||
| using (SqlCommand warmupCmd = new SqlCommand( | ||
| commandText, sqlConnection, null, SqlCommandColumnEncryptionSetting.Enabled)) | ||
| { | ||
| warmupCmd.Parameters.AddWithValue("@CustomerId", values[0]); | ||
| warmupCmd.Parameters.AddWithValue("@FirstName", values[1]); | ||
| warmupCmd.Parameters.AddWithValue("@Delay", "00:00:00"); | ||
|
|
||
| using (var reader = await warmupCmd.ExecuteReaderAsync()) | ||
| { | ||
| while (await reader.ReadAsync()) { } | ||
| while (await reader.NextResultAsync()) { } | ||
| } | ||
| } | ||
|
|
||
| // Now execute the same command with a long delay and cancel it. | ||
| // With cached metadata, this goes through CreateLocalCompletionTask's internal-end path. | ||
| // The WAITFOR blocks before any result-set metadata is returned, so | ||
| // ExecuteReaderAsync should be cancelled before a reader is produced. | ||
| using (SqlCommand sqlCommand = new SqlCommand( | ||
| commandText, sqlConnection, null, SqlCommandColumnEncryptionSetting.Enabled)) | ||
| { | ||
| sqlCommand.Parameters.AddWithValue("@CustomerId", values[0]); | ||
| sqlCommand.Parameters.AddWithValue("@FirstName", values[1]); | ||
| sqlCommand.Parameters.AddWithValue("@Delay", "00:01:00"); | ||
| sqlCommand.CommandTimeout = 90; | ||
|
|
||
| using (CancellationTokenSource cts = new CancellationTokenSource(TimeSpan.FromSeconds(3))) | ||
| { | ||
| System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew(); | ||
|
|
||
| Exception caughtException = null; | ||
| try | ||
| { | ||
| using (SqlDataReader reader = await sqlCommand.ExecuteReaderAsync(cts.Token)) | ||
| { | ||
| // If we reach here, cancellation failed during EndExecuteReaderInternal. | ||
| Assert.Fail("ExecuteReaderAsync should have been cancelled before returning a reader."); | ||
| } | ||
| } | ||
| catch (OperationCanceledException ex) | ||
| { | ||
| caughtException = ex; | ||
| } | ||
| catch (SqlException ex) | ||
| { | ||
| // Attention ack from server manifests as SqlException | ||
| caughtException = ex; | ||
| } | ||
|
|
||
| stopwatch.Stop(); | ||
|
|
||
| Assert.NotNull(caughtException); | ||
| Assert.True(cts.IsCancellationRequested, | ||
| "CancellationTokenSource was not cancelled; exception may be unrelated to cancellation."); | ||
| Assert.True(stopwatch.ElapsedMilliseconds < 30000, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this become another flaky test? Is there a way to programmatically cancel the token rather than giving it a delayed cancellation (3 seconds)? Could you cancel it from another thread after starting the async read? Same for the next test as well. Ideally, these would be unit tests and we would feed bytes to our TDS parser and no timing would be involved. Next best would be to expand the TDS Test server to allow programmatic delays and whatnot. |
||
| $"Cancellation took {stopwatch.ElapsedMilliseconds}ms, expected < 30000ms. " + | ||
| "Attention signal may not have been sent during AE async execution."); | ||
| } | ||
| } | ||
|
|
||
| // Verify the connection is still usable after cancellation. | ||
| using (SqlCommand verifyCmd = new SqlCommand( | ||
| $"SELECT COUNT(*) FROM [{_tableName}]", sqlConnection)) | ||
| { | ||
| object result = await verifyCmd.ExecuteScalarAsync(); | ||
| Assert.Equal(numberOfRows, (int)result); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsSGXEnclaveConnStringSetup))] | ||
| public void TestNoneAttestationProtocolWithSGXEnclave() | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.