Fixed Issue dotnet#1126 .NET Core SqlConnection ConnectTimout 15 less…

… than not work?
arellegue · Jun 13, 2023 · cde0430 · cde0430
1 parent 9b9af7e
commit cde0430
Show file tree

Hide file tree

Showing 10 changed files with 56 additions and 25 deletions.
diff --git a/build.proj b/build.proj
@@ -105,7 +105,7 @@
 
   <Target Name="BuildTools" Condition="'$(BuildTools)' == 'true'">
     <PropertyGroup>
-      <DotnetBuildCmd>$(DotNetCmd) dotnet build -c Release -p:ReferenceType=$(ReferenceType)"</DotnetBuildCmd>
+      <DotnetBuildCmd>$(DotNetCmd) dotnet build -c Release -p:ReferenceType=$(ReferenceType)</DotnetBuildCmd>
     </PropertyGroup>
     <Exec Command="$(DotnetBuildCmd)" WorkingDirectory="$(GenAPISrcDir)Microsoft.DotNet.GenAPI\" />
   </Target>

diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs
@@ -130,7 +130,6 @@ private static bool IsErrorStatus(SecurityStatusPalErrorCode errorCode)
         /// Create a SNI connection handle
         /// </summary>
         /// <param name="fullServerName">Full server name from connection string</param>
-        /// <param name="ignoreSniOpenTimeout">Ignore open timeout</param>
         /// <param name="timerExpire">Timer expiration</param>
         /// <param name="instanceName">Instance name</param>
         /// <param name="spnBuffer">SPN</param>
@@ -148,7 +147,6 @@ private static bool IsErrorStatus(SecurityStatusPalErrorCode errorCode)
         /// <returns>SNI handle</returns>
         internal static SNIHandle CreateConnectionHandle(
             string fullServerName,
-            bool ignoreSniOpenTimeout,
             long timerExpire,
             out byte[] instanceName,
             ref byte[][] spnBuffer,

diff --git a/...Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/...Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
@@ -1538,7 +1538,6 @@ private void LoginNoFailover(ServerInfo serverInfo,
                     AttemptOneLogin(serverInfo,
                                     newPassword,
                                     newSecurePassword,
-                                    !connectionOptions.MultiSubnetFailover,    // ignore timeout for SniOpen call unless MSF
                                     connectionOptions.MultiSubnetFailover ? intervalTimer : timeout);
 
                     if (connectionOptions.MultiSubnetFailover && null != ServerProvidedFailOverPartner)
@@ -1777,7 +1776,6 @@ TimeoutTimer timeout
                             currentServerInfo,
                             newPassword,
                             newSecurePassword,
-                            false,          // Use timeout in SniOpen
                             intervalTimer,
                             withFailover: true
                             );
@@ -1905,7 +1903,6 @@ private void AttemptOneLogin(
                                 ServerInfo serverInfo,
                                 string newPassword,
                                 SecureString newSecurePassword,
-                                bool ignoreSniOpenTimeout,
                                 TimeoutTimer timeout,
                                 bool withFailover = false)
         {
@@ -1916,7 +1913,6 @@ private void AttemptOneLogin(
 
             _parser.Connect(serverInfo,
                             this,
-                            ignoreSniOpenTimeout,
                             timeout.LegacyTimerExpire,
                             ConnectionOptions,
                             withFailover);

diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
@@ -361,7 +361,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)
         internal void Connect(
             ServerInfo serverInfo,
             SqlInternalConnectionTds connHandler,
-            bool ignoreSniOpenTimeout,
             long timerExpire,
             SqlConnectionString connectionOptions,
             bool withFailover)
@@ -445,7 +444,6 @@ internal void Connect(
             // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
             _physicalStateObj.CreatePhysicalSNIHandle(
                 serverInfo.ExtendedServerName,
-                ignoreSniOpenTimeout,
                 timerExpire,
                 out instanceName,
                 ref _sniSpnBuffer,
@@ -544,7 +542,6 @@ internal void Connect(
 
                 _physicalStateObj.CreatePhysicalSNIHandle(
                     serverInfo.ExtendedServerName,
-                    ignoreSniOpenTimeout,
                     timerExpire, out instanceName,
                     ref _sniSpnBuffer,
                     true,

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
@@ -197,7 +197,6 @@ private void ResetCancelAndProcessAttention()
 
         internal abstract void CreatePhysicalSNIHandle(
             string serverName,
-            bool ignoreSniOpenTimeout,
             long timerExpire,
             out byte[] instanceName,
             ref byte[][] spnBuffer,

diff --git a/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs
@@ -77,7 +77,6 @@ protected override uint SNIPacketGetData(PacketHandle packet, byte[] inBuff, ref
 
         internal override void CreatePhysicalSNIHandle(
             string serverName,
-            bool ignoreSniOpenTimeout,
             long timerExpire,
             out byte[] instanceName,
             ref byte[][] spnBuffer,
@@ -93,7 +92,7 @@ internal override void CreatePhysicalSNIHandle(
             string hostNameInCertificate,
             string serverCertificateFilename)
         {
-            SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, ignoreSniOpenTimeout, timerExpire, out instanceName, ref spnBuffer, serverSPN,
+            SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, timerExpire, out instanceName, ref spnBuffer, serverSPN,
                 flushCache, async, parallel, isIntegratedSecurity, iPAddressPreference, cachedFQDN, ref pendingDNSInfo, tlsFirst,
                 hostNameInCertificate, serverCertificateFilename);
 

diff --git a/...crosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/...crosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
@@ -140,7 +140,6 @@ private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async)
 
         internal override void CreatePhysicalSNIHandle(
             string serverName,
-            bool ignoreSniOpenTimeout,
             long timerExpire,
             out byte[] instanceName,
             ref byte[][] spnBuffer,
@@ -199,7 +198,7 @@ internal override void CreatePhysicalSNIHandle(
             SQLDNSInfo cachedDNSInfo;
             bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out cachedDNSInfo);
 
-            _sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer[0], ignoreSniOpenTimeout, checked((int)timeout), out instanceName,
+            _sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer[0], checked((int)timeout), out instanceName,
                 flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate);
         }
 

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
@@ -319,7 +319,7 @@ internal void CreatePhysicalSNIHandle(
 
             _ = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out SQLDNSInfo cachedDNSInfo);
 
-            _sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer, ignoreSniOpenTimeout, checked((int)timeout),
+            _sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer, checked((int)timeout),
                 out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout,
                 ipPreference, cachedDNSInfo, hostNameInCertificate);
         }

diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs
@@ -150,7 +150,6 @@ internal SNIHandle(
             SNINativeMethodWrapper.ConsumerInfo myInfo,
             string serverName,
             byte[] spnBuffer,
-            bool ignoreSniOpenTimeout,
             int timeout,
             out byte[] instanceName,
             bool flushCache,
@@ -174,14 +173,6 @@ internal SNIHandle(
             {
                 _fSync = fSync;
                 instanceName = new byte[256]; // Size as specified by netlibs.
-                if (ignoreSniOpenTimeout)
-                {
-                    // UNDONE: ITEM12001110 (DB Mirroring Reconnect) Old behavior of not truly honoring timeout presevered 
-                    //  for non-failover scenarios to avoid breaking changes as part of a QFE.  Consider fixing timeout
-                    //  handling in next full release and removing ignoreSniOpenTimeout parameter.
-                    timeout = Timeout.Infinite; // -1 == native SNIOPEN_TIMEOUT_VALUE / INFINITE
-                }
-
 #if NETFRAMEWORK
                 int transparentNetworkResolutionStateNo = (int)transparentNetworkResolutionState;
                 _status = SNINativeMethodWrapper.SNIOpenSyncEx(myInfo, serverName, ref base.handle,

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs
@@ -248,5 +248,57 @@ public void ConnectionTestValidCredentialCombination()
 
             Assert.Equal(sqlCredential, conn.Credential);
         }
+
+        [Theory]
+        [InlineData(60)]
+        [InlineData(30)]
+        [InlineData(15)]
+        [InlineData(10)]
+        [InlineData(5)]
+        [InlineData(1)]
+        public void ConnectionTimeoutTest(int timeout)
+        {
+            // Start a server with connection timeout from the inline data.
+            using TestTdsServer server = TestTdsServer.StartTestServer(false, false, timeout);
+            using SqlConnection connection = new SqlConnection(server.ConnectionString);
+
+            // Dispose the server to force connection timeout 
+            server.Dispose();
+
+            // Measure the actual time it took to timeout and compare it with configured timeout
+            var start = DateTime.Now;
+            var end = start;
+
+            // Open a connection with the server disposed.
+            try
+            {
+                connection.Open();
+            }
+            catch (Exception)
+            {
+                end = DateTime.Now;
+            }
+
+            // Calculate actual duration of timeout
+            TimeSpan s = end - start;
+            // Did not time out?
+            if (s.TotalSeconds == 0)
+                Assert.True(s.TotalSeconds == 0);
+
+            // Is actual time out the same as configured timeout or within an additional 3 second threshold because of overhead?
+            if (s.TotalSeconds > 0)
+                Assert.True(s.TotalSeconds <= timeout + 3);
+        }
+
+        [Theory]
+        [InlineData(-5)]
+        public void ConnectionInvalidTimeoutTest(int timeout)
+        {
+            // Start a server with connection timeout from the inline data.
+            Assert.Throws<ArgumentException>(() =>
+            {
+                using TestTdsServer server = TestTdsServer.StartTestServer(false, false, timeout);
+            });
+        }
     }
 }