Fix connection pool concurrency issue (#3632)

Author: cheenamalhotra

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

@@ -383,6 +383,41 @@ internal WaitHandle[] GetHandles(bool withCreate)
             }
         }
 
+        /// <summary>
+        /// Helper class to obtain and release a semaphore.
+        /// </summary>
+        internal class SemaphoreHolder : IDisposable
+        {
+            private readonly Semaphore _semaphore;
+
+            /// <summary>
+            /// Whether the semaphore was successfully obtained within the timeout.
+            /// </summary>
+            internal bool Obtained { get; private set; }
+
+            /// <summary>
+            /// Obtains the semaphore, waiting up to the specified timeout.
+            /// </summary>
+            /// <param name="semaphore"></param>
+            /// <param name="timeout"></param>
+            internal SemaphoreHolder(Semaphore semaphore, int timeout)
+            {
+                _semaphore = semaphore;
+                Obtained = _semaphore.WaitOne(timeout);
+            }
+
+            /// <summary>
+            /// Releases the semaphore if it was successfully obtained.
+            /// </summary>
+            public void Dispose()
+            {
+                if (Obtained)
+                {
+                    _semaphore.Release(1);
+                }
+            }
+        }
+
         private const int MAX_Q_SIZE = 0x00100000;
 
         // The order of these is important; we want the WaitAny call to be signaled
@@ -1321,20 +1356,14 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj
 
                                     if (onlyOneCheckConnection)
                                     {
-                                        if (_waitHandles.CreationSemaphore.WaitOne(unchecked((int)waitForMultipleObjectsTimeout)))
+                                        using SemaphoreHolder semaphoreHolder = new(_waitHandles.CreationSemaphore, unchecked((int)waitForMultipleObjectsTimeout));
+                                        if (semaphoreHolder.Obtained)
                                         {
 #if NETFRAMEWORK
                                             RuntimeHelpers.PrepareConstrainedRegions();
 #endif
-                                            try
-                                            {
-                                                SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Creating new connection.", Id);
-                                                obj = UserCreateRequest(owningObject, userOptions);
-                                            }
-                                            finally
-                                            {
-                                                _waitHandles.CreationSemaphore.Release(1);
-                                            }
+                                            SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Creating new connection.", Id);
+                                            obj = UserCreateRequest(owningObject, userOptions);
                                         }
                                         else
                                         {
@@ -1553,26 +1582,20 @@ private void PoolCreateRequest(object state)
                             {
                                 return;
                             }
-                            int waitResult = BOGUS_HANDLE;
 
 #if NETFRAMEWORK
                             RuntimeHelpers.PrepareConstrainedRegions();
 #endif
                             try
                             {
                                 // Obtain creation mutex so we're the only one creating objects
-                                // and we must have the wait result
+                                using SemaphoreHolder semaphoreHolder = new(_waitHandles.CreationSemaphore, CreationTimeout);
+
+                                if (semaphoreHolder.Obtained)
+                                {
 #if NETFRAMEWORK
                                 RuntimeHelpers.PrepareConstrainedRegions();
 #endif
-                                try
-                                { }
-                                finally
-                                {
-                                    waitResult = WaitHandle.WaitAny(_waitHandles.GetHandles(withCreate: true), CreationTimeout);
-                                }
-                                if (CREATION_HANDLE == waitResult)
-                                {
                                     DbConnectionInternal newObj;
 
                                     // Check ErrorOccurred again after obtaining mutex
@@ -1606,17 +1629,12 @@ private void PoolCreateRequest(object state)
                                         }
                                     }
                                 }
-                                else if (WaitHandle.WaitTimeout == waitResult)
+                                else
                                 {
                                     // do not wait forever and potential block this worker thread
                                     // instead wait for a period of time and just requeue to try again
                                     QueuePoolCreateRequest();
                                 }
-                                else
-                                {
-                                    // trace waitResult and ignore the failure
-                                    SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.PoolCreateRequest|RES|CPOOL> {0}, PoolCreateRequest called WaitForSingleObject failed {1}", Id, waitResult);
-                                }
                             }
                             catch (Exception e)
                             {
@@ -1630,14 +1648,6 @@ private void PoolCreateRequest(object state)
                                 // thrown to the user the next time they request a connection.
                                 SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.PoolCreateRequest|RES|CPOOL> {0}, PoolCreateRequest called CreateConnection which threw an exception: {1}", Id, e);
                             }
-                            finally
-                            {
-                                if (CREATION_HANDLE == waitResult)
-                                {
-                                    // reuse waitResult and ignore its value
-                                    _waitHandles.CreationSemaphore.Release(1);
-                                }
-                            }
                         }
                     }
                 }

src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj

@@ -258,6 +258,7 @@
     <None Include="SQL\ParameterTest\SqlParameterTest_ReleaseMode_Azure.bsl" />
   </ItemGroup>
   <ItemGroup Condition="'$(TestSet)' == '' OR '$(TestSet)' == '3'">
+    <Compile Include="SQL\ConnectionPoolTest\ConnectionPoolStressTest.cs" />
     <Compile Include="TracingTests\MetricsTest.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetGroup)'=='netcoreapp' AND ('$(TestSet)' == '' OR '$(TestSet)' == '3')">