Close remaining ChannelDbConnectionPool parity gaps with WaitHandleDbConnectionPool

Differential testing of the two pool implementations (running the full
Functional/Unit/Manual suites under both, plus a bespoke harness) turned up
four behavioural gaps in ChannelDbConnectionPool that had no test coverage.

1. Emancipated connection reclamation. A SqlConnection that was garbage
   collected without ever being closed or disposed left its internal connection
   permanently occupying a pool slot. At MaxPoolSize this meant every
   subsequent Open timed out forever. GetInternalConnection now sweeps for
   emancipated connections before parking on the idle channel, mirroring
   WaitHandleDbConnectionPool. The sweep is confined to the slow path since it
   is O(MaxPoolSize) and allocates a snapshot.

2. Pool metrics were never emitted. PooledConnections, FreeConnections,
   ActiveConnections and the soft/hard connect and disconnect counters all read
   zero under this pool. Wired up the same call sites the wait handle pool uses.

3. Count reported reservations rather than connections. Reservations include
   connections that are still being opened, which broke the SQL Express user
   instance path in SqlConnectionFactory.CreateConnection (a `pool.Count <= 0`
   check) with a NullReferenceException. Added
   ConnectionPoolSlots.ConnectionCount and pointed Count at it.

4. Async opens always completed asynchronously. The wait handle pool makes a
   non-blocking, non-creating attempt at an idle connection before enqueuing a
   pending open; this pool did not, so OpenAsync against a warm pool always
   took a thread pool hop. Added the same fast path, excluding transactional
   requests so they still go through the transacted store and enlist properly.

Test changes:

- Added ConnectionPoolVersionScope, which flips the pool version switch and
  clears all pools on both entry and exit. Clearing is required because a pool
  binds to its implementation at creation time, so without it pools leak across
  tests.
- Parameterized ReclaimEmancipatedOnOpenTest, MaxPoolWaitForConnectionTest,
  ConnectionResiliencySPIDTest and MetricsTest.PooledConnectionsCounters_Functional
  by pool version. Each was verified to fail before the corresponding fix.
- ChannelDbConnectionPoolTest.StressTestAsync awaited its TaskCompletionSource
  unconditionally, which hangs now that TryGetConnection can complete
  synchronously.
- Three pool-exhaustion unit tests let their owning SqlConnections go out of
  scope, so reclamation could legitimately hand the "should time out" waiter a
  connection. They now keep the owners alive.
- TvpTest.TestPacketNumberWraparound passed an async lambda to
  Task.Factory.StartNew and so awaited a Task<Task>, never observing the inner
  task. Added the missing Unwrap.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: mdaigle

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

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Data.Common;
 using System.Diagnostics;
@@ -209,7 +210,7 @@ public ConcurrentDictionary<
         public SqlConnectionFactory ConnectionFactory { get; }
 
         /// <inheritdoc />
-        public int Count => _connectionSlots.ReservationCount;
+        public int Count => _connectionSlots.ConnectionCount;
 
         /// <inheritdoc />
         public int IdleCount => _idleChannel.Count;
@@ -447,8 +448,23 @@ public DbConnectionInternal ReplaceConnection(
         /// <inheritdoc />
         public void ReturnInternalConnection(DbConnectionInternal connection, DbConnection owningObject)
         {
+            SqlClientDiagnostics.Metrics.SoftDisconnectRequest();
+
             ValidateOwnershipAndSetPoolingState(connection, owningObject);
 
+            DeactivateAndRouteConnection(connection);
+        }
+
+        /// <summary>
+        /// Deactivates a connection that is already marked as owned by the pool (via
+        /// <see cref="DbConnectionInternal.PrePush"/>) and routes it to the idle channel, the
+        /// transacted pool, stasis, or destruction as appropriate. Shared by the normal return path
+        /// and by emancipated connection reclamation, which has already performed the
+        /// <c>PrePush</c> itself and must not re-validate ownership.
+        /// </summary>
+        /// <param name="connection">The connection to deactivate and route.</param>
+        private void DeactivateAndRouteConnection(DbConnectionInternal connection)
+        {
             SqlClientEventSource.Log.TryPoolerTraceEvent(
                 "<prov.DbConnectionPool.DeactivateObject|RES|CPOOL> {0}, Connection {1}, Deactivating.",
                 Id,
@@ -826,6 +842,30 @@ public bool TryGetConnection(
             // processes pending opens on a dedicated non-thread-pool thread.
             Transaction? ambientTransaction = taskCompletionSource.Task.AsyncState as Transaction;
 
+            // Try to satisfy the request synchronously from the idle channel before paying for a
+            // thread pool hop. WaitHandleDbConnectionPool makes the same non-blocking, non-creating
+            // attempt before enqueuing a pending open, so without this an async open against a warm
+            // pool would always complete asynchronously under this pool but synchronously under the
+            // other -- a behavioural difference callers can observe. We deliberately do not try to
+            // *create* a connection here; that can block on the wire and must stay off the caller's
+            // thread.
+            //
+            // Transactional requests are excluded: they must first consult the transacted store for
+            // a connection already enlisted in the same transaction, which only GetInternalConnection
+            // does. Taking a plain idle connection here would both miss that affinity and skip
+            // enlistment.
+            if (!(HasTransactionAffinity && ambientTransaction is not null))
+            {
+                DbConnectionInternal? idleConnection = GetIdleConnection();
+                if (idleConnection is not null)
+                {
+                    PrepareConnection(owningObject, idleConnection);
+                    SqlClientDiagnostics.Metrics.SoftConnectRequest();
+                    connection = idleConnection;
+                    return true;
+                }
+            }
+
             Task.Run(async () =>
             {
                 if (taskCompletionSource.Task.IsCompleted)
@@ -1008,6 +1048,8 @@ _connectionCreationRateLimiter is not null &&
 
                 if (connection is not null)
                 {
+                    SqlClientDiagnostics.Metrics.EnterPooledConnection();
+
                     // A new connection was added to the pool. If we've grown past MinPoolSize,
                     // start the pruning timer so idle connections can be reclaimed.
                     Pruner?.UpdateTimer();
@@ -1113,14 +1155,18 @@ private void RemoveConnection(DbConnectionInternal connection)
                 return;
             }
 
-            _connectionSlots.TryRemove(connection);
+            if (_connectionSlots.TryRemove(connection))
+            {
+                SqlClientDiagnostics.Metrics.ExitPooledConnection();
+            }
 
             // Removing a connection from the pool opens a free slot.
             // Write a null to the idle connection channel to wake up a waiter, who can now open a new
             // connection. Statement order is important since we have synchronous completions on the channel.
             _idleChannel.TryWrite(null);
 
             connection.Dispose();
+            SqlClientDiagnostics.Metrics.HardDisconnectRequest();
 
             // If this removal brought us back to MinPoolSize, disable the pruning timer.
             Pruner?.UpdateTimer();
@@ -1225,6 +1271,18 @@ private async Task<DbConnectionInternal> GetInternalConnection(
                         cancellationToken,
                         timeout);
 
+                    // Before parking on the idle channel (potentially for the full timeout), sweep
+                    // for connections whose owning SqlConnection was garbage collected without ever
+                    // being closed or disposed. Those "emancipated" connections still occupy pool
+                    // slots, so at MaxPoolSize every subsequent request would otherwise time out
+                    // forever. WaitHandleDbConnectionPool performs the same sweep before waiting.
+                    // This is deliberately confined to the slow path: it is O(MaxPoolSize) and
+                    // allocates a snapshot, so it must not run on the hot acquire path.
+                    if (connection is null && ReclaimEmancipatedConnections())
+                    {
+                        connection = GetIdleConnection();
+                    }
+
                     // If we're at max capacity and couldn't open a connection. Block on the idle channel with a
                     // timeout. Note that Channels guarantee fair FIFO behavior to callers of ReadAsync
                     // (first-come, first-served), which is crucial to us.
@@ -1255,9 +1313,73 @@ private async Task<DbConnectionInternal> GetInternalConnection(
             }
 
             PrepareConnection(owningConnection, connection, transaction);
+            SqlClientDiagnostics.Metrics.SoftConnectRequest();
             return connection;
         }
 
+        /// <summary>
+        /// Reclaims connections whose owning <see cref="DbConnection"/> has been garbage collected
+        /// without being closed or disposed. Such connections are still tracked by the pool but can
+        /// never be returned by their owner, so without this sweep they would leak pool slots.
+        /// </summary>
+        /// <returns>True if at least one connection was reclaimed; otherwise, false.</returns>
+        private bool ReclaimEmancipatedConnections()
+        {
+            SqlClientEventSource.Log.TryPoolerTraceEvent(
+                "<prov.DbConnectionPool.ReclaimEmancipatedObjects|RES|CPOOL> {0}", Id);
+
+            List<DbConnectionInternal>? reclaimed = null;
+
+            foreach (DbConnectionInternal connection in _connectionSlots.Snapshot())
+            {
+                // TryEnter rather than Enter: IsEmancipated must be read under the connection lock to
+                // avoid racing PrePush/PostPop, but a connection that is currently locked is being
+                // actively handed out or returned and therefore is not emancipated anyway. Skipping
+                // it keeps this sweep from blocking the caller.
+                bool locked = false;
+                try
+                {
+                    Monitor.TryEnter(connection, ref locked);
+
+                    if (locked && connection.IsEmancipated)
+                    {
+                        // Do as little as possible under the lock: just claim the connection for the
+                        // pool and defer deactivation (which can make server round trips) until the
+                        // lock is released.
+                        connection.PrePush(null);
+                        (reclaimed ??= new List<DbConnectionInternal>()).Add(connection);
+                    }
+                }
+                finally
+                {
+                    if (locked)
+                    {
+                        Monitor.Exit(connection);
+                    }
+                }
+            }
+
+            if (reclaimed is null)
+            {
+                return false;
+            }
+
+            foreach (DbConnectionInternal connection in reclaimed)
+            {
+                SqlClientEventSource.Log.TryPoolerTraceEvent(
+                    "<prov.DbConnectionPool.ReclaimEmancipatedObjects|RES|CPOOL> {0}, Connection {1}, Reclaiming.",
+                    Id,
+                    connection.ObjectID);
+
+                SqlClientDiagnostics.Metrics.ReclaimedConnectionRequest();
+
+                connection.DetachCurrentTransactionIfEnded();
+                DeactivateAndRouteConnection(connection);
+            }
+
+            return true;
+        }
+
         /// <summary>
         /// Performs a blocking synchronous read from the idle connection channel.
         /// </summary>

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;
@@ -60,6 +61,7 @@ internal void Keep()
         private readonly DbConnectionInternal?[] _connections;
         private readonly uint _capacity;
         private volatile int _reservations;
+        private volatile int _connectionCount;
 
         /// <summary>
         /// Constructs a ConnectionPoolSlots instance with the given fixed capacity.
@@ -82,14 +84,23 @@ internal ConnectionPoolSlots(uint fixedCapacity)
 
             _capacity = fixedCapacity;
             _reservations = 0;
+            _connectionCount = 0;
             _connections = new DbConnectionInternal?[fixedCapacity];
         }
 
         /// <summary>
-        /// Gets the total number of reservations currently held.
+        /// Gets the total number of reservations currently held. This includes reservations held on
+        /// behalf of connections that are still being opened and are therefore not yet tracked.
         /// </summary>
         internal int ReservationCount => _reservations;
 
+        /// <summary>
+        /// Gets the number of connections currently tracked by this collection. Unlike
+        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
+        /// still being opened, so it reports connections that actually belong to the pool.
+        /// </summary>
+        internal int ConnectionCount => _connectionCount;
+
         /// <summary>
         /// Adds a connection to the collection.
         /// </summary>
@@ -127,6 +138,7 @@ internal ConnectionPoolSlots(uint fixedCapacity)
                 {
                     if (Interlocked.CompareExchange(ref _connections[i], connection, null) == null)
                     {
+                        Interlocked.Increment(ref _connectionCount);
                         reservation.Keep();
                         return connection;
                     }
@@ -162,6 +174,7 @@ internal bool TryRemove(DbConnectionInternal connection)
             {
                 if (Interlocked.CompareExchange(ref _connections[i], null, connection) == connection)
                 {
+                    Interlocked.Decrement(ref _connectionCount);
                     ReleaseReservation();
                     return true;
                 }
@@ -190,6 +203,28 @@ internal bool TryReplace(DbConnectionInternal oldConnection, DbConnectionInterna
             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/ConnectionPool/IdleConnectionChannel.cs

@@ -56,6 +56,7 @@ internal bool TryWrite(DbConnectionInternal? connection)
                 if (connection is not null)
                 {
                     Interlocked.Increment(ref _count);
+                    SqlClientDiagnostics.Metrics.EnterFreeConnection();
                 }
                 return true;
             }
@@ -74,6 +75,7 @@ internal bool TryRead(out DbConnectionInternal? connection)
                 if (connection is not null)
                 {
                     Interlocked.Decrement(ref _count);
+                    SqlClientDiagnostics.Metrics.ExitFreeConnection();
                 }
 
                 return true;
@@ -93,6 +95,7 @@ internal bool TryRead(out DbConnectionInternal? connection)
             if (connection is not null)
             {
                 Interlocked.Decrement(ref _count);
+                SqlClientDiagnostics.Metrics.ExitFreeConnection();
             }
 
             return connection;

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();
+        }
+    }
+}