From 6daefef2deb7dd892d31c871ec0e3a4fc3d4ceee Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 8 Oct 2019 14:20:22 -0700 Subject: [PATCH 1/2] Port CoreFx PR 38266: Fix MARS timeout cancellation (Managed SNI) --- .../src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs | 5 ++++- .../netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs | 6 ++++++ .../SQL/ConnectionPoolTest/ConnectionPoolTest.cs | 4 ++-- .../ConnectionPoolTest/PoolBlockPeriodTest.netcoreapp.cs | 6 +++--- .../ManualTests/SQL/DataStreamTest/DataStreamTest.cs | 2 +- .../tests/ManualTests/SQL/MARSTest/MARSTest.cs | 4 ++-- .../SQL/SqlCredentialTest/SqlCredentialTest.cs | 8 ++++---- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs index 53ba03ca06..f0629c8eac 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs @@ -132,7 +132,10 @@ public void HandleReceiveError(SNIPacket packet) Debug.Assert(Monitor.IsEntered(this), "HandleReceiveError was called without being locked."); foreach (SNIMarsHandle handle in _sessions.Values) { - handle.HandleReceiveError(packet); + if (packet.HasCompletionCallback) + { + handle.HandleReceiveError(packet); + } } packet?.Dispose(); } 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..aa173c0070 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 @@ -44,6 +44,12 @@ public string Description _description = value; } } + public bool HasCompletionCallback => !(_completionCallback is null); + + /// + /// Dispose Packet data + /// + public void Dispose() => Release(); /// /// Length of data left to process diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index 5cde4243b1..0d9c3e34d2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -15,13 +15,13 @@ public static class ConnectionPoolTest private static readonly string _tcpConnStr = (new SqlConnectionStringBuilder(DataTestUtility.TcpConnStr) { MultipleActiveResultSets = false, Pooling = true }).ConnectionString; private static readonly string _tcpMarsConnStr = (new SqlConnectionStringBuilder(DataTestUtility.TcpConnStr) { MultipleActiveResultSets = true, Pooling = true }).ConnectionString; - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)]: */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void ConnectionPool_NonMars() { RunDataTestForSingleConnString(_tcpConnStr); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void ConnectionPool_Mars() { RunDataTestForSingleConnString(_tcpMarsConnStr); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/PoolBlockPeriodTest.netcoreapp.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/PoolBlockPeriodTest.netcoreapp.cs index 4c5282cbdf..7124d33766 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/PoolBlockPeriodTest.netcoreapp.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/PoolBlockPeriodTest.netcoreapp.cs @@ -23,7 +23,7 @@ public class PoolBlockPeriodTest private static bool AreConnectionStringsSetup() => DataTestUtility.AreConnStringsSetup(); [OuterLoop()] - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [InlineData("Azure with Default Policy must Disable blocking (*.database.windows.net)", new object[] { AzureEndpointSample })] [InlineData("Azure with Default Policy must Disable blocking (*.database.chinacloudapi.cn)", new object[] { AzureChinaEnpointSample })] [InlineData("Azure with Default Policy must Disable blocking (*.database.usgovcloudapi.net)", new object[] { AzureUSGovernmentEndpointSample })] @@ -49,7 +49,7 @@ public void TestAzureBlockingPeriod(string description, object[] Params) } [OuterLoop()] - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [InlineData("NonAzure with Default Policy must Enable blocking", new object[] { NonExistentServer })] [InlineData("NonAzure with Auto Policy must Enable Blocking", new object[] { NonExistentServer, PoolBlockingPeriod.Auto })] [InlineData("NonAzure with Always Policy must Enable Blocking", new object[] { NonExistentServer, PoolBlockingPeriod.AlwaysBlock })] @@ -72,7 +72,7 @@ public void TestNonAzureBlockingPeriod(string description, object[] Params) } [OuterLoop()] - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [InlineData("Test policy with Auto (lowercase)", "auto")] [InlineData("Test policy with Auto (PascalCase)", "Auto")] [InlineData("Test policy with Always (lowercase)", "alwaysblock")] diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs index 1a0fbf7205..3ab5724056 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs @@ -68,7 +68,7 @@ private static void RunAllTestsForSingleServer(string connectionString, bool usi // These tests fail with named pipes, since they try to do DNS lookups on named pipe paths. if (!usingNamePipes) { - if (DataTestUtility.IsUsingNativeSNI()) /* [ActiveIssue(108)] */ + if (DataTestUtility.IsUsingNativeSNI()) { TimeoutDuringReadAsyncWithClosedReaderTest(connectionString); } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSTest/MARSTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSTest/MARSTest.cs index e2255d11ff..10f86d8f26 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSTest/MARSTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSTest/MARSTest.cs @@ -35,7 +35,7 @@ public static void NamedPipesMARSTest() } #if DEBUG - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void MARSAsyncTimeoutTest() { using (SqlConnection connection = new SqlConnection(_connStr)) @@ -75,7 +75,7 @@ public static void MARSAsyncTimeoutTest() } } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void MARSSyncTimeoutTest() { using (SqlConnection connection = new SqlConnection(_connStr)) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCredentialTest/SqlCredentialTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCredentialTest/SqlCredentialTest.cs index 995bc9dce5..fa6fd9f83c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCredentialTest/SqlCredentialTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCredentialTest/SqlCredentialTest.cs @@ -12,7 +12,7 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests public static class SqlCredentialTest { - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer), /* [ActiveIssue(108)] */nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void CreateSqlConnectionWithCredential() { var user = "u" + Guid.NewGuid().ToString().Replace("-", ""); @@ -45,7 +45,7 @@ public static void CreateSqlConnectionWithCredential() } [ActiveIssue("9196")] - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer), /* [ActiveIssue(108)] */nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void SqlConnectionChangePasswordPlaintext() { var user = "u" + Guid.NewGuid().ToString().Replace("-", ""); @@ -79,7 +79,7 @@ public static void SqlConnectionChangePasswordPlaintext() } [ActiveIssue("9196")] - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer), /* [ActiveIssue(108)] */ nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void SqlConnectionChangePasswordSecureString() { var user = "u" + Guid.NewGuid().ToString().Replace("-", ""); @@ -119,7 +119,7 @@ public static void SqlConnectionChangePasswordSecureString() } } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer), /* [ActiveIssue(108)] */nameof(DataTestUtility.IsUsingNativeSNI))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void OldCredentialsShouldFail() { String user = "u" + Guid.NewGuid().ToString().Replace("-", ""); From 37f14d59ab6e51005e4a3dd312145ec20c2a3e8b Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 8 Oct 2019 14:25:11 -0700 Subject: [PATCH 2/2] Fix duplicated method --- .../netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs | 8 -------- 1 file changed, 8 deletions(-) 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 aa173c0070..17437880a2 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 @@ -66,14 +66,6 @@ public string Description /// public bool IsInvalid => (_data == null); - /// - /// Packet data - /// - public void Dispose() - { - Release(); - } - /// /// Set async completion callback ///