Reclaim emancipated connections in ChannelDbConnectionPool

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

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
 using Microsoft.Data.ProviderBase;
@@ -170,6 +171,48 @@ internal bool TryRemove(DbConnectionInternal connection)
             return false;
         }
 
+        /// <summary>
+        /// Atomically replaces an existing connection with a new one in the same slot.
+        /// The reservation count is unchanged because the slot is reused.
+        /// </summary>
+        /// <param name="oldConnection">The connection currently occupying the slot.</param>
+        /// <param name="newConnection">The connection to place into the slot.</param>
+        /// <returns>True if the old connection was found and replaced; otherwise, false.</returns>
+        internal bool TryReplace(DbConnectionInternal oldConnection, DbConnectionInternal newConnection)
+        {
+            for (int i = 0; i < _connections.Length; i++)
+            {
+                if (Interlocked.CompareExchange(ref _connections[i], newConnection, oldConnection) == oldConnection)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        /// <summary>
+        /// Returns a point-in-time snapshot of the connections currently tracked by this collection.
+        /// The snapshot is best-effort: connections may be added or removed while it is being taken,
+        /// so callers must tolerate entries that have since left the pool. Intended for infrequent
+        /// bookkeeping passes (e.g. reclaiming emancipated connections), not for hot paths.
+        /// </summary>
+        internal List<DbConnectionInternal> Snapshot()
+        {
+            List<DbConnectionInternal> snapshot = new(_connections.Length);
+
+            for (int i = 0; i < _connections.Length; i++)
+            {
+                DbConnectionInternal? connection = Volatile.Read(ref _connections[i]);
+                if (connection is not null)
+                {
+                    snapshot.Add(connection);
+                }
+            }
+
+            return snapshot;
+        }
+
         /// <summary>
         /// Attempts to reserve a spot in the collection.
         /// </summary>

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs

@@ -1616,7 +1616,9 @@ public void Open(SqlConnectionOverrides overrides)
                 {
                     statistics = SqlStatistics.StartTimer(Statistics);
 
-                    if (!(IsProviderRetriable ? TryOpenWithRetry(null, false, overrides) : TryOpen(null, false, overrides)))
+                    if (!(IsProviderRetriable ?
+                            TryOpenWithRetry(retry: null, forceNewConnection: false, overrides: overrides) :
+                            TryOpen(retry: null, forceNewConnection: false, overrides: overrides)))
                     {
                         throw ADP.InternalError(ADP.InternalErrorCode.SynchronousConnectReturnedPending);
                     }
@@ -2252,16 +2254,22 @@ private bool TryOpen(TaskCompletionSource<DbConnectionInternal> retry, bool forc
         /// Completes the inner open/replace operation and initializes parser state for the active inner connection.
         /// </summary>
         /// <param name="retry">Retry continuation used by async open paths.</param>
-        /// <param name="forceNewConnection">Provide true to forcibly overwrite the existing connection. Provide false if connecting for the first time.</param>
+        /// <param name="forceNewConnection">Provide <see langword="true"/> to replace the existing inner connection with a freshly established one (for example, during reconnect after a transient fault); provide <see langword="false"/> when opening for the first time.</param>
         /// <returns><see langword="true"/> when open initialization completed synchronously; otherwise <see langword="false"/>.</returns>
         /// <remarks>
         /// The inner connection is snapshotted after the open call so downstream parser access uses a single observed
         /// instance and does not rely on a second racy read of <see cref="InnerConnection"/>.
-        /// 
-        /// forceNewConnection may only be true when the connection is already open (or was open) and needs to be replaced. If the connection has never
-        /// been opened, passing true will result in an exception. It may only be false when the connection has never been opened or is
-        /// currently disconnected. If the connection is currently open, passing false will result in an exception. See SqlConnection state
-        /// transitions and subclasses for more details.
+        /// <para>
+        /// <paramref name="forceNewConnection"/> may be <see langword="true"/> when the connection is currently open, or when
+        /// it was previously opened and is now disconnected (the reconnect case handled by
+        /// <c>DbConnectionClosedPreviouslyOpened</c> and <c>DbConnectionClosedConnecting</c>). Passing <see langword="true"/>
+        /// on a connection that has never been opened will result in an exception.
+        /// </para>
+        /// <para>
+        /// <paramref name="forceNewConnection"/> may be <see langword="false"/> when the connection has never been opened or is
+        /// currently disconnected. Passing <see langword="false"/> on a connection that is already open will result in an
+        /// exception.
+        /// </para>
         /// </remarks>
         internal bool TryOpenInner(TaskCompletionSource<DbConnectionInternal> retry, bool forceNewConnection)
         {

src/Microsoft.Data.SqlClient/src/Resources/Strings.resx

@@ -2157,6 +2157,9 @@
   <data name="SQL_ConnectionPoolNoEmptySlot" xml:space="preserve">
     <value>Could not find an empty slot in the connection pool.</value>
   </data>
+  <data name="SQL_ConnectionPoolReplaceConnectionFailed" xml:space="preserve">
+    <value>Could not replace the connection because it is no longer in the connection pool.</value>
+  </data>
   <data name="SQL_ConnectionPoolShutDown" xml:space="preserve">
     <value>The connection pool has been shut down.</value>
   </data>

src/Microsoft.Data.SqlClient/tests/Common/ConnectionPoolVersionScope.cs

@@ -0,0 +1,62 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace Microsoft.Data.SqlClient.Tests.Common;
+
+/// <summary>
+/// Selects the connection pool implementation (<c>WaitHandleDbConnectionPool</c> or
+/// <c>ChannelDbConnectionPool</c>) for the duration of a test.
+///
+/// A pool is bound to an implementation when it is created, so simply flipping the
+/// <c>UseConnectionPoolV2</c> switch is not enough: pools created before the switch was flipped
+/// keep their original implementation, and pools created inside the scope would otherwise outlive
+/// it and leak the chosen implementation into unrelated tests. This scope therefore clears all
+/// pools both on entry and on exit.
+///
+/// This follows the RAII pattern; construct it at the start of a test and dispose it at the end.
+/// Like <see cref="LocalAppContextSwitchesHelper"/>, it manipulates global state and enforces a
+/// single-instance policy, so it must not be held for longer than necessary.
+/// </summary>
+public sealed class ConnectionPoolVersionScope : IDisposable
+{
+    private readonly LocalAppContextSwitchesHelper _switches;
+
+    /// <summary>
+    /// Clears all existing pools and selects the requested pool implementation.
+    /// </summary>
+    /// <param name="usePoolV2">
+    /// True to use <c>ChannelDbConnectionPool</c>; false to use <c>WaitHandleDbConnectionPool</c>.
+    /// </param>
+    public ConnectionPoolVersionScope(bool usePoolV2)
+    {
+        _switches = new LocalAppContextSwitchesHelper();
+
+        try
+        {
+            SqlConnection.ClearAllPools();
+            _switches.UseConnectionPoolV2 = usePoolV2;
+        }
+        catch
+        {
+            _switches.Dispose();
+            throw;
+        }
+    }
+
+    /// <summary>
+    /// Clears all pools created under the selected implementation and restores the original
+    /// switch values.
+    /// </summary>
+    public void Dispose()
+    {
+        try
+        {
+            SqlConnection.ClearAllPools();
+        }
+        finally
+        {
+            _switches.Dispose();
+        }
+    }
+}

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs

@@ -150,8 +150,7 @@ public static void AccessTokenConnectionPoolingTest()
         [ClassData(typeof(ConnectionPoolConnectionStringAndPoolVersionProvider))]
         public static void ClearAllPoolsTest(string connectionString, bool usePoolV2)
         {
-            using LocalAppContextSwitchesHelper switchesHelper = new();
-            switchesHelper.UseConnectionPoolV2 = usePoolV2;
+            using ConnectionPoolVersionScope poolVersion = new(usePoolV2);
 
             SqlConnection.ClearAllPools();
             Assert.True(0 == ConnectionPoolWrapper.AllConnectionPools().Length, "Pools exist after clearing all pools");
@@ -178,9 +177,11 @@ public static void ClearAllPoolsTest(string connectionString, bool usePoolV2)
         /// NOTE: 'emancipated' means that the internal connection's SqlConnection has fallen out of scope and has no references, but was not explicitly disposed\closed
         /// </summary>
         [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
-        [ClassData(typeof(ConnectionPoolConnectionStringProvider))]
-        public static void ReclaimEmancipatedOnOpenTest(string connectionString)
+        [ClassData(typeof(ConnectionPoolConnectionStringAndPoolVersionProvider))]
+        public static void ReclaimEmancipatedOnOpenTest(string connectionString, bool usePoolV2)
         {
+            using ConnectionPoolVersionScope poolVersion = new(usePoolV2);
+
             string newConnectionString = (new SqlConnectionStringBuilder(connectionString) { MaxPoolSize = 1 }).ConnectionString;
             SqlConnection.ClearAllPools();
 
@@ -205,9 +206,11 @@ public static void ReclaimEmancipatedOnOpenTest(string connectionString)
         /// Tests if, when max pool size is reached, Open() will block until a connection becomes available
         /// </summary>
         [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
-        [ClassData(typeof(ConnectionPoolConnectionStringProvider))]
-        public static void MaxPoolWaitForConnectionTest(string connectionString)
+        [ClassData(typeof(ConnectionPoolConnectionStringAndPoolVersionProvider))]
+        public static void MaxPoolWaitForConnectionTest(string connectionString, bool usePoolV2)
         {
+            using ConnectionPoolVersionScope poolVersion = new(usePoolV2);
+
             string newConnectionString = (new SqlConnectionStringBuilder(connectionString) { MaxPoolSize = 1 }).ConnectionString;
             SqlConnection.ClearAllPools();