From ce65597028d3f74da2113abb3f3d492c50dc23c5 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Wed, 3 Mar 2021 11:04:37 -0800 Subject: [PATCH] [Release 1.1] Fix | MARS header errors when MakeReadAsyncBlocking = false --- .../Data/SqlClient/TdsParserStateObject.cs | 9 + .../Data/SqlClient/TdsParserStateObject.cs | 13 +- .../AsyncCancelledConnectionsTest.cs | 209 ++++++++++-------- 3 files changed, 142 insertions(+), 89 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 931bb53f7e..b101b7fbea 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -4128,6 +4128,15 @@ internal void ResetSnapshotState() _stateObj._cleanupMetaData = _snapshotCleanupMetaData; _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; + // Make sure to go through the appropriate increment/decrement methods if changing the OpenResult flag + if (!_stateObj.HasOpenResult && _state.HasFlag(SnapshottedStateFlags.OpenResult)) + { + _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); + } + else if (_stateObj.HasOpenResult && !_state.HasFlag(SnapshottedStateFlags.OpenResult)) + { + _stateObj.DecrementOpenResultCount(); + } _stateObj._snapshottedState = _state; // Reset partially read state (these only need to be maintained if doing async without snapshot) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index b10acfd1e5..d0fe83bf52 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -4221,7 +4221,18 @@ internal void ResetSnapshotState() _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; _stateObj._cleanupMetaData = _snapshotCleanupMetaData; _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; - _stateObj._hasOpenResult = _snapshotHasOpenResult; + + // Make sure to go through the appropriate increment/decrement methods if changing HasOpenResult + if (!_stateObj._hasOpenResult && _snapshotHasOpenResult) + { + _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); + } + else if (_stateObj._hasOpenResult && !_snapshotHasOpenResult) + { + _stateObj.DecrementOpenResultCount(); + } + //else _stateObj._hasOpenResult is already == _snapshotHasOpenResult + _stateObj._receivedColMetaData = _snapshotReceivedColumnMetadata; _stateObj._attentionReceived = _snapshotAttentionReceived; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs index 5578e968ce..33578b2f6d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs @@ -25,7 +25,20 @@ public AsyncCancelledConnectionsTest(ITestOutputHelper output) [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public void CancelAsyncConnections() { - string connectionString = DataTestUtility.TCPConnectionString; + SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); + builder.MultipleActiveResultSets = false; + RunCancelAsyncConnections(builder, false); + RunCancelAsyncConnections(builder, true); + builder.MultipleActiveResultSets = true; + RunCancelAsyncConnections(builder, false); + RunCancelAsyncConnections(builder, true); + } + + private void RunCancelAsyncConnections(SqlConnectionStringBuilder connectionStringBuilder, bool makeAsyncBlocking) + { + SqlConnection.ClearAllPools(); + AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking", makeAsyncBlocking); + _watch = Stopwatch.StartNew(); _random = new Random(4); // chosen via fair dice role. ParallelLoopResult results = new ParallelLoopResult(); @@ -37,7 +50,7 @@ public void CancelAsyncConnections() results = Parallel.For( fromInclusive: 0, toExclusive: NumberOfTasks, - (int i) => DoManyAsync(connectionString).GetAwaiter().GetResult()); + (int i) => DoManyAsync(connectionStringBuilder).GetAwaiter().GetResult()); } } catch (Exception ex) @@ -77,18 +90,26 @@ private void DisplaySummary() } // This is the the main body that our Tasks run - private async Task DoManyAsync(string connectionString) + private async Task DoManyAsync(SqlConnectionStringBuilder connectionStringBuilder) { Interlocked.Increment(ref _start); Interlocked.Increment(ref _inFlight); - // First poison - await DoOneAsync(connectionString, poison: true); - - for (int i = 0; i < NumberOfNonPoisoned && _continue; i++) + using (SqlConnection marsConnection = new SqlConnection(connectionStringBuilder.ToString())) { - // now run some without poisoning - await DoOneAsync(connectionString); + if (connectionStringBuilder.MultipleActiveResultSets) + { + await marsConnection.OpenAsync(); + } + + // First poison + await DoOneAsync(marsConnection, connectionStringBuilder.ToString(), poison: true); + + for (int i = 0; i < NumberOfNonPoisoned && _continue; i++) + { + // now run some without poisoning + await DoOneAsync(marsConnection, connectionStringBuilder.ToString()); + } } Interlocked.Decrement(ref _inFlight); @@ -99,95 +120,30 @@ private async Task DoManyAsync(string connectionString) // if we are poisoning we will // 1 - Interject some sleeps in the sql statement so that it will run long enough that we can cancel it // 2 - Setup a time bomb task that will cancel the command a random amount of time later - private async Task DoOneAsync(string connectionString, bool poison = false) + private async Task DoOneAsync(SqlConnection marsConnection, string connectionString, bool poison = false) { try { - using (var connection = new SqlConnection(connectionString)) + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 4; i++) { - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < 4; i++) + builder.AppendLine("SELECT name FROM sys.tables"); + if (poison && i < 3) { - builder.AppendLine("SELECT name FROM sys.tables"); - if (poison && i < 3) - { - builder.AppendLine("WAITFOR DELAY '00:00:01'"); - } + builder.AppendLine("WAITFOR DELAY '00:00:01'"); } + } - int rowsRead = 0; - int resultRead = 0; - - try + using (var connection = new SqlConnection(connectionString)) + { + if (marsConnection != null && marsConnection.State == System.Data.ConnectionState.Open) { - await connection.OpenAsync(); - using (var command = connection.CreateCommand()) - { - Task timeBombTask = default; - try - { - // Setup our time bomb - if (poison) - { - timeBombTask = TimeBombAsync(command); - } - - command.CommandText = builder.ToString(); - - // Attempt to read all of the data - using (var reader = await command.ExecuteReaderAsync()) - { - try - { - do - { - resultRead++; - while (await reader.ReadAsync() && _continue) - { - rowsRead++; - } - } - while (await reader.NextResultAsync() && _continue); - } - catch when (poison) - { - // This looks a little strange, we failed to read above so this should fail too - // But consider the case where this code is elsewhere (in the Dispose method of a class holding this logic) - try - { - while (await reader.NextResultAsync()) - { - } - } - catch - { - Interlocked.Increment(ref _poisonCleanUpExceptions); - } - - throw; - } - } - } - finally - { - // Make sure to clean up our time bomb - // It is unlikely, but the timebomb may get delayed in the Task Queue - // And we don't want it running after we dispose the command - if (timeBombTask != default) - { - await timeBombTask; - } - } - } + await RunCommand(marsConnection, builder.ToString(), poison); } - finally + else { - Interlocked.Add(ref _rowsRead, rowsRead); - Interlocked.Add(ref _resultRead, resultRead); - if (poison) - { - Interlocked.Increment(ref _poisonedEnded); - } + await connection.OpenAsync(); + await RunCommand(connection, builder.ToString(), poison); } } } @@ -223,6 +179,83 @@ private async Task DoOneAsync(string connectionString, bool poison = false) } } + private async Task RunCommand(SqlConnection connection, string commandText, bool poison) + { + int rowsRead = 0; + int resultRead = 0; + + try + { + using (var command = connection.CreateCommand()) + { + Task timeBombTask = default; + try + { + // Setup our time bomb + if (poison) + { + timeBombTask = TimeBombAsync(command); + } + + command.CommandText = commandText; + + // Attempt to read all of the data + using (var reader = await command.ExecuteReaderAsync()) + { + try + { + do + { + resultRead++; + while (await reader.ReadAsync() && _continue) + { + rowsRead++; + } + } + while (await reader.NextResultAsync() && _continue); + } + catch when (poison) + { + // This looks a little strange, we failed to read above so this should fail too + // But consider the case where this code is elsewhere (in the Dispose method of a class holding this logic) + try + { + while (await reader.NextResultAsync()) + { + } + } + catch + { + Interlocked.Increment(ref _poisonCleanUpExceptions); + } + + throw; + } + } + } + finally + { + // Make sure to clean up our time bomb + // It is unlikely, but the timebomb may get delayed in the Task Queue + // And we don't want it running after we dispose the command + if (timeBombTask != default) + { + await timeBombTask; + } + } + } + } + finally + { + Interlocked.Add(ref _rowsRead, rowsRead); + Interlocked.Add(ref _resultRead, resultRead); + if (poison) + { + Interlocked.Increment(ref _poisonedEnded); + } + } + } + private async Task TimeBombAsync(SqlCommand command) { await SleepAsync(100, 3000);