From e161d9d1be11ccaa97deacf3b4d0e609041230c3 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 8 Oct 2019 13:13:18 -0700 Subject: [PATCH 1/7] Port CoreFx PR 38271: Fix Statement Command Cancellation (Managed SNI) --- .../Data/SqlClient/SNI/SNINpHandle.cs | 47 ++++++++++++--- .../Microsoft/Data/SqlClient/SNI/SNIPacket.cs | 6 +- .../Data/SqlClient/SNI/SNITcpHandle.cs | 50 ++++++++++++---- .../SqlClient/TdsParserStateObjectManaged.cs | 1 + .../SQL/SqlCommand/SqlCommandCancelTest.cs | 60 +++++++++++++++++++ 5 files changed, 145 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs index 2716bde270..2275300f5f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs @@ -9,6 +9,7 @@ using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; +using System.Threading; namespace Microsoft.Data.SqlClient.SNI { @@ -22,6 +23,7 @@ internal class SNINpHandle : SNIHandle private readonly string _targetServer; private readonly object _callbackObject; + private readonly object _sendSync; private Stream _stream; private NamedPipeClientStream _pipeStream; @@ -37,6 +39,7 @@ internal class SNINpHandle : SNIHandle public SNINpHandle(string serverName, string pipeName, long timerExpire, object callbackObject) { + _sendSync = new object(); _targetServer = serverName; _callbackObject = callbackObject; @@ -193,20 +196,48 @@ public override uint ReceiveAsync(ref SNIPacket packet) public override uint Send(SNIPacket packet) { - lock (this) + bool releaseLock = false; + try { - try + // is the packet is marked out out-of-band (attention packets only) it must be + // sent immediately even if a send of recieve operation is already in progress + // because out of band packets are used to cancel ongoing operations + // so try to take the lock if possible but continue even if it can't be taken + if (packet.IsOutOfBand) { - packet.WriteToStream(_stream); - return TdsEnums.SNI_SUCCESS; + Monitor.TryEnter(this, ref releaseLock); } - catch (ObjectDisposedException ode) + else { - return ReportErrorAndReleasePacket(packet, ode); + Monitor.Enter(this); + releaseLock = true; } - catch (IOException ioe) + + // this lock ensures that two packets are not being written to the transport at the same time + // so that sending a standard and an out-of-band packet are both written atomically no data is + // interleaved + lock (_sendSync) { - return ReportErrorAndReleasePacket(packet, ioe); + try + { + packet.WriteToStream(_stream); + return TdsEnums.SNI_SUCCESS; + } + catch (ObjectDisposedException ode) + { + return ReportErrorAndReleasePacket(packet, ode); + } + catch (IOException ioe) + { + return ReportErrorAndReleasePacket(packet, ioe); + } + } + } + finally + { + if (releaseLock) + { + Monitor.Exit(this); } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs index efb9a4e334..45832ee2d4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs @@ -19,7 +19,6 @@ internal partial class SNIPacket : IDisposable, IEquatable private int _offset; private string _description; private SNIAsyncCallback _completionCallback; - private bool _isBufferFromArrayPool; public SNIPacket() { } @@ -50,6 +49,11 @@ public string Description /// public int DataLeft => (_length - _offset); + /// + /// Indicates that the packet should be sent out of band bypassing the normal send-recieve lock + /// + public bool IsOutOfBand { get; set; } + /// /// Length of data /// diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs index 5f4bb63fea..ab0986dbad 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs @@ -24,6 +24,7 @@ internal class SNITCPHandle : SNIHandle { private readonly string _targetServer; private readonly object _callbackObject; + private readonly object _sendSync; private readonly Socket _socket; private NetworkStream _tcpStream; @@ -104,6 +105,7 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba { _callbackObject = callbackObject; _targetServer = serverName; + _sendSync = new object(); try { @@ -420,24 +422,52 @@ public override void SetBufferSize(int bufferSize) /// SNI error code public override uint Send(SNIPacket packet) { - lock (this) + bool releaseLock = false; + try { - try + // is the packet is marked out out-of-band (attention packets only) it must be + // sent immediately even if a send of recieve operation is already in progress + // because out of band packets are used to cancel ongoing operations + // so try to take the lock if possible but continue even if it can't be taken + if (packet.IsOutOfBand) { - packet.WriteToStream(_stream); - return TdsEnums.SNI_SUCCESS; + Monitor.TryEnter(this, ref releaseLock); } - catch (ObjectDisposedException ode) + else { - return ReportTcpSNIError(ode); + Monitor.Enter(this); + releaseLock = true; } - catch (SocketException se) + + // this lock ensures that two packets are not being written to the transport at the same time + // so that sending a standard and an out-of-band packet are both written atomically no data is + // interleaved + lock (_sendSync) { - return ReportTcpSNIError(se); + try + { + packet.WriteToStream(_stream); + return TdsEnums.SNI_SUCCESS; + } + catch (ObjectDisposedException ode) + { + return ReportTcpSNIError(ode); + } + catch (SocketException se) + { + return ReportTcpSNIError(se); + } + catch (IOException ioe) + { + return ReportTcpSNIError(ioe); + } } - catch (IOException ioe) + } + finally + { + if (releaseLock) { - return ReportTcpSNIError(ioe); + Monitor.Exit(this); } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs index 51c81340a6..1f97c96fd3 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -163,6 +163,7 @@ internal override PacketHandle CreateAndSetAttentionPacket() SetPacketData(PacketHandle.FromManagedPacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN); _sniAsyncAttnPacket = attnPacket; } + PacketHandle.FromManagedPacket(_sniAsyncAttnPacket).ManagedPacket.IsOutOfBand = true; return PacketHandle.FromManagedPacket(_sniAsyncAttnPacket); } 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..44b73cc220 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,65 @@ private static void TimeOutDuringRead(string constr) throw; } } + + [CheckConnStrSetupFact] + public static void CancelDoesNotWait() + { + 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)) + { + conn.Open(); + + Task.Delay(TimeSpan.FromSeconds(cancelSeconds)) + .ContinueWith(t => cmd.Cancel()); + + DateTime started = DateTime.UtcNow; + DateTime ended = default; + Exception exception = null; + try + { + cmd.ExecuteNonQuery(); + } + catch (Exception ex) + { + exception = ex; + } + ended = DateTime.UtcNow; + + Assert.NotNull(exception); + Assert.InRange((ended - started).TotalSeconds, cancelSeconds, delaySeconds - 1); + } + } + + [CheckConnStrSetupFact] + 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(1000).Token); + } + catch (Exception ex) + { + exception = ex; + } + DateTime ended = DateTime.UtcNow; + + Assert.NotNull(exception); + Assert.InRange((ended - started).TotalSeconds, cancelSeconds, delaySeconds - 1); + } + } } } From 4a81b64f80840a21dea57cfb763d058ba25c94d1 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 8 Oct 2019 13:53:49 -0700 Subject: [PATCH 2/7] Fix default literal --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 44b73cc220..dd40c2dd27 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -298,7 +298,7 @@ public static void CancelDoesNotWait() .ContinueWith(t => cmd.Cancel()); DateTime started = DateTime.UtcNow; - DateTime ended = default; + DateTime ended = DateTime.UtcNow; Exception exception = null; try { From 23abcf520c6048b4d656ab3a6bdabaa7cf6c24ef Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 7 Jan 2020 09:13:54 -0800 Subject: [PATCH 3/7] Minor test changes --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 83 ++++++++++--------- 1 file changed, 46 insertions(+), 37 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 8f872c61f2..c564c9c5b9 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -125,21 +125,23 @@ private static void MultiThreadedCancel(string constr, bool async) using (SqlConnection con = new SqlConnection(constr)) { con.Open(); - var command = con.CreateCommand(); - command.CommandText = "select * from orders; waitfor delay '00:00:08'; select * from customers"; + using (var command = con.CreateCommand()) + { + command.CommandText = "select * from orders; waitfor delay '00:00:08'; select * from customers"; - Barrier threadsReady = new Barrier(2); - object state = new Tuple(async, command, threadsReady); + Barrier threadsReady = new Barrier(2); + object state = new Tuple(async, command, threadsReady); - Task[] tasks = new Task[2]; - tasks[0] = new Task(ExecuteCommandCancelExpected, state); - tasks[1] = new Task(CancelSharedCommand, state); - tasks[0].Start(); - tasks[1].Start(); + Task[] tasks = new Task[2]; + tasks[0] = new Task(ExecuteCommandCancelExpected, state); + tasks[1] = new Task(CancelSharedCommand, state); + tasks[0].Start(); + tasks[1].Start(); - Task.WaitAll(tasks, 15 * 1000); + Task.WaitAll(tasks, 15 * 1000); - SqlCommandCancelTest.VerifyConnection(command); + SqlCommandCancelTest.VerifyConnection(command); + } } } @@ -148,14 +150,16 @@ private static void TimeoutCancel(string constr) using (SqlConnection con = new SqlConnection(constr)) { con.Open(); - SqlCommand cmd = con.CreateCommand(); - cmd.CommandTimeout = 1; - cmd.CommandText = "WAITFOR DELAY '00:00:30';select * from Customers"; + using (SqlCommand cmd = con.CreateCommand()) + { + cmd.CommandTimeout = 1; + cmd.CommandText = "WAITFOR DELAY '00:00:30';select * from Customers"; - string errorMessage = SystemDataResourceManager.Instance.SQL_Timeout_Execution; - DataTestUtility.ExpectFailure(() => cmd.ExecuteReader(), new string[] { errorMessage }); + string errorMessage = SystemDataResourceManager.Instance.SQL_Timeout_Execution; + DataTestUtility.ExpectFailure(() => cmd.ExecuteReader(), new string[] { errorMessage }); - VerifyConnection(cmd); + VerifyConnection(cmd); + } } } @@ -253,34 +257,39 @@ private static void TimeOutDuringRead(string constr) { // Start the command conn.Open(); - SqlCommand cmd = new SqlCommand("SELECT @p", conn); - cmd.Parameters.AddWithValue("p", new byte[20000]); - SqlDataReader reader = cmd.ExecuteReader(); - reader.Read(); - - // Tweak the timeout to 1ms, stop the proxy from proxying and then try GetValue (which should timeout) - reader.SetDefaultTimeout(1); - proxy.PauseCopying(); - string errorMessage = SystemDataResourceManager.Instance.SQL_Timeout_Execution; - Exception exception = Assert.Throws(() => reader.GetValue(0)); - Assert.Contains(errorMessage, exception.Message); - - // Return everything to normal and close - proxy.ResumeCopying(); - reader.SetDefaultTimeout(30000); - reader.Dispose(); + using (SqlCommand cmd = new SqlCommand("SELECT @p", conn)) + { + cmd.Parameters.AddWithValue("p", new byte[20000]); + using (SqlDataReader reader = cmd.ExecuteReader()) + { + reader.Read(); + + // Tweak the timeout to 1ms, stop the proxy from proxying and then try GetValue (which should timeout) + reader.SetDefaultTimeout(1); + proxy.PauseCopying(); + string errorMessage = SystemDataResourceManager.Instance.SQL_Timeout_Execution; + Exception exception = Assert.Throws(() => reader.GetValue(0)); + Assert.Contains(errorMessage, exception.Message); + + // Return everything to normal and close + proxy.ResumeCopying(); + reader.SetDefaultTimeout(30000); + reader.Dispose(); + } + } } - - proxy.Stop(); } catch { // In case of error, stop the proxy and dump its logs (hopefully this will help with debugging proxy.Stop(); Console.WriteLine(proxy.GetServerEventLog()); - Assert.True(false, "Error while reading through proxy"); throw; } + finally + { + proxy.Stop(); + } } [CheckConnStrSetupFact] @@ -330,7 +339,7 @@ public static async Task AsyncCancelDoesNotWait() Exception exception = null; try { - await cmd.ExecuteNonQueryAsync(new CancellationTokenSource(1000).Token); + await cmd.ExecuteNonQueryAsync(new CancellationTokenSource(2000).Token); } catch (Exception ex) { From 922d3d4a8977e7dec7d015df4b2cdf6a9458894c Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 7 Jan 2020 09:22:30 -0800 Subject: [PATCH 4/7] Minor change to avoid random failures --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c564c9c5b9..ccb52be19d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -303,7 +303,7 @@ public static void CancelDoesNotWait() { conn.Open(); - Task.Delay(TimeSpan.FromSeconds(cancelSeconds)) + Task.Delay(TimeSpan.FromSeconds(cancelSeconds * 2)) .ContinueWith(t => cmd.Cancel()); DateTime started = DateTime.UtcNow; From 39be357a6ca69eee8285762d277fb9d957fa011e Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 7 Jan 2020 12:32:35 -0800 Subject: [PATCH 5/7] Run tests with both TCP and NP connection strings --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 68 +++++++++++++------ 1 file changed, 49 insertions(+), 19 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 ccb52be19d..d6a8b039c4 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -13,30 +13,35 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests public static class SqlCommandCancelTest { // Shrink the packet size - this should make timeouts more likely - private static readonly string s_connStr = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { PacketSize = 512 }).ConnectionString; + private static readonly string tcp_connStr = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { PacketSize = 512 }).ConnectionString; + private static readonly string np_connStr = (new SqlConnectionStringBuilder(DataTestUtility.NPConnectionString) { PacketSize = 512 }).ConnectionString; [CheckConnStrSetupFact] public static void PlainCancelTest() { - PlainCancel(s_connStr); + PlainCancel(tcp_connStr); + PlainCancel(np_connStr); } [CheckConnStrSetupFact] public static void PlainMARSCancelTest() { - PlainCancel((new SqlConnectionStringBuilder(s_connStr) { MultipleActiveResultSets = true }).ConnectionString); + PlainCancel((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); + PlainCancel((new SqlConnectionStringBuilder(np_connStr) { MultipleActiveResultSets = true }).ConnectionString); } [CheckConnStrSetupFact] public static void PlainCancelTestAsync() { - PlainCancelAsync(s_connStr); + PlainCancelAsync(tcp_connStr); + PlainCancelAsync(np_connStr); } [CheckConnStrSetupFact] public static void PlainMARSCancelTestAsync() { - PlainCancelAsync((new SqlConnectionStringBuilder(s_connStr) { MultipleActiveResultSets = true }).ConnectionString); + PlainCancelAsync((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); + PlainCancelAsync((new SqlConnectionStringBuilder(np_connStr) { MultipleActiveResultSets = true }).ConnectionString); } private static void PlainCancel(string connString) @@ -92,32 +97,51 @@ private static void PlainCancelAsync(string connString) [CheckConnStrSetupFact] public static void MultiThreadedCancel_NonAsync() { - MultiThreadedCancel(s_connStr, false); + MultiThreadedCancel(tcp_connStr, false); + MultiThreadedCancel(np_connStr, false); } [CheckConnStrSetupFact] public static void MultiThreadedCancel_Async() { - MultiThreadedCancel(s_connStr, true); + MultiThreadedCancel(tcp_connStr, true); + MultiThreadedCancel(np_connStr, true); } [CheckConnStrSetupFact] public static void TimeoutCancel() { - TimeoutCancel(s_connStr); + TimeoutCancel(tcp_connStr); + TimeoutCancel(np_connStr); } [CheckConnStrSetupFact] public static void CancelAndDisposePreparedCommand() { - CancelAndDisposePreparedCommand(s_connStr); + CancelAndDisposePreparedCommand(tcp_connStr); + CancelAndDisposePreparedCommand(np_connStr); } [ActiveIssue(5541)] [CheckConnStrSetupFact] public static void TimeOutDuringRead() { - TimeOutDuringRead(s_connStr); + TimeOutDuringRead(tcp_connStr); + TimeOutDuringRead(np_connStr); + } + + [CheckConnStrSetupFact] + public static void CancelDoesNotWait() + { + CancelDoesNotWait(tcp_connStr); + CancelDoesNotWait(np_connStr); + } + + [CheckConnStrSetupFact] + public static void AsyncCancelDoesNotWait() + { + AsyncCancelDoesNotWait(tcp_connStr).Wait(); + AsyncCancelDoesNotWait(np_connStr).Wait(); } private static void MultiThreadedCancel(string constr, bool async) @@ -153,16 +177,22 @@ private static void TimeoutCancel(string constr) using (SqlCommand cmd = con.CreateCommand()) { cmd.CommandTimeout = 1; - cmd.CommandText = "WAITFOR DELAY '00:00:30';select * from Customers"; + cmd.CommandText = "WAITFOR DELAY '00:00:20';select * from Customers"; string errorMessage = SystemDataResourceManager.Instance.SQL_Timeout_Execution; - DataTestUtility.ExpectFailure(() => cmd.ExecuteReader(), new string[] { errorMessage }); + DataTestUtility.ExpectFailure(() => ExecuteReaderOnCmd(cmd), new string[] { errorMessage }); VerifyConnection(cmd); } } } + private static void ExecuteReaderOnCmd(SqlCommand cmd) + { + using (SqlDataReader reader = cmd.ExecuteReader()) + { } + } + //InvalidOperationException from connection.Dispose if that connection has prepared command cancelled during reading of data private static void CancelAndDisposePreparedCommand(string constr) { @@ -292,18 +322,18 @@ private static void TimeOutDuringRead(string constr) } } - [CheckConnStrSetupFact] - public static void CancelDoesNotWait() + private static void CancelDoesNotWait(string connStr) { const int delaySeconds = 30; const int cancelSeconds = 1; - using (SqlConnection conn = new SqlConnection(s_connStr)) + using (SqlConnection conn = new SqlConnection(connStr)) using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn)) { conn.Open(); - Task.Delay(TimeSpan.FromSeconds(cancelSeconds * 2)) + // Cancel after 2 seconds as sometimes total time elapsed can be .99 in case of 1 second that causes random failures + Task.Delay(TimeSpan.FromSeconds(cancelSeconds + 1)) .ContinueWith(t => cmd.Cancel()); DateTime started = DateTime.UtcNow; @@ -324,13 +354,12 @@ public static void CancelDoesNotWait() } } - [CheckConnStrSetupFact] - public static async Task AsyncCancelDoesNotWait() + private static async Task AsyncCancelDoesNotWait(string connStr) { const int delaySeconds = 30; const int cancelSeconds = 1; - using (SqlConnection conn = new SqlConnection(s_connStr)) + using (SqlConnection conn = new SqlConnection(connStr)) using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn)) { await conn.OpenAsync(); @@ -339,6 +368,7 @@ public static async Task AsyncCancelDoesNotWait() Exception exception = null; try { + // Cancel after 2 seconds as sometimes total time elapsed can be .99 in case of 1 second that causes random failures await cmd.ExecuteNonQueryAsync(new CancellationTokenSource(2000).Token); } catch (Exception ex) From be5565f0a0656d50ee48f04afa4e699725c3b680 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 7 Jan 2020 12:52:43 -0800 Subject: [PATCH 6/7] Separate tests --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) 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 d6a8b039c4..fd52e3e899 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -20,6 +20,12 @@ public static class SqlCommandCancelTest public static void PlainCancelTest() { PlainCancel(tcp_connStr); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void PlainCancelTestNP() + { PlainCancel(np_connStr); } @@ -27,6 +33,12 @@ public static void PlainCancelTest() public static void PlainMARSCancelTest() { PlainCancel((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void PlainMARSCancelTestNP() + { PlainCancel((new SqlConnectionStringBuilder(np_connStr) { MultipleActiveResultSets = true }).ConnectionString); } @@ -34,6 +46,12 @@ public static void PlainMARSCancelTest() public static void PlainCancelTestAsync() { PlainCancelAsync(tcp_connStr); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void PlainCancelTestAsyncNP() + { PlainCancelAsync(np_connStr); } @@ -41,6 +59,12 @@ public static void PlainCancelTestAsync() public static void PlainMARSCancelTestAsync() { PlainCancelAsync((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void PlainMARSCancelTestAsyncNP() + { PlainCancelAsync((new SqlConnectionStringBuilder(np_connStr) { MultipleActiveResultSets = true }).ConnectionString); } @@ -98,6 +122,12 @@ private static void PlainCancelAsync(string connString) public static void MultiThreadedCancel_NonAsync() { MultiThreadedCancel(tcp_connStr, false); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void MultiThreadedCancel_NonAsyncNP() + { MultiThreadedCancel(np_connStr, false); } @@ -105,6 +135,12 @@ public static void MultiThreadedCancel_NonAsync() public static void MultiThreadedCancel_Async() { MultiThreadedCancel(tcp_connStr, true); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void MultiThreadedCancel_AsyncNP() + { MultiThreadedCancel(np_connStr, true); } @@ -112,6 +148,12 @@ public static void MultiThreadedCancel_Async() public static void TimeoutCancel() { TimeoutCancel(tcp_connStr); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void TimeoutCancelNP() + { TimeoutCancel(np_connStr); } @@ -119,6 +161,12 @@ public static void TimeoutCancel() public static void CancelAndDisposePreparedCommand() { CancelAndDisposePreparedCommand(tcp_connStr); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void CancelAndDisposePreparedCommandNP() + { CancelAndDisposePreparedCommand(np_connStr); } @@ -127,6 +175,13 @@ public static void CancelAndDisposePreparedCommand() public static void TimeOutDuringRead() { TimeOutDuringRead(tcp_connStr); + } + + [ActiveIssue(5541)] + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void TimeOutDuringReadNP() + { TimeOutDuringRead(np_connStr); } @@ -134,6 +189,12 @@ public static void TimeOutDuringRead() public static void CancelDoesNotWait() { CancelDoesNotWait(tcp_connStr); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void CancelDoesNotWaitNP() + { CancelDoesNotWait(np_connStr); } @@ -141,6 +202,12 @@ public static void CancelDoesNotWait() public static void AsyncCancelDoesNotWait() { AsyncCancelDoesNotWait(tcp_connStr).Wait(); + } + + [CheckConnStrSetupFact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void AsyncCancelDoesNotWaitNP() + { AsyncCancelDoesNotWait(np_connStr).Wait(); } From 58cdfc619e19bf878cd2f06f15cabb2290e9f90c Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 7 Jan 2020 13:32:02 -0800 Subject: [PATCH 7/7] Skip Named Pipes on Azure --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 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 fd52e3e899..4740daaaf2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -22,7 +22,7 @@ public static void PlainCancelTest() PlainCancel(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void PlainCancelTestNP() { @@ -35,7 +35,7 @@ public static void PlainMARSCancelTest() PlainCancel((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void PlainMARSCancelTestNP() { @@ -48,7 +48,7 @@ public static void PlainCancelTestAsync() PlainCancelAsync(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void PlainCancelTestAsyncNP() { @@ -61,7 +61,7 @@ public static void PlainMARSCancelTestAsync() PlainCancelAsync((new SqlConnectionStringBuilder(tcp_connStr) { MultipleActiveResultSets = true }).ConnectionString); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void PlainMARSCancelTestAsyncNP() { @@ -124,7 +124,7 @@ public static void MultiThreadedCancel_NonAsync() MultiThreadedCancel(tcp_connStr, false); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void MultiThreadedCancel_NonAsyncNP() { @@ -137,7 +137,7 @@ public static void MultiThreadedCancel_Async() MultiThreadedCancel(tcp_connStr, true); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void MultiThreadedCancel_AsyncNP() { @@ -150,7 +150,7 @@ public static void TimeoutCancel() TimeoutCancel(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void TimeoutCancelNP() { @@ -163,7 +163,7 @@ public static void CancelAndDisposePreparedCommand() CancelAndDisposePreparedCommand(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void CancelAndDisposePreparedCommandNP() { @@ -178,7 +178,7 @@ public static void TimeOutDuringRead() } [ActiveIssue(5541)] - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void TimeOutDuringReadNP() { @@ -191,7 +191,7 @@ public static void CancelDoesNotWait() CancelDoesNotWait(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void CancelDoesNotWaitNP() { @@ -204,7 +204,7 @@ public static void AsyncCancelDoesNotWait() AsyncCancelDoesNotWait(tcp_connStr).Wait(); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public static void AsyncCancelDoesNotWaitNP() {