Preserve in-flight opens when clearing connection pools

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

@@ -900,12 +900,8 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource<DbC
                 return true;
             }
 
-            // Shutdown short-circuit for the async path. The inner TryGetConnection returns false
-            // when it observes State is not Running mid-WaitAny. Without this re-check, we would
-            // enqueue a PendingGetConnection and spin up a WaitForPendingOpen background thread
-            // against an already-shut-down pool; the caller would eventually surface a misleading
-            // PooledOpenTimeout instead of a deterministic shutdown signal. Returning (true, null)
-            // matches the sync path and the channel pool's TryGetConnection convention.
+            // Before queueing, the factory can still redirect this request to a new pool.
+            // Once queued, the request must finish on this pool, even if it is retired.
             if (State is not Running)
             {
                 SqlClientEventSource.Log.TryPoolerTraceEvent(
@@ -980,34 +976,6 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj
                     {
                         waitResult = WaitHandle.WaitAny(_waitHandles.GetHandles(allowCreate), unchecked((int)waitForMultipleObjectsTimeout));
 
-                        // After waking, observe shutdown state and bail out so waiters
-                        // do not spin against a drained pool. If WaitAny consumed a
-                        // PoolSemaphore slot, release it back so the accounting stays
-                        // balanced; otherwise the slot would leak and other waiters
-                        // (or callers that arrive after Shutdown completes its own
-                        // Release loop) would starve. CreationSemaphore does NOT need
-                        // compensation here because the outer finally below already
-                        // releases it whenever waitResult == CREATION_HANDLE, and
-                        // that finally runs even on this early return.
-                        if (State is not Running)
-                        {
-                            SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Pool is shutting down; abandoning wait.", Id);
-                            if (waitResult == SEMAPHORE_HANDLE || waitResult == WAIT_ABANDONED + SEMAPHORE_HANDLE)
-                            {
-                                try
-                                {
-                                    _waitHandles.PoolSemaphore.Release(1);
-                                }
-                                catch (SemaphoreFullException)
-                                {
-                                    // Pool semaphore was already saturated by Shutdown's bulk release; safe to ignore.
-                                }
-                            }
-                            Interlocked.Decrement(ref _waitCount);
-                            connection = null;
-                            return false;
-                        }
-
                         // From the WaitAny docs: "If more than one object became signaled during
                         // the call, this is the array index of the signaled object with the
                         // smallest index value of all the signaled objects."  This is important
@@ -1597,37 +1565,17 @@ public void Shutdown()
             }
             State = ShuttingDown;
 
-            // Dispose all background timers so they no longer schedule new work.
-            // Note that any timer callback already in flight may still observe State == ShuttingDown
-            // and short-circuit (see CleanupCallback / ErrorCallback).
+            // Stop maintenance, but let admitted requests finish. Their connections are
+            // destroyed by DeactivateObject when returned to this retired pool.
             Timer cleanup = Interlocked.Exchange(ref _cleanupTimer, null);
             cleanup?.Dispose();
 
-            _errorState.Dispose();
-
-            // Wake any threads parked in WaitHandle.WaitAny by releasing as many semaphore
-            // slots as there are recorded waiters. Using _waitCount (rather than MaxPoolSize)
-            // avoids ArgumentOutOfRangeException when MaxPoolSize == 0 (unlimited) and ensures
-            // we wake every parked waiter even when _waitCount exceeds MaxPoolSize. Waiters
-            // observe State is not Running after wake-up and bail.
-            int waitersToWake = Volatile.Read(ref _waitCount);
-            if (waitersToWake > 0)
-            {
-                try
-                {
-                    _waitHandles.PoolSemaphore.Release(waitersToWake);
-                }
-                catch (SemaphoreFullException)
-                {
-                    // Semaphore already saturated; nothing to do.
-                }
-            }
+            // Keep the cached error and its expiry timer available to admitted waiters.
+            // Disposing the error state here leaves ErrorEvent signaled without an error.
 
-            // Reuse Clear() to doom every connection (including active checked-out ones), drain
-            // both idle stacks, and reclaim emancipated objects. Active connections destroy
-            // themselves on return either via the doom flag or via DeactivateObject's
-            // State == ShuttingDown branch.
-            Clear();
+            // Leave Clear() to the factory's explicit-clear or deferred-pruning path.
+            // Shutdown can run under the pool-group lock, where reclamation and connection
+            // disposal must not be added before the pool is queued for release.
         }
 
         // TransactionEnded merely provides the plumbing for DbConnectionInternal to access the transacted pool

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs

@@ -130,6 +130,34 @@ public void TryGetConnection_WhenFactoryThrows_EntersBlockingPeriod()
         Assert.Equal(1, factory.CreateConnectionCallCount);
     }
 
+    /// <summary>
+    /// Shutdown preserves the cached error for admitted waiters and lets its timer expire.
+    /// </summary>
+    [Fact]
+    public void Shutdown_WhileBlocked_PreservesErrorUntilExpiry()
+    {
+        // Arrange
+        var clock = new FakeTimeProvider();
+        SqlException failure = SqlExceptionHelper.CreateSqlException("server unreachable");
+        var factory = new ConfigurableSqlConnectionFactory(_ => throw failure);
+        var pool = CreatePool(factory, timeProvider: clock);
+        using var owner = new SqlConnection();
+
+        Assert.Throws<SqlException>(() => TryGetConnectionSync(pool, owner, out _));
+
+        // Act
+        pool.Shutdown();
+
+        // Assert
+        Assert.True(pool.ErrorOccurred);
+
+        // Act: expire the preserved blocking period.
+        clock.Advance(TimeSpan.FromSeconds(5));
+
+        // Assert
+        Assert.False(pool.ErrorOccurred);
+    }
+
     /// <summary>
     /// Verifies that once the pool is in the blocking period, a subsequent request fast-fails
     /// with the cached exception without invoking the connection factory again. The first throw