From e529adcfa086a0f0622c8034d7784aa570476bb9 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sun, 29 Sep 2019 18:34:20 +0100 Subject: [PATCH 1/3] add cancel running query test and remove state lock in cancel method --- .../Data/SqlClient/TdsParserStateObject.cs | 50 +++++++++---------- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 33 ++++++++++++ 2 files changed, 58 insertions(+), 25 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 499b22de44..e23e87e698 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 @@ -598,41 +598,41 @@ internal void Cancel(object caller) // Keep looping until we either grabbed the lock (and therefore sent attention) or the connection closes\breaks while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) { + // try to take the lock so that if another command is attempted it will queue on the lock + // but don't require that the lock be taken because otherwise attention cannot be sent + // during command execution causing cancellation to wait + // This lock is also protecting against concurrent close and async continuations Monitor.TryEnter(this, _waitForCancellationLockPollTimeout, ref hasLock); - if (hasLock) - { // Lock for the time being - since we need to synchronize the attention send. - // This lock is also protecting against concurrent close and async continuations - // Ensure that, once we have the lock, that we are still the owner - if ((!_cancelled) && (_cancellationOwner.Target == caller)) - { - _cancelled = true; + // Ensure that that we are still the owner + if ((!_cancelled) && (_cancellationOwner.Target == caller)) + { + _cancelled = true; - if (_pendingData && !_attentionSent) + if (_pendingData && !_attentionSent) + { + bool hasParserLock = false; + // Keep looping until we have the parser lock (and so are allowed to write), or the connection closes\breaks + while ((!hasParserLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) { - bool hasParserLock = false; - // Keep looping until we have the parser lock (and so are allowed to write), or the connection closes\breaks - while ((!hasParserLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) + try { - try + _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: _waitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); + if (hasParserLock) { - _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: _waitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); - if (hasParserLock) - { - _parser.Connection.ThreadHasParserLockForClose = true; - SendAttention(); - } + _parser.Connection.ThreadHasParserLockForClose = true; + SendAttention(); } - finally + } + finally + { + if (hasParserLock) { - if (hasParserLock) + if (_parser.Connection.ThreadHasParserLockForClose) { - if (_parser.Connection.ThreadHasParserLockForClose) - { - _parser.Connection.ThreadHasParserLockForClose = false; - } - _parser.Connection._parserLock.Release(); + _parser.Connection.ThreadHasParserLockForClose = false; } + _parser.Connection._parserLock.Release(); } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 7d8c233904..861e686ca8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -282,5 +282,38 @@ private static void TimeOutDuringRead(string constr) throw; } } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CancelInInfiniteLoop() + { + Exception exception = null; + DateTime started = DateTime.UtcNow; + using (var connection = new SqlConnection(s_connStr)) + { + await connection.OpenAsync().ConfigureAwait(false); + using (var command = connection.CreateCommand()) + { + command.CommandText = @" + WHILE 1 = 1 + BEGIN + DECLARE @x INT = 1 + END + "; + command.CommandTimeout = 30; + CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(1000); + try + { + await command.ExecuteNonQueryAsync(cancellationTokenSource.Token).ConfigureAwait(false); + } + catch (Exception ex) + { + exception = ex; + } + } + } + DateTime ended = DateTime.UtcNow; + Assert.InRange((ended - started).TotalSeconds, 1, 9); + Assert.NotNull(exception); + } } } From 27cf2e52714626911959d319e897bf04238f8877 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Thu, 17 Oct 2019 11:38:39 +0100 Subject: [PATCH 2/3] use delay wait in cancel tests to avoid hanging test runner --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 111 ++++++++++++++---- 1 file changed, 86 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 861e686ca8..140f0b890b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -283,37 +283,98 @@ private static void TimeOutDuringRead(string constr) } } + //[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + //public static async Task CancelInInfiniteLoop() + //{ + // Exception exception = null; + // DateTime started = DateTime.UtcNow; + // using (var connection = new SqlConnection(s_connStr)) + // { + // await connection.OpenAsync().ConfigureAwait(false); + // using (var command = connection.CreateCommand()) + // { + // command.CommandText = @" + // WHILE 1 = 1 + // BEGIN + // DECLARE @x INT = 1 + // END + // "; + // command.CommandTimeout = 30; + // CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(1000); + // try + // { + // await command.ExecuteNonQueryAsync(cancellationTokenSource.Token).ConfigureAwait(false); + // } + // catch (Exception ex) + // { + // exception = ex; + // } + // } + // } + // DateTime ended = DateTime.UtcNow; + // Assert.InRange((ended - started).TotalSeconds, 1, 9); + // Assert.NotNull(exception); + //} + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] - public static async Task CancelInInfiniteLoop() + public static void CancelDoesNotWait() { - Exception exception = null; - DateTime started = DateTime.UtcNow; - using (var connection = new SqlConnection(s_connStr)) + const int delaySeconds = 30; + const int cancelSeconds = 1; + + using (SqlConnection conn = new SqlConnection(s_connStr)) + using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn)) { - await connection.OpenAsync().ConfigureAwait(false); - using (var command = connection.CreateCommand()) + conn.Open(); + + Task.Delay(TimeSpan.FromSeconds(cancelSeconds)) + .ContinueWith(t => cmd.Cancel()); + + DateTime started = DateTime.UtcNow; + DateTime ended = DateTime.MinValue; + Exception exception = null; + try { - command.CommandText = @" - WHILE 1 = 1 - BEGIN - DECLARE @x INT = 1 - END - "; - command.CommandTimeout = 30; - CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(1000); - try - { - await command.ExecuteNonQueryAsync(cancellationTokenSource.Token).ConfigureAwait(false); - } - catch (Exception ex) - { - exception = ex; - } + cmd.ExecuteNonQuery(); + } + catch (Exception ex) + { + exception = ex; + } + ended = DateTime.UtcNow; + + Assert.NotNull(exception); + Assert.InRange((ended - started).TotalSeconds, 0, delaySeconds - 1); + } + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task AsyncCancelDoesNotWait() + { + const int delaySeconds = 30; + const int cancelSeconds = 1; + + using (SqlConnection conn = new SqlConnection(s_connStr)) + using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn)) + { + await conn.OpenAsync(); + + DateTime started = DateTime.UtcNow; + Exception exception = null; + try + { + await cmd.ExecuteNonQueryAsync(new CancellationTokenSource(TimeSpan.FromSeconds(cancelSeconds)).Token); } + catch (Exception ex) + { + exception = ex; + } + DateTime ended = DateTime.UtcNow; + + Assert.NotNull(exception); + Assert.InRange((ended - started).TotalSeconds, cancelSeconds, delaySeconds - 1); } - DateTime ended = DateTime.UtcNow; - Assert.InRange((ended - started).TotalSeconds, 1, 9); - Assert.NotNull(exception); } + } } From 8016662cb55cbd850ba9d9e10115ecb522304ca4 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Thu, 17 Oct 2019 11:50:33 +0100 Subject: [PATCH 3/3] remove commented out test --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 140f0b890b..e7f213837b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -283,39 +283,6 @@ private static void TimeOutDuringRead(string constr) } } - //[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] - //public static async Task CancelInInfiniteLoop() - //{ - // Exception exception = null; - // DateTime started = DateTime.UtcNow; - // using (var connection = new SqlConnection(s_connStr)) - // { - // await connection.OpenAsync().ConfigureAwait(false); - // using (var command = connection.CreateCommand()) - // { - // command.CommandText = @" - // WHILE 1 = 1 - // BEGIN - // DECLARE @x INT = 1 - // END - // "; - // command.CommandTimeout = 30; - // CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(1000); - // try - // { - // await command.ExecuteNonQueryAsync(cancellationTokenSource.Token).ConfigureAwait(false); - // } - // catch (Exception ex) - // { - // exception = ex; - // } - // } - // } - // DateTime ended = DateTime.UtcNow; - // Assert.InRange((ended - started).TotalSeconds, 1, 9); - // Assert.NotNull(exception); - //} - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void CancelDoesNotWait() {