From d245dd4b20847ea5d238c525686f27e0fb1be475 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 13 Apr 2026 15:23:56 -0700 Subject: [PATCH 01/10] Implement clear --- .../001-pool-clear/checklists/requirements.md | 37 +++ specs/001-pool-clear/spec.md | 112 +++++++++ .../Data/ProviderBase/DbConnectionInternal.cs | 11 + .../ConnectionPool/ChannelDbConnectionPool.cs | 71 +++++- .../WaitHandleDbConnectionPool.cs | 2 - .../Data/SqlClient/SqlConnectionFactory.cs | 6 +- .../ChannelDbConnectionPoolTest.cs | 225 +++++++++++++++++- 7 files changed, 448 insertions(+), 16 deletions(-) create mode 100644 specs/001-pool-clear/checklists/requirements.md create mode 100644 specs/001-pool-clear/spec.md diff --git a/specs/001-pool-clear/checklists/requirements.md b/specs/001-pool-clear/checklists/requirements.md new file mode 100644 index 0000000000..915b707f1d --- /dev/null +++ b/specs/001-pool-clear/checklists/requirements.md @@ -0,0 +1,37 @@ +# Specification Quality Checklist: Pool Clear + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-04-10 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass validation. The spec correctly separates Phase 1 (basic clear) from Phase 2 (transaction-aware clear) via assumptions. +- Generation counter is mentioned as a Key Entity (what), not as implementation detail (how). The concept is domain-level — it describes the invalidation mechanism in user terms. +- SC-004 references "pool v1/v2" which is borderline implementation detail but is user-facing since pool selection is via AppContext switch. Accepted. +- Ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/001-pool-clear/spec.md b/specs/001-pool-clear/spec.md new file mode 100644 index 0000000000..bfea5e80fc --- /dev/null +++ b/specs/001-pool-clear/spec.md @@ -0,0 +1,112 @@ +# Feature Specification: Pool Clear + +**Feature Branch**: `dev/mdaigle/connection-pool-designs` +**Created**: 2026-04-10 +**Status**: Draft +**Input**: User description: "Implement Clear() in ChannelDbConnectionPool so that SqlConnection.ClearPool() and SqlConnection.ClearAllPools() work with pool v2, using a generation counter approach to lazily invalidate connections." + +## User Scenarios & Testing + +### User Story 1 - Clear a Specific Connection Pool (Priority: P1) + +As an application developer, I want to call `SqlConnection.ClearPool(connection)` to invalidate all connections in a specific pool, so that the next connection request creates a fresh physical connection (e.g., after a failover, credential rotation, or configuration change on the server). + +**Why this priority**: This is the primary use case for pool clearing — an application detects a problem with existing connections and needs to force the pool to start fresh. Without this, applications have no way to recover from server-side changes when using pool v2. + +**Independent Test**: Can be tested by opening connections, calling `ClearPool`, then verifying that subsequent connection requests do not reuse pre-clear connections. + +**Acceptance Scenarios**: + +1. **Given** a pool with 10 idle connections, **When** `ClearPool` is called, **Then** all idle connections are closed and new connection requests create fresh physical connections. +2. **Given** a pool with 5 busy (in-use) connections and 5 idle connections, **When** `ClearPool` is called, **Then** idle connections are closed immediately and busy connections are destroyed when they are returned to the pool (not while in use). +3. **Given** a pool that has been cleared, **When** a previously-busy connection is returned, **Then** the connection is destroyed rather than returned to the idle channel. + +--- + +### User Story 2 - Clear All Connection Pools (Priority: P1) + +As an application developer, I want to call `SqlConnection.ClearAllPools()` to invalidate connections across all pools in the application, so that I can recover from broad infrastructure changes (e.g., DNS migration, certificate rollover). + +**Why this priority**: This is equally critical to single-pool clear — it's the same mechanism applied globally. Both entry points must work. + +**Independent Test**: Can be tested by creating connections to multiple connection strings, calling `ClearAllPools`, and verifying all pools produce fresh connections. + +**Acceptance Scenarios**: + +1. **Given** multiple pools each with idle connections, **When** `ClearAllPools` is called, **Then** all idle connections across all pools are closed. +2. **Given** multiple pools, **When** `ClearAllPools` is called, **Then** busy connections in all pools are destroyed when returned. + +--- + +### User Story 3 - Lazy Invalidation of Busy Connections (Priority: P2) + +As the pool infrastructure, I need busy connections that were opened before a clear to be destroyed when they are returned to the pool, so that clearing does not interrupt active operations but still ensures all pre-clear connections are eventually removed. + +**Why this priority**: This ensures clear is non-disruptive to in-flight operations. Without lazy invalidation, either busy connections leak or active queries are interrupted. + +**Independent Test**: Can be tested by opening a connection, calling `ClearPool`, executing a query on the open connection (should succeed), closing the connection, then verifying the pool destroyed it rather than reusing it. + +**Acceptance Scenarios**: + +1. **Given** a connection opened before a pool clear, **When** the connection is used after `ClearPool`, **Then** the connection continues to work normally. +2. **Given** a connection opened before a pool clear, **When** the connection is returned to the pool, **Then** the pool detects that it predates the clear and destroys it. +3. **Given** a connection opened after a pool clear, **When** the connection is returned to the pool, **Then** it is returned to the idle channel normally. + +--- + +### User Story 4 - Multiple Consecutive Clears (Priority: P3) + +As an application developer, I want multiple rapid calls to `ClearPool` to behave correctly, so that retry logic or concurrent failover detection doesn't corrupt pool state. + +**Why this priority**: Correctness concern — concurrent or rapid clears must not cause double-frees, missed connections, or counter overflow. + +**Independent Test**: Can be tested by calling `ClearPool` multiple times in rapid succession and verifying no exceptions, no connection leaks, and correct pool behavior afterward. + +**Acceptance Scenarios**: + +1. **Given** a pool with connections, **When** `ClearPool` is called twice rapidly, **Then** both calls complete without error, and only connections predating both clears are invalidated. +2. **Given** a pool where connections are being opened concurrently with a clear, **When** ClearPool is called, **Then** connections opened after the clear are not invalidated. + +--- + +### Edge Cases + +- What happens if `ClearPool` is called on an empty pool? The generation counter increments but no connections are closed. Subsequent connections are fresh. +- What happens if `ClearPool` is called during pool shutdown? The clear should be a no-op or complete harmlessly — shutdown already destroys all connections. +- What happens if a connection is returned to the pool between the generation increment and the idle channel drain? The connection is caught by the generation check in `ReturnInternalConnection` or on next retrieval from the idle channel. +- What happens with many clears causing generation counter overflow? With `int` counter, overflow at 2^31. Even at 1 clear/second, this is 68 years. Overflow is not a practical concern. If it wraps, the worst case is one stale connection survives a single retrieval cycle. + +## Requirements + +### Functional Requirements + +- **FR-001**: System MUST implement a generation counter that increments atomically on each `Clear()` call. +- **FR-002**: System MUST stamp each new connection with the current pool generation at creation time. +- **FR-003**: System MUST reject connections whose generation does not match the current pool generation when they are retrieved from the idle channel or returned to the pool. +- **FR-004**: System MUST drain all idle connections from the channel on `Clear()`, closing each one. +- **FR-005**: System MUST NOT interrupt busy (in-use) connections during a clear — busy connections are destroyed lazily when returned. +- **FR-006**: System MUST allow connections opened after a clear to be pooled normally. +- **FR-007**: System MUST support concurrent calls to `Clear()` without corrupting pool state. +- **FR-008**: System MUST integrate with the existing `SqlConnection.ClearPool()` and `SqlConnection.ClearAllPools()` call paths. + +### Key Entities + +- **Pool Generation Counter (`_clearGeneration`)**: A pool-level `volatile int` incremented atomically via `Interlocked.Increment` on each `Clear()` call. Represents the current "epoch" of the pool. +- **Connection Generation (`PoolGeneration`)**: A property on `DbConnectionInternal` stamped when the connection is created or added to the pool. Used to compare against the pool's current generation. +- **Stale Connection**: A connection whose `PoolGeneration` does not match the pool's `_clearGeneration`. Stale connections are destroyed rather than returned to the idle channel. + +## Success Criteria + +### Measurable Outcomes + +- **SC-001**: After `ClearPool` is called, 100% of subsequent connection acquisitions produce fresh physical connections (no pre-clear connections reused). +- **SC-002**: Busy connections continue to operate normally during and after a pool clear — no active queries are interrupted. +- **SC-003**: Pool correctly handles at least 1000 rapid consecutive `Clear()` calls without exceptions, connection leaks, or state corruption. +- **SC-004**: `ClearPool` and `ClearAllPools` work identically whether using pool v1 (WaitHandle) or pool v2 (Channel), from the caller's perspective. + +## Assumptions + +- The generation counter approach is preferred over the WaitHandle pool's `DoNotPoolThisConnection()` mark-all pattern because `ConnectionPoolSlots` is not iterable by design (CAS-based slot array). +- `DbConnectionInternal` can accommodate a new `PoolGeneration` property without breaking existing functionality or requiring changes to the WaitHandle pool. +- The existing `SqlConnection.ClearPool()` → `SqlConnectionFactory.ClearPool()` → `IDbConnectionPool.Clear()` call chain is already wired up and only requires the `ChannelDbConnectionPool.Clear()` implementation. +- Transacted connections with stale generations are handled separately as part of the transactions feature and are out of scope for Phase 1 of pool clear. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index b4426bc4d6..d7c5fd97d8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -92,6 +92,17 @@ internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool all /// internal DateTime CreateTime { get; } + /// + /// The pool generation at the time this connection was created or added to the pool. + /// Used by to detect stale connections after a pool clear. + /// + /// + /// Not safe, should only be set by the connection pool. + /// + // TODO: Ideally this would be readonly and set in the constructor. Piping the value all the way through the connection factory is too complicated to be worth it. + // If we can expose the constructor to the connection pool in the future, it can be set at in the constructor. + internal int PoolGeneration { get; set; } + internal bool AllowSetConnectionString { get; } internal bool CanBePooled => !IsConnectionDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out _); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 553e6cd395..1bb8771f25 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -78,6 +78,20 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool /// private readonly ChannelReader _idleConnectionReader; private readonly ChannelWriter _idleConnectionWriter; + + /// + /// The current generation of the pool. Incremented atomically on each call. + /// Connections stamped with a generation that does not match are considered stale and are destroyed + /// rather than returned to the idle channel. + /// + private volatile int _clearCounter; + + /// + /// Guard to prevent concurrent operations from draining the idle channel + /// simultaneously. The generation counter is still incremented by every caller so stale connections + /// are always caught lazily, but only one thread performs the actual drain. + /// + private volatile int _isClearing; #endregion /// @@ -162,7 +176,43 @@ public ConcurrentDictionary< /// public void Clear() { - throw new NotImplementedException(); + SqlClientEventSource.Log.TryPoolerTraceEvent( + " {0}, Clearing.", Id); + + Interlocked.Increment(ref _clearCounter); + + // If another thread is already draining, skip the drain. The generation counter has + // already been incremented, so stale connections will still be caught lazily by + // IsLiveConnection on their next retrieval or return. + if (Interlocked.CompareExchange(ref _isClearing, 1, 0) == 1) + { + SqlClientEventSource.Log.TryPoolerTraceEvent( + " {0}, Skip drain, already clearing.", Id); + return; + } + + try + { + // Drain idle connections from the channel and destroy them. Limit iterations to + // the current pool count to prevent an unbounded loop if connections are + // concurrently returned to the channel during the drain. + int numToDrain = Count; + while (numToDrain > 0 && _idleConnectionReader.TryRead(out DbConnectionInternal? connection)) + { + if (connection is not null) + { + RemoveConnection(connection); + numToDrain--; + } + } + } + finally + { + _isClearing = 0; + } + + SqlClientEventSource.Log.TryPoolerTraceEvent( + " {0}, Cleared.", Id); } /// @@ -353,12 +403,17 @@ public bool TryGetConnection( // DbConnectionInternal doesn't support an async open. It's better to block this thread and keep // throughput high than to queue all of our opens onto a single worker thread. Add an async path // when this support is added to DbConnectionInternal. - return ConnectionFactory.CreatePooledConnection( + var connection = ConnectionFactory.CreatePooledConnection( owningConnection, this, - PoolGroup.PoolKey, - PoolGroup.ConnectionOptions, userOptions); + + if (connection is not null) + { + connection.PoolGeneration = _clearCounter; + } + + return connection; }, cleanupCallback: (newConnection) => { @@ -376,16 +431,24 @@ public bool TryGetConnection( /// Returns true if the connection is live and unexpired, otherwise returns false. private bool IsLiveConnection(DbConnectionInternal connection) { + // Broken physical connection if (!connection.IsConnectionAlive()) { return false; } + // Connection has been alive longer than the load balance timeout if (LoadBalanceTimeout != TimeSpan.Zero && DateTime.UtcNow > connection.CreateTime + LoadBalanceTimeout) { return false; } + // Connection was created before the last Clear, so it's stale. + if (connection.PoolGeneration != _clearCounter) + { + return false; + } + return true; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 8d2311b5cd..12bff3b598 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -528,8 +528,6 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio newObj = _connectionFactory.CreatePooledConnection( owningObject, this, - _connectionPoolGroup.PoolKey, - _connectionPoolGroup.ConnectionOptions, userOptions); lock (_objectList) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 89be9767d8..8d49f4dc30 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -141,15 +141,13 @@ internal DbConnectionInternal CreateNonPooledConnection( internal DbConnectionInternal CreatePooledConnection( DbConnection owningConnection, IDbConnectionPool pool, - DbConnectionPoolKey poolKey, - DbConnectionOptions options, DbConnectionOptions userOptions) { Debug.Assert(pool != null, "null pool?"); DbConnectionInternal newConnection = CreateConnection( - options, - poolKey, // @TODO: is pool.PoolGroup.Key the same thing? + pool.PoolGroup.ConnectionOptions, + pool.PoolGroup.PoolKey, pool.PoolGroup.ProviderInfo, pool, owningConnection, diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 219bb1fdf6..e7ad8242e3 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -681,12 +681,7 @@ public void TestUseLoadBalancing() #region Not Implemented Method Tests - [Fact] - public void TestClear() - { - var pool = ConstructPool(SuccessfulConnectionFactory); - Assert.Throws(() => pool.Clear()); - } + [Fact] public void TestPutObjectFromTransactedPool() @@ -724,6 +719,224 @@ public void TestTransactionEnded() } #endregion + #region Pool Clear Tests + + + [Fact] + public void Clear_MultipleIdleConnections_AllAreDestroyed() + { + // Arrange + int numConnections = 5; + var pool = ConstructPool(SuccessfulConnectionFactory); + var owningConnections = new SqlConnection[numConnections]; + var internalConnections = new DbConnectionInternal?[numConnections]; + + for (int i = 0; i < numConnections; i++) + { + owningConnections[i] = new SqlConnection(); + pool.TryGetConnection( + owningConnections[i], + taskCompletionSource: null, + new DbConnectionOptions("", null), + out internalConnections[i] + ); + Assert.Equal(0, internalConnections[i]!.PoolGeneration); + } + + // Return all connections to the pool + for (int i = 0; i < numConnections; i++) + { + pool.ReturnInternalConnection(internalConnections[i]!, owningConnections[i]); + } + + // Act + pool.Clear(); + + // Assert + Assert.Equal(0, pool.Count); + } + + [Fact] + public void Clear_BusyConnections_NotDestroyedImmediately() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? busyConnection + ); + Assert.NotNull(busyConnection); + Assert.Equal(0, busyConnection.PoolGeneration); + + // Act - Clear while connection is still busy + pool.Clear(); + + // Assert - Busy connection is still tracked in the pool and retains its old generation + Assert.Equal(1, pool.Count); + Assert.Equal(0, busyConnection.PoolGeneration); + } + + [Fact] + public void Clear_BusyConnectionReturned_IsDestroyed() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? busyConnection + ); + Assert.NotNull(busyConnection); + Assert.Equal(0, busyConnection.PoolGeneration); + + // Act - Clear, then return the busy connection + pool.Clear(); + + // Assert - Busy connection is still tracked but has stale generation + Assert.Equal(1, pool.Count); + + // Act - Return the busy connection + pool.ReturnInternalConnection(busyConnection, owningConnection); + + // Assert - The connection should have been destroyed on return (generation mismatch) + Assert.Equal(0, pool.Count); + } + + [Fact] + public void Clear_MixedBusyAndIdle_OnlyIdleDestroyedImmediately() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection busyOwner = new(); + SqlConnection idleOwner = new(); + + pool.TryGetConnection( + busyOwner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? busyConnection + ); + pool.TryGetConnection( + idleOwner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? idleConnection + ); + Assert.NotNull(busyConnection); + Assert.NotNull(idleConnection); + Assert.Equal(0, busyConnection.PoolGeneration); + Assert.Equal(0, idleConnection.PoolGeneration); + + // Return only the idle connection + pool.ReturnInternalConnection(idleConnection, idleOwner); + + // Act + pool.Clear(); + + // Assert - Only the busy connection remains with stale generation + Assert.Equal(1, pool.Count); + Assert.Equal(0, busyConnection.PoolGeneration); + + // Now return the busy connection - it should be destroyed (generation 0 != pool generation 1) + pool.ReturnInternalConnection(busyConnection, busyOwner); + Assert.Equal(0, pool.Count); + } + + [Fact] + public void Clear_NewConnectionsAfterClear_ArePooledNormally() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? oldConnection + ); + Assert.Equal(0, oldConnection!.PoolGeneration); + pool.ReturnInternalConnection(oldConnection, owningConnection); + + // Act + pool.Clear(); + + // Get a new connection after clear + SqlConnection newOwner = new(); + pool.TryGetConnection( + newOwner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? newConnection + ); + Assert.NotNull(newConnection); + + // The new connection should be different from the old one and have generation 1 + Assert.NotSame(oldConnection, newConnection); + Assert.Equal(1, newConnection.PoolGeneration); + + // Return the new connection - it should be pooled normally + pool.ReturnInternalConnection(newConnection, newOwner); + Assert.Equal(1, pool.Count); + + // Get another connection - it should reuse the post-clear connection (same generation) + SqlConnection reuseOwner = new(); + pool.TryGetConnection( + reuseOwner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? reusedConnection + ); + Assert.Same(newConnection, reusedConnection); + Assert.Equal(1, reusedConnection!.PoolGeneration); + } + + [Fact] + public void Clear_MultipleClearCalls_DoNotCorruptState() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? connection + ); + Assert.Equal(0, connection!.PoolGeneration); + pool.ReturnInternalConnection(connection, owningConnection); + + // Act - Call clear multiple times rapidly + pool.Clear(); + pool.Clear(); + pool.Clear(); + + // Assert - Pool state is still valid + Assert.Equal(0, pool.Count); + + // New connections should have generation 3 (incremented three times) + SqlConnection newOwner = new(); + pool.TryGetConnection( + newOwner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? newConnection + ); + Assert.NotNull(newConnection); + Assert.Equal(1, pool.Count); + Assert.Equal(3, newConnection.PoolGeneration); + } + + #endregion + #region Test classes internal class SuccessfulSqlConnectionFactory : SqlConnectionFactory { From e3b1b56bd01b5fe414eb212ea317f0411460eb84 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 13 Apr 2026 15:43:57 -0700 Subject: [PATCH 02/10] Hook up clearallpools testing --- specs/001-pool-clear/spec.md | 3 +-- .../ConnectionPool/ChannelDbConnectionPool.cs | 3 ++- .../ConnectionPool/DbConnectionPoolGroup.cs | 2 +- .../ConnectionPoolHelper.cs | 20 ++++++++++++-- .../ConnectionPoolTest/ConnectionPoolTest.cs | 27 +++++++++++++++++-- .../ChannelDbConnectionPoolTest.cs | 17 +++++++----- 6 files changed, 57 insertions(+), 15 deletions(-) diff --git a/specs/001-pool-clear/spec.md b/specs/001-pool-clear/spec.md index bfea5e80fc..5c5d5f3a2b 100644 --- a/specs/001-pool-clear/spec.md +++ b/specs/001-pool-clear/spec.md @@ -101,8 +101,7 @@ As an application developer, I want multiple rapid calls to `ClearPool` to behav - **SC-001**: After `ClearPool` is called, 100% of subsequent connection acquisitions produce fresh physical connections (no pre-clear connections reused). - **SC-002**: Busy connections continue to operate normally during and after a pool clear — no active queries are interrupted. -- **SC-003**: Pool correctly handles at least 1000 rapid consecutive `Clear()` calls without exceptions, connection leaks, or state corruption. -- **SC-004**: `ClearPool` and `ClearAllPools` work identically whether using pool v1 (WaitHandle) or pool v2 (Channel), from the caller's perspective. +- **SC-003**: `ClearPool` and `ClearAllPools` work identically whether using pool v1 (WaitHandle) or pool v2 (Channel), from the caller's perspective. ## Assumptions diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 1bb8771f25..0238590da2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -201,6 +201,7 @@ public void Clear() { if (connection is not null) { + //TODO: should we check pool generation of the connection here? RemoveConnection(connection); numToDrain--; } @@ -269,7 +270,7 @@ public void Shutdown() /// public void Startup() { - throw new NotImplementedException(); + // No-op for now, warmup will be implemented later. } /// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs index 786f91684b..13af25b684 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs @@ -179,7 +179,7 @@ internal IDbConnectionPool GetConnectionPool(SqlConnectionFactory connectionFact IDbConnectionPool newPool; if (LocalAppContextSwitches.UseConnectionPoolV2) { - throw new NotImplementedException(); + newPool = new ChannelDbConnectionPool(connectionFactory, this, currentIdentity, connectionPoolProviderInfo); } else { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs index d447720a1a..f51aaa6a37 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs @@ -16,13 +16,15 @@ internal static class ConnectionPoolHelper private static Assembly s_MicrosoftDotData = Assembly.Load(new AssemblyName(typeof(SqlConnection).GetTypeInfo().Assembly.FullName)); private static Type s_dbConnectionPool = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.IDbConnectionPool"); private static Type s_waitHandleDbConnectionPool = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.WaitHandleDbConnectionPool"); + private static Type s_channelDbConnectionPool = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.ChannelDbConnectionPool"); private static Type s_dbConnectionPoolGroup = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolGroup"); private static Type s_dbConnectionPoolIdentity = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolIdentity"); private static Type s_sqlConnectionFactory = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.SqlConnectionFactory"); private static Type s_dbConnectionPoolKey = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolKey"); private static Type s_dictStringPoolGroup = typeof(Dictionary<,>).MakeGenericType(s_dbConnectionPoolKey, s_dbConnectionPoolGroup); private static Type s_dictPoolIdentityPool = typeof(ConcurrentDictionary<,>).MakeGenericType(s_dbConnectionPoolIdentity, s_dbConnectionPool); - private static PropertyInfo s_dbConnectionPoolCount = s_waitHandleDbConnectionPool.GetProperty("Count", BindingFlags.Instance | BindingFlags.Public); + // Resolve Count from the interface so it works with both pool implementations + private static PropertyInfo s_dbConnectionPoolCount = s_dbConnectionPool.GetProperty("Count", BindingFlags.Instance | BindingFlags.Public); private static PropertyInfo s_dictStringPoolGroupGetKeys = s_dictStringPoolGroup.GetProperty("Keys"); private static PropertyInfo s_dictPoolIdentityPoolValues = s_dictPoolIdentityPool.GetProperty("Values"); private static PropertyInfo s_sqlConnectionFactorySingleton = s_sqlConnectionFactory.GetProperty("Instance", BindingFlags.Static | BindingFlags.NonPublic); @@ -37,6 +39,13 @@ public static int CountFreeConnections(object pool) { VerifyObjectIsPool(pool); + if (s_channelDbConnectionPool.IsInstanceOfType(pool)) + { + // ChannelDbConnectionPool doesn't have separate stacks; + // Count represents idle connections available in the channel. + return (int)s_dbConnectionPoolCount.GetValue(pool, null); + } + ICollection oldStack = (ICollection)s_dbConnectionPoolStackOld.GetValue(pool); ICollection newStack = (ICollection)s_dbConnectionPoolStackNew.GetValue(pool); @@ -107,10 +116,17 @@ public static object ConnectionPoolFromString(string connectionString) /// /// Causes the cleanup timer code in the connection pool to be invoked /// - /// A connection pool object + /// A connection pool object internal static void CleanConnectionPool(object pool) { VerifyObjectIsPool(pool); + + if (s_channelDbConnectionPool.IsInstanceOfType(pool)) + { + // ChannelDbConnectionPool does not have a cleanup timer callback. + return; + } + s_dbConnectionPoolCleanup.Invoke(pool, new object[] { null }); } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index 10c3939774..491316c56d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using Microsoft.Data.SqlClient.Tests.Common; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests @@ -28,6 +29,25 @@ public IEnumerator GetEnumerator() IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } + public class ConnectionPoolConnectionStringAndPoolVersionProvider : IEnumerable + { + private static readonly string _TCPConnectionString = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = false, Pooling = true }).ConnectionString; + private static readonly string _tcpMarsConnStr = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = true, Pooling = true }).ConnectionString; + + public IEnumerator GetEnumerator() + { + yield return new object[] { _TCPConnectionString, false }; + yield return new object[] { _TCPConnectionString, true }; + if (DataTestUtility.IsNotAzureSynapse()) + { + yield return new object[] { _tcpMarsConnStr, false }; + yield return new object[] { _tcpMarsConnStr, true }; + } + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + // TODO Synapse: Fix these tests for Azure Synapse. public static class ConnectionPoolTest { @@ -118,9 +138,12 @@ public static void AccessTokenConnectionPoolingTest() /// Tests if clearing all of the pools does actually remove the pools /// [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] - [ClassData(typeof(ConnectionPoolConnectionStringProvider))] - public static void ClearAllPoolsTest(string connectionString) + [ClassData(typeof(ConnectionPoolConnectionStringAndPoolVersionProvider))] + public static void ClearAllPoolsTest(string connectionString, bool usePoolV2) { + using LocalAppContextSwitchesHelper switchesHelper = new(); + switchesHelper.UseConnectionPoolV2 = usePoolV2; + SqlConnection.ClearAllPools(); Assert.True(0 == ConnectionPoolWrapper.AllConnectionPools().Length, "Pools exist after clearing all pools"); diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index e7ad8242e3..b7e45ddc3a 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -704,13 +704,6 @@ public void TestShutdown() Assert.Throws(() => pool.Shutdown()); } - [Fact] - public void TestStartup() - { - var pool = ConstructPool(SuccessfulConnectionFactory); - Assert.Throws(() => pool.Startup()); - } - [Fact] public void TestTransactionEnded() { @@ -721,6 +714,16 @@ public void TestTransactionEnded() #region Pool Clear Tests + [Fact] + public void Clear_EmptyPool_DoesNotThrow() + { + // Arrange + var pool = ConstructPool(SuccessfulConnectionFactory); + + // Act & Assert - Should complete without error + pool.Clear(); + Assert.Equal(0, pool.Count); + } [Fact] public void Clear_MultipleIdleConnections_AllAreDestroyed() From 1b1b003eab35a1043a409856af889783681ef14f Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 14 Apr 2026 12:58:35 -0700 Subject: [PATCH 03/10] Rename generation property. --- specs/001-pool-clear/spec.md | 34 ++++++------------- .../Data/ProviderBase/DbConnectionInternal.cs | 2 +- .../ConnectionPool/ChannelDbConnectionPool.cs | 4 +-- .../ChannelDbConnectionPoolTest.cs | 24 ++++++------- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/specs/001-pool-clear/spec.md b/specs/001-pool-clear/spec.md index 5c5d5f3a2b..dc1d60b9d6 100644 --- a/specs/001-pool-clear/spec.md +++ b/specs/001-pool-clear/spec.md @@ -7,65 +7,54 @@ ## User Scenarios & Testing -### User Story 1 - Clear a Specific Connection Pool (Priority: P1) +### User Story 1 - Clear a Specific Connection Pool As an application developer, I want to call `SqlConnection.ClearPool(connection)` to invalidate all connections in a specific pool, so that the next connection request creates a fresh physical connection (e.g., after a failover, credential rotation, or configuration change on the server). -**Why this priority**: This is the primary use case for pool clearing — an application detects a problem with existing connections and needs to force the pool to start fresh. Without this, applications have no way to recover from server-side changes when using pool v2. - **Independent Test**: Can be tested by opening connections, calling `ClearPool`, then verifying that subsequent connection requests do not reuse pre-clear connections. **Acceptance Scenarios**: 1. **Given** a pool with 10 idle connections, **When** `ClearPool` is called, **Then** all idle connections are closed and new connection requests create fresh physical connections. -2. **Given** a pool with 5 busy (in-use) connections and 5 idle connections, **When** `ClearPool` is called, **Then** idle connections are closed immediately and busy connections are destroyed when they are returned to the pool (not while in use). -3. **Given** a pool that has been cleared, **When** a previously-busy connection is returned, **Then** the connection is destroyed rather than returned to the idle channel. +2. **Given** a pool with 5 busy (in-use) connections and 5 idle connections, **When** `ClearPool` is called, **Then** idle connections are closed immediately. --- -### User Story 2 - Clear All Connection Pools (Priority: P1) +### User Story 2 - Clear All Connection Pools As an application developer, I want to call `SqlConnection.ClearAllPools()` to invalidate connections across all pools in the application, so that I can recover from broad infrastructure changes (e.g., DNS migration, certificate rollover). -**Why this priority**: This is equally critical to single-pool clear — it's the same mechanism applied globally. Both entry points must work. - **Independent Test**: Can be tested by creating connections to multiple connection strings, calling `ClearAllPools`, and verifying all pools produce fresh connections. **Acceptance Scenarios**: 1. **Given** multiple pools each with idle connections, **When** `ClearAllPools` is called, **Then** all idle connections across all pools are closed. -2. **Given** multiple pools, **When** `ClearAllPools` is called, **Then** busy connections in all pools are destroyed when returned. --- -### User Story 3 - Lazy Invalidation of Busy Connections (Priority: P2) +### User Story 3 - Lazy Invalidation of Busy Connections As the pool infrastructure, I need busy connections that were opened before a clear to be destroyed when they are returned to the pool, so that clearing does not interrupt active operations but still ensures all pre-clear connections are eventually removed. -**Why this priority**: This ensures clear is non-disruptive to in-flight operations. Without lazy invalidation, either busy connections leak or active queries are interrupted. - **Independent Test**: Can be tested by opening a connection, calling `ClearPool`, executing a query on the open connection (should succeed), closing the connection, then verifying the pool destroyed it rather than reusing it. **Acceptance Scenarios**: -1. **Given** a connection opened before a pool clear, **When** the connection is used after `ClearPool`, **Then** the connection continues to work normally. -2. **Given** a connection opened before a pool clear, **When** the connection is returned to the pool, **Then** the pool detects that it predates the clear and destroys it. +1. **Given** a connection opened and in use before a clear, **When** `ClearPool` is called, **Then** the connection continues to work normally. +2. **Given** a connection opened and in use before a pool clear, **When** the connection is returned to the pool, **Then** the pool detects that it predates the clear and destroys it. 3. **Given** a connection opened after a pool clear, **When** the connection is returned to the pool, **Then** it is returned to the idle channel normally. --- -### User Story 4 - Multiple Consecutive Clears (Priority: P3) - -As an application developer, I want multiple rapid calls to `ClearPool` to behave correctly, so that retry logic or concurrent failover detection doesn't corrupt pool state. +### User Story 4 - Multiple Consecutive Clears -**Why this priority**: Correctness concern — concurrent or rapid clears must not cause double-frees, missed connections, or counter overflow. +As an application developer, I want multiple/concurrent calls to `ClearPool` to behave correctly, so that pool state is not corrupted. **Independent Test**: Can be tested by calling `ClearPool` multiple times in rapid succession and verifying no exceptions, no connection leaks, and correct pool behavior afterward. **Acceptance Scenarios**: 1. **Given** a pool with connections, **When** `ClearPool` is called twice rapidly, **Then** both calls complete without error, and only connections predating both clears are invalidated. -2. **Given** a pool where connections are being opened concurrently with a clear, **When** ClearPool is called, **Then** connections opened after the clear are not invalidated. --- @@ -73,7 +62,6 @@ As an application developer, I want multiple rapid calls to `ClearPool` to behav - What happens if `ClearPool` is called on an empty pool? The generation counter increments but no connections are closed. Subsequent connections are fresh. - What happens if `ClearPool` is called during pool shutdown? The clear should be a no-op or complete harmlessly — shutdown already destroys all connections. -- What happens if a connection is returned to the pool between the generation increment and the idle channel drain? The connection is caught by the generation check in `ReturnInternalConnection` or on next retrieval from the idle channel. - What happens with many clears causing generation counter overflow? With `int` counter, overflow at 2^31. Even at 1 clear/second, this is 68 years. Overflow is not a practical concern. If it wraps, the worst case is one stale connection survives a single retrieval cycle. ## Requirements @@ -92,8 +80,8 @@ As an application developer, I want multiple rapid calls to `ClearPool` to behav ### Key Entities - **Pool Generation Counter (`_clearGeneration`)**: A pool-level `volatile int` incremented atomically via `Interlocked.Increment` on each `Clear()` call. Represents the current "epoch" of the pool. -- **Connection Generation (`PoolGeneration`)**: A property on `DbConnectionInternal` stamped when the connection is created or added to the pool. Used to compare against the pool's current generation. -- **Stale Connection**: A connection whose `PoolGeneration` does not match the pool's `_clearGeneration`. Stale connections are destroyed rather than returned to the idle channel. +- **Connection Generation (`ClearGeneration`)**: A property on `DbConnectionInternal` stamped when the connection is created or added to the pool. Used to compare against the pool's current generation. +- **Stale Connection**: A connection whose `ClearGeneration` does not match the pool's `_clearGeneration`. Stale connections are destroyed rather than returned to the idle channel. ## Success Criteria @@ -106,6 +94,6 @@ As an application developer, I want multiple rapid calls to `ClearPool` to behav ## Assumptions - The generation counter approach is preferred over the WaitHandle pool's `DoNotPoolThisConnection()` mark-all pattern because `ConnectionPoolSlots` is not iterable by design (CAS-based slot array). -- `DbConnectionInternal` can accommodate a new `PoolGeneration` property without breaking existing functionality or requiring changes to the WaitHandle pool. +- `DbConnectionInternal` can accommodate a new `ClearGeneration` property without breaking existing functionality or requiring changes to the WaitHandle pool. - The existing `SqlConnection.ClearPool()` → `SqlConnectionFactory.ClearPool()` → `IDbConnectionPool.Clear()` call chain is already wired up and only requires the `ChannelDbConnectionPool.Clear()` implementation. - Transacted connections with stale generations are handled separately as part of the transactions feature and are out of scope for Phase 1 of pool clear. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index d7c5fd97d8..794b32b23a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -101,7 +101,7 @@ internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool all /// // TODO: Ideally this would be readonly and set in the constructor. Piping the value all the way through the connection factory is too complicated to be worth it. // If we can expose the constructor to the connection pool in the future, it can be set at in the constructor. - internal int PoolGeneration { get; set; } + internal int ClearGeneration { get; set; } internal bool AllowSetConnectionString { get; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 0238590da2..4aee069513 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -411,7 +411,7 @@ public bool TryGetConnection( if (connection is not null) { - connection.PoolGeneration = _clearCounter; + connection.ClearGeneration = _clearCounter; } return connection; @@ -445,7 +445,7 @@ private bool IsLiveConnection(DbConnectionInternal connection) } // Connection was created before the last Clear, so it's stale. - if (connection.PoolGeneration != _clearCounter) + if (connection.ClearGeneration != _clearCounter) { return false; } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index b7e45ddc3a..f5020b2948 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -743,7 +743,7 @@ public void Clear_MultipleIdleConnections_AllAreDestroyed() new DbConnectionOptions("", null), out internalConnections[i] ); - Assert.Equal(0, internalConnections[i]!.PoolGeneration); + Assert.Equal(0, internalConnections[i]!.ClearGeneration); } // Return all connections to the pool @@ -773,14 +773,14 @@ public void Clear_BusyConnections_NotDestroyedImmediately() out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); - Assert.Equal(0, busyConnection.PoolGeneration); + Assert.Equal(0, busyConnection.ClearGeneration); // Act - Clear while connection is still busy pool.Clear(); // Assert - Busy connection is still tracked in the pool and retains its old generation Assert.Equal(1, pool.Count); - Assert.Equal(0, busyConnection.PoolGeneration); + Assert.Equal(0, busyConnection.ClearGeneration); } [Fact] @@ -797,7 +797,7 @@ public void Clear_BusyConnectionReturned_IsDestroyed() out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); - Assert.Equal(0, busyConnection.PoolGeneration); + Assert.Equal(0, busyConnection.ClearGeneration); // Act - Clear, then return the busy connection pool.Clear(); @@ -834,8 +834,8 @@ out DbConnectionInternal? idleConnection ); Assert.NotNull(busyConnection); Assert.NotNull(idleConnection); - Assert.Equal(0, busyConnection.PoolGeneration); - Assert.Equal(0, idleConnection.PoolGeneration); + Assert.Equal(0, busyConnection.ClearGeneration); + Assert.Equal(0, idleConnection.ClearGeneration); // Return only the idle connection pool.ReturnInternalConnection(idleConnection, idleOwner); @@ -845,7 +845,7 @@ out DbConnectionInternal? idleConnection // Assert - Only the busy connection remains with stale generation Assert.Equal(1, pool.Count); - Assert.Equal(0, busyConnection.PoolGeneration); + Assert.Equal(0, busyConnection.ClearGeneration); // Now return the busy connection - it should be destroyed (generation 0 != pool generation 1) pool.ReturnInternalConnection(busyConnection, busyOwner); @@ -865,7 +865,7 @@ public void Clear_NewConnectionsAfterClear_ArePooledNormally() new DbConnectionOptions("", null), out DbConnectionInternal? oldConnection ); - Assert.Equal(0, oldConnection!.PoolGeneration); + Assert.Equal(0, oldConnection!.ClearGeneration); pool.ReturnInternalConnection(oldConnection, owningConnection); // Act @@ -883,7 +883,7 @@ out DbConnectionInternal? newConnection // The new connection should be different from the old one and have generation 1 Assert.NotSame(oldConnection, newConnection); - Assert.Equal(1, newConnection.PoolGeneration); + Assert.Equal(1, newConnection.ClearGeneration); // Return the new connection - it should be pooled normally pool.ReturnInternalConnection(newConnection, newOwner); @@ -898,7 +898,7 @@ out DbConnectionInternal? newConnection out DbConnectionInternal? reusedConnection ); Assert.Same(newConnection, reusedConnection); - Assert.Equal(1, reusedConnection!.PoolGeneration); + Assert.Equal(1, reusedConnection!.ClearGeneration); } [Fact] @@ -914,7 +914,7 @@ public void Clear_MultipleClearCalls_DoNotCorruptState() new DbConnectionOptions("", null), out DbConnectionInternal? connection ); - Assert.Equal(0, connection!.PoolGeneration); + Assert.Equal(0, connection!.ClearGeneration); pool.ReturnInternalConnection(connection, owningConnection); // Act - Call clear multiple times rapidly @@ -935,7 +935,7 @@ out DbConnectionInternal? newConnection ); Assert.NotNull(newConnection); Assert.Equal(1, pool.Count); - Assert.Equal(3, newConnection.PoolGeneration); + Assert.Equal(3, newConnection.ClearGeneration); } #endregion From 63a78ae2d7920876ea4f1c9ca7501aec665c8f22 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 14 Apr 2026 14:44:49 -0700 Subject: [PATCH 04/10] Expose IdleCount. Wrap idle channel in helper class to safely control IdleCount calculation. --- .../ConnectionPool/ChannelDbConnectionPool.cs | 37 ++- .../ConnectionPool/IDbConnectionPool.cs | 5 + .../SqlClient/ConnectionPool/IdleChannel.cs | 86 +++++++ .../WaitHandleDbConnectionPool.cs | 6 +- .../ConnectionPoolHelper.cs | 16 +- .../ConnectionPool/IdleChannelTest.cs | 214 ++++++++++++++++++ .../TransactedConnectionPoolTest.cs | 1 + 7 files changed, 329 insertions(+), 36 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 4aee069513..d0b1006bfe 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -73,11 +73,10 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool private readonly ConnectionPoolSlots _connectionSlots; /// - /// Reader side for the idle connection channel. Contains nulls in order to release waiting attempts after - /// a connection has been physically closed/broken. + /// The idle connection channel. Contains nulls in order to release waiting attempts after + /// a connection has been physically closed/broken. Also tracks the count of non-null idle connections. /// - private readonly ChannelReader _idleConnectionReader; - private readonly ChannelWriter _idleConnectionWriter; + private readonly IdleConnectionChannel _idleChannel; /// /// The current generation of the pool. Incremented atomically on each call. @@ -113,13 +112,7 @@ internal ChannelDbConnectionPool( TransactedConnectionPool = new(this); _connectionSlots = new(MaxPoolSize); - - // We enforce Max Pool Size, so no need to create a bounded channel (which is less efficient) - // On the consuming side, we have the multiplexing write loop but also non-multiplexing Rents - // On the producing side, we have connections being released back into the pool (both multiplexing and not) - var idleChannel = Channel.CreateUnbounded(); - _idleConnectionReader = idleChannel.Reader; - _idleConnectionWriter = idleChannel.Writer; + _idleChannel = new(); State = Running; } @@ -136,6 +129,9 @@ public ConcurrentDictionary< /// public int Count => _connectionSlots.ReservationCount; + /// + public int IdleCount => _idleChannel.Count; + /// public bool ErrorOccurred => throw new NotImplementedException(); @@ -194,14 +190,13 @@ public void Clear() try { // Drain idle connections from the channel and destroy them. Limit iterations to - // the current pool count to prevent an unbounded loop if connections are + // the current idle count to prevent an unbounded loop if connections are // concurrently returned to the channel during the drain. - int numToDrain = Count; - while (numToDrain > 0 && _idleConnectionReader.TryRead(out DbConnectionInternal? connection)) + int numToDrain = IdleCount; + while (numToDrain > 0 && _idleChannel.TryRead(out DbConnectionInternal? connection)) { if (connection is not null) { - //TODO: should we check pool generation of the connection here? RemoveConnection(connection); numToDrain--; } @@ -256,7 +251,7 @@ public void ReturnInternalConnection(DbConnectionInternal connection, DbConnecti } else { - var written = _idleConnectionWriter.TryWrite(connection); + var written = _idleChannel.TryWrite(connection); Debug.Assert(written, "Failed to write returning connection to the idle channel."); } } @@ -420,7 +415,7 @@ public bool TryGetConnection( { // If we fail to open a connection, we need to write a null to the idle channel to // wake up any waiters - _idleConnectionWriter?.TryWrite(null); + _idleChannel?.TryWrite(null); newConnection?.Dispose(); }); } @@ -464,7 +459,7 @@ private void RemoveConnection(DbConnectionInternal connection) // 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. - _idleConnectionWriter.TryWrite(null); + _idleChannel.TryWrite(null); connection.Dispose(); } @@ -476,7 +471,7 @@ private void RemoveConnection(DbConnectionInternal connection) private DbConnectionInternal? GetIdleConnection() { // The channel may contain nulls. Read until we find a non-null connection or exhaust the channel. - while (_idleConnectionReader.TryRead(out DbConnectionInternal? connection)) + while (_idleChannel.TryRead(out DbConnectionInternal? connection)) { if (connection is null) { @@ -542,7 +537,7 @@ private async Task GetInternalConnection( // (first-come, first-served), which is crucial to us. if (async) { - connection ??= await _idleConnectionReader.ReadAsync(cancellationToken).ConfigureAwait(false); + connection ??= await _idleChannel.ReadAsync(cancellationToken).ConfigureAwait(false); } else { @@ -589,7 +584,7 @@ private async Task GetInternalConnection( try { ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter = - _idleConnectionReader.ReadAsync(cancellationToken).ConfigureAwait(false).GetAwaiter(); + _idleChannel.ReadAsync(cancellationToken).ConfigureAwait(false).GetAwaiter(); using ManualResetEventSlim mres = new ManualResetEventSlim(false, 0); // Cancellation happens through the ReadAsync call, which will complete the task. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs index bfc5789d3f..3a984a5883 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs @@ -38,6 +38,11 @@ internal interface IDbConnectionPool /// int Count { get; } + /// + /// The number of connections currently sitting idle in the pool. + /// + int IdleCount { get; } + /// /// Indicates whether an error has occurred in the pool. /// Primarily used to support the pool blocking period feature. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs new file mode 100644 index 0000000000..5ad80ab11e --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs @@ -0,0 +1,86 @@ +// 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. +using System.Threading; +using System.Threading.Channels; +using Microsoft.Data.ProviderBase; + +#nullable enable + +namespace Microsoft.Data.SqlClient.ConnectionPool +{ + /// + /// Wraps an unbounded of idle connections and tracks the number of + /// non-null connections it contains. Unbounded channels do not support + /// , so this class maintains the count via + /// operations on every read and write of a non-null value. + /// + internal sealed class IdleConnectionChannel + { + private readonly ChannelReader _reader; + private readonly ChannelWriter _writer; + private volatile int _count; + + internal IdleConnectionChannel() + { + var channel = Channel.CreateUnbounded(); + _reader = channel.Reader; + _writer = channel.Writer; + } + + /// + /// The number of non-null connections currently in the channel. + /// + internal int Count => _count; + + /// + /// Writes a connection (or null wake-up signal) to the channel. + /// Increments the idle count when is not null. + /// + /// if the value was written; otherwise . + internal bool TryWrite(DbConnectionInternal? connection) + { + if (connection is not null) + { + Interlocked.Increment(ref _count); + } + + return _writer.TryWrite(connection); + } + + /// + /// Tries to read a value from the channel without blocking. + /// Decrements the idle count when a non-null connection is read. + /// + internal bool TryRead(out DbConnectionInternal? connection) + { + if (_reader.TryRead(out connection)) + { + if (connection is not null) + { + Interlocked.Decrement(ref _count); + } + + return true; + } + + return false; + } + + /// + /// Asynchronously reads a value from the channel. + /// Decrements the idle count when a non-null connection is read. + /// + internal async System.Threading.Tasks.ValueTask ReadAsync(CancellationToken cancellationToken) + { + var connection = await _reader.ReadAsync(cancellationToken).ConfigureAwait(false); + + if (connection is not null) + { + Interlocked.Decrement(ref _count); + } + + return connection; + } + } +} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 12bff3b598..b7fa972e85 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -259,8 +259,12 @@ private int CreationTimeout get { return PoolGroupOptions.CreationTimeout; } } + /// public int Count => _totalObjects; + /// + public int IdleCount => _stackNew.Count + _stackOld.Count; + public SqlConnectionFactory ConnectionFactory => _connectionFactory; public bool ErrorOccurred => _errorOccurred; @@ -290,7 +294,7 @@ private bool NeedToReplenish return true; } - int freeObjects = _stackNew.Count + _stackOld.Count; + int freeObjects = IdleCount; int waitingRequests = _waitCount; bool needToReplenish = (freeObjects < waitingRequests) || ((freeObjects == waitingRequests) && (totalObjects > 1)); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs index f51aaa6a37..acaf348a52 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs @@ -25,31 +25,19 @@ internal static class ConnectionPoolHelper private static Type s_dictPoolIdentityPool = typeof(ConcurrentDictionary<,>).MakeGenericType(s_dbConnectionPoolIdentity, s_dbConnectionPool); // Resolve Count from the interface so it works with both pool implementations private static PropertyInfo s_dbConnectionPoolCount = s_dbConnectionPool.GetProperty("Count", BindingFlags.Instance | BindingFlags.Public); + private static PropertyInfo s_dbConnectionPoolIdleCount = s_dbConnectionPool.GetProperty("IdleCount", BindingFlags.Instance | BindingFlags.Public); private static PropertyInfo s_dictStringPoolGroupGetKeys = s_dictStringPoolGroup.GetProperty("Keys"); private static PropertyInfo s_dictPoolIdentityPoolValues = s_dictPoolIdentityPool.GetProperty("Values"); private static PropertyInfo s_sqlConnectionFactorySingleton = s_sqlConnectionFactory.GetProperty("Instance", BindingFlags.Static | BindingFlags.NonPublic); private static FieldInfo s_dbConnectionFactoryPoolGroupList = s_sqlConnectionFactory.GetField("_connectionPoolGroups", BindingFlags.Instance | BindingFlags.NonPublic); private static FieldInfo s_dbConnectionPoolGroupPoolCollection = s_dbConnectionPoolGroup.GetField("_poolCollection", BindingFlags.Instance | BindingFlags.NonPublic); - private static FieldInfo s_dbConnectionPoolStackOld = s_waitHandleDbConnectionPool.GetField("_stackOld", BindingFlags.Instance | BindingFlags.NonPublic); - private static FieldInfo s_dbConnectionPoolStackNew = s_waitHandleDbConnectionPool.GetField("_stackNew", BindingFlags.Instance | BindingFlags.NonPublic); private static MethodInfo s_dbConnectionPoolCleanup = s_waitHandleDbConnectionPool.GetMethod("CleanupCallback", BindingFlags.Instance | BindingFlags.NonPublic); private static MethodInfo s_dictStringPoolGroupTryGetValue = s_dictStringPoolGroup.GetMethod("TryGetValue"); public static int CountFreeConnections(object pool) { VerifyObjectIsPool(pool); - - if (s_channelDbConnectionPool.IsInstanceOfType(pool)) - { - // ChannelDbConnectionPool doesn't have separate stacks; - // Count represents idle connections available in the channel. - return (int)s_dbConnectionPoolCount.GetValue(pool, null); - } - - ICollection oldStack = (ICollection)s_dbConnectionPoolStackOld.GetValue(pool); - ICollection newStack = (ICollection)s_dbConnectionPoolStackNew.GetValue(pool); - - return (oldStack.Count + newStack.Count); + return (int)s_dbConnectionPoolIdleCount.GetValue(pool, null); } /// diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs new file mode 100644 index 0000000000..ed2acc6d5c --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs @@ -0,0 +1,214 @@ +// 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. + +using System; +using System.Data.Common; +using System.Threading; +using System.Threading.Tasks; +using System.Transactions; +using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.ConnectionPool; +using Xunit; + +#nullable enable + +namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool +{ + public class IdleChannelTest + { + #region TryWrite + + [Fact] + public void TryWrite_NonNullConnection_IncrementsCount() + { + var channel = new IdleConnectionChannel(); + + Assert.True(channel.TryWrite(new StubDbConnectionInternal())); + Assert.Equal(1, channel.Count); + } + + [Fact] + public void TryWrite_NullConnection_DoesNotIncrementCount() + { + var channel = new IdleConnectionChannel(); + + Assert.True(channel.TryWrite(null)); + Assert.Equal(0, channel.Count); + } + + [Fact] + public void TryWrite_MultipleConnections_TracksCountCorrectly() + { + var channel = new IdleConnectionChannel(); + + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(null); + channel.TryWrite(new StubDbConnectionInternal()); + + Assert.Equal(3, channel.Count); + } + + #endregion + + #region TryRead + + [Fact] + public void TryRead_NonNullConnection_DecrementsCount() + { + var channel = new IdleConnectionChannel(); + channel.TryWrite(new StubDbConnectionInternal()); + Assert.Equal(1, channel.Count); + + Assert.True(channel.TryRead(out var connection)); + Assert.NotNull(connection); + Assert.Equal(0, channel.Count); + } + + [Fact] + public void TryRead_NullConnection_DoesNotDecrementCount() + { + var channel = new IdleConnectionChannel(); + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(null); + Assert.Equal(1, channel.Count); + + // Read the non-null connection first (FIFO) + Assert.True(channel.TryRead(out var first)); + Assert.NotNull(first); + Assert.Equal(0, channel.Count); + + // Read the null + Assert.True(channel.TryRead(out var second)); + Assert.Null(second); + Assert.Equal(0, channel.Count); + } + + [Fact] + public void TryRead_EmptyChannel_ReturnsFalse() + { + var channel = new IdleConnectionChannel(); + + Assert.False(channel.TryRead(out var connection)); + Assert.Null(connection); + Assert.Equal(0, channel.Count); + } + + #endregion + + #region ReadAsync + + [Fact] + public async Task ReadAsync_NonNullConnection_DecrementsCount() + { + var channel = new IdleConnectionChannel(); + channel.TryWrite(new StubDbConnectionInternal()); + Assert.Equal(1, channel.Count); + + var connection = await channel.ReadAsync(CancellationToken.None); + + Assert.NotNull(connection); + Assert.Equal(0, channel.Count); + } + + [Fact] + public async Task ReadAsync_NullConnection_DoesNotDecrementCount() + { + var channel = new IdleConnectionChannel(); + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(null); + Assert.Equal(1, channel.Count); + + // First read returns the non-null connection (FIFO) + var first = await channel.ReadAsync(CancellationToken.None); + Assert.NotNull(first); + Assert.Equal(0, channel.Count); + + // Second read returns null + var second = await channel.ReadAsync(CancellationToken.None); + Assert.Null(second); + Assert.Equal(0, channel.Count); + } + + [Fact] + public async Task ReadAsync_WaitsForWrite() + { + var channel = new IdleConnectionChannel(); + var expected = new StubDbConnectionInternal(); + + var readTask = channel.ReadAsync(CancellationToken.None); + Assert.False(readTask.IsCompleted); + + channel.TryWrite(expected); + + var connection = await readTask; + Assert.Same(expected, connection); + Assert.Equal(0, channel.Count); + } + + [Fact] + public async Task ReadAsync_Cancelled_ThrowsOperationCanceledException() + { + var channel = new IdleConnectionChannel(); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + await Assert.ThrowsAnyAsync( + () => channel.ReadAsync(cts.Token).AsTask()); + } + + #endregion + + #region Mixed operations + + [Fact] + public void WriteAndReadSequence_CountStaysConsistent() + { + var channel = new IdleConnectionChannel(); + + // Write 3 + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(new StubDbConnectionInternal()); + channel.TryWrite(new StubDbConnectionInternal()); + Assert.Equal(3, channel.Count); + + // Read 2 + channel.TryRead(out _); + channel.TryRead(out _); + Assert.Equal(1, channel.Count); + + // Write 1 more + channel.TryWrite(new StubDbConnectionInternal()); + Assert.Equal(2, channel.Count); + + // Read remaining 2 + channel.TryRead(out _); + channel.TryRead(out _); + Assert.Equal(0, channel.Count); + + // Channel is empty + Assert.False(channel.TryRead(out _)); + Assert.Equal(0, channel.Count); + } + + #endregion + + #region Helpers + + private class StubDbConnectionInternal : DbConnectionInternal + { + public override string ServerVersion => throw new NotImplementedException(); + + public override DbTransaction BeginTransaction(System.Data.IsolationLevel il) + => throw new NotImplementedException(); + + public override void EnlistTransaction(Transaction transaction) { } + protected override void Activate(Transaction transaction) { } + protected override void Deactivate() { } + internal override void ResetConnection() { } + } + + #endregion + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs index 18bd9c5ea3..9f284be22a 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs @@ -661,6 +661,7 @@ internal class MockDbConnectionPool : IDbConnectionPool public int Count => throw new NotImplementedException(); public bool ErrorOccurred => throw new NotImplementedException(); public int Id { get; } = 1; + public int IdleCount => throw new NotImplementedException(); public DbConnectionPoolIdentity Identity => throw new NotImplementedException(); public bool IsRunning => throw new NotImplementedException(); public TimeSpan LoadBalanceTimeout => throw new NotImplementedException(); From 0e73d4128fd25b1523abe0594ea17346257970fc Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 14 Apr 2026 14:49:02 -0700 Subject: [PATCH 05/10] Make basic lifecycle methods no-ops to enable manual tests. --- .../SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | 5 +++-- .../ConnectionPool/ChannelDbConnectionPoolTest.cs | 9 +-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index d0b1006bfe..6ca5559061 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -133,7 +133,8 @@ public ConcurrentDictionary< public int IdleCount => _idleChannel.Count; /// - public bool ErrorOccurred => throw new NotImplementedException(); + /// This will be implemented later when we add support for the pool blocking period after errors. For now, it always returns false. + public bool ErrorOccurred => false; /// public int Id => _instanceId; @@ -259,7 +260,7 @@ public void ReturnInternalConnection(DbConnectionInternal connection, DbConnecti /// public void Shutdown() { - throw new NotImplementedException(); + // No-op for now, warmup will be implemented later. } /// diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index f5020b2948..40901753b8 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -570,7 +570,7 @@ public void TestCount() public void TestErrorOccurred() { var pool = ConstructPool(SuccessfulConnectionFactory); - Assert.Throws(() => _ = pool.ErrorOccurred); + Assert.False(pool.ErrorOccurred); } [Fact] @@ -697,13 +697,6 @@ public void TestReplaceConnection() Assert.Throws(() => pool.ReplaceConnection(null!, null!, null!)); } - [Fact] - public void TestShutdown() - { - var pool = ConstructPool(SuccessfulConnectionFactory); - Assert.Throws(() => pool.Shutdown()); - } - [Fact] public void TestTransactionEnded() { From 0be9d4c30d43a7032b8379b81719e5f485e384bb Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 14 Apr 2026 15:06:32 -0700 Subject: [PATCH 06/10] Address copilot comments. --- .../SQL/ConnectionPoolTest/ConnectionPoolTest.cs | 16 ++++++++++++---- .../ChannelDbConnectionPoolTest.cs | 2 -- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index 491316c56d..0d9b076dd2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -14,8 +14,12 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests { public class ConnectionPoolConnectionStringProvider : IEnumerable { - private static readonly string _TCPConnectionString = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = false, Pooling = true }).ConnectionString; - private static readonly string _tcpMarsConnStr = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = true, Pooling = true }).ConnectionString; + private static readonly string _TCPConnectionString = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { + MultipleActiveResultSets = false, + Pooling = true}.ConnectionString; + private static readonly string _tcpMarsConnStr = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { + MultipleActiveResultSets = true, + Pooling = true }.ConnectionString; public IEnumerator GetEnumerator() { @@ -31,8 +35,12 @@ public IEnumerator GetEnumerator() public class ConnectionPoolConnectionStringAndPoolVersionProvider : IEnumerable { - private static readonly string _TCPConnectionString = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = false, Pooling = true }).ConnectionString; - private static readonly string _tcpMarsConnStr = (new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = true, Pooling = true }).ConnectionString; + private static readonly string _TCPConnectionString = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { + MultipleActiveResultSets = false, + Pooling = true }.ConnectionString; + private static readonly string _tcpMarsConnStr = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { + MultipleActiveResultSets = true, + Pooling = true }.ConnectionString; public IEnumerator GetEnumerator() { diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 40901753b8..76bcdc4aa7 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -681,8 +681,6 @@ public void TestUseLoadBalancing() #region Not Implemented Method Tests - - [Fact] public void TestPutObjectFromTransactedPool() { From ed07c99e53b465159e78ff5a3fca05802c06bfe5 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 17 Apr 2026 12:53:47 -0700 Subject: [PATCH 07/10] Review changes --- specs/001-pool-clear/spec.md | 2 ++ .../src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | 2 +- .../SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | 6 +++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/specs/001-pool-clear/spec.md b/specs/001-pool-clear/spec.md index dc1d60b9d6..c8d06a3c9c 100644 --- a/specs/001-pool-clear/spec.md +++ b/specs/001-pool-clear/spec.md @@ -63,6 +63,8 @@ As an application developer, I want multiple/concurrent calls to `ClearPool` to - What happens if `ClearPool` is called on an empty pool? The generation counter increments but no connections are closed. Subsequent connections are fresh. - What happens if `ClearPool` is called during pool shutdown? The clear should be a no-op or complete harmlessly — shutdown already destroys all connections. - What happens with many clears causing generation counter overflow? With `int` counter, overflow at 2^31. Even at 1 clear/second, this is 68 years. Overflow is not a practical concern. If it wraps, the worst case is one stale connection survives a single retrieval cycle. +- What happens if `ClearPool` is called during pool startup? `ClearPool` can be called at any time after the pool is instantiated. If connections are being added to the pool while clearing, they are closed subject to the conditions of the clear operation. +- What happens if ClearPool is called while a SqlConnection is waiting to receive a connection from the pool? If a SqlConnection is waiting for a connection, then there are no idle connections in the pool, so `ClearPool` will not have any effect. ## Requirements diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index 794b32b23a..5d3143783f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -100,7 +100,7 @@ internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool all /// Not safe, should only be set by the connection pool. /// // TODO: Ideally this would be readonly and set in the constructor. Piping the value all the way through the connection factory is too complicated to be worth it. - // If we can expose the constructor to the connection pool in the future, it can be set at in the constructor. + // If we can expose the constructor to the connection pool in the future, it can be set in the constructor. internal int ClearGeneration { get; set; } internal bool AllowSetConnectionString { get; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 6ca5559061..e253c655a3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -82,6 +82,7 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool /// The current generation of the pool. Incremented atomically on each call. /// Connections stamped with a generation that does not match are considered stale and are destroyed /// rather than returned to the idle channel. + /// Must be updated using operations to ensure thread safety. /// private volatile int _clearCounter; @@ -89,6 +90,7 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool /// Guard to prevent concurrent operations from draining the idle channel /// simultaneously. The generation counter is still incremented by every caller so stale connections /// are always caught lazily, but only one thread performs the actual drain. + /// Must be updated using operations to ensure thread safety. /// private volatile int _isClearing; #endregion @@ -193,6 +195,8 @@ public void Clear() // Drain idle connections from the channel and destroy them. Limit iterations to // the current idle count to prevent an unbounded loop if connections are // concurrently returned to the channel during the drain. + // Any connections from a previous generation that are returned to the pool + // after we start draining will fail the _clearCounter comparison and will be closed. int numToDrain = IdleCount; while (numToDrain > 0 && _idleChannel.TryRead(out DbConnectionInternal? connection)) { @@ -205,7 +209,7 @@ public void Clear() } finally { - _isClearing = 0; + Interlocked.Exchange(ref _isClearing, 0); } SqlClientEventSource.Log.TryPoolerTraceEvent( From 3f5e4ae9cda0d7a15b184542ae15506a80bc7186 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 22 Apr 2026 15:06:09 -0700 Subject: [PATCH 08/10] Review changes 1 Co-authored-by: Copilot --- .../SqlConnection.xml | 20 ++++++++-- .../001-pool-clear/checklists/requirements.md | 37 ------------------- specs/001-pool-clear/spec.md | 3 +- .../ConnectionPool/ChannelDbConnectionPool.cs | 8 ++-- .../ConnectionPool/IDbConnectionPool.cs | 7 ++++ ...dleChannel.cs => IdleConnectionChannel.cs} | 15 +++++--- .../ChannelDbConnectionPoolTest.cs | 2 +- 7 files changed, 40 insertions(+), 52 deletions(-) delete mode 100644 specs/001-pool-clear/checklists/requirements.md rename src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/{IdleChannel.cs => IdleConnectionChannel.cs} (86%) diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml index f0eddd56fa..f2e03ebf26 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml @@ -827,10 +827,17 @@ The following example creates a an - Empties the connection pool. + Empties all connection pools. - resets (or empties) the connection pool. If there are connections in use at the time of the call, they are marked appropriately and will be discarded (instead of being returned to the pool) when is called on them. + resets (or empties) all connection pools. If there are connections in use at the time of the call, they are marked appropriately and will be discarded (instead of being returned to a pool) when is called on them. + +> [!CAUTION] +> Clearing the pool is an expensive operation and should only be used if required. This operation may negatively interfere with pool warmup and generate high connection churn as the warmup operation continually opens new connections to attempt to reach min pool size. This situation is especially likely if clear is called in a tight loop. + +]]> @@ -841,7 +848,14 @@ The following example creates a an Empties the connection pool associated with the specified connection. - clears the connection pool that is associated with the . If additional connections associated with are in use at the time of the call, they are marked appropriately and are discarded (instead of being returned to the pool) when is called on them. + clears the connection pool that is associated with the `connection`. If additional connections associated with `connection` are in use at the time of the call, they are marked appropriately and are discarded (instead of being returned to the pool) when is called on them. + +> [!CAUTION] +> Clearing the pool is an expensive operation and should only be used if required. This operation may negatively interfere with pool warmup and generate high connection churn as the warmup operation continually opens new connections to attempt to reach min pool size. This situation is especially likely if clear is called in a tight loop. + +]]> diff --git a/specs/001-pool-clear/checklists/requirements.md b/specs/001-pool-clear/checklists/requirements.md deleted file mode 100644 index 915b707f1d..0000000000 --- a/specs/001-pool-clear/checklists/requirements.md +++ /dev/null @@ -1,37 +0,0 @@ -# Specification Quality Checklist: Pool Clear - -**Purpose**: Validate specification completeness and quality before proceeding to planning -**Created**: 2026-04-10 -**Feature**: [spec.md](../spec.md) - -## Content Quality - -- [x] No implementation details (languages, frameworks, APIs) -- [x] Focused on user value and business needs -- [x] Written for non-technical stakeholders -- [x] All mandatory sections completed - -## Requirement Completeness - -- [x] No [NEEDS CLARIFICATION] markers remain -- [x] Requirements are testable and unambiguous -- [x] Success criteria are measurable -- [x] Success criteria are technology-agnostic (no implementation details) -- [x] All acceptance scenarios are defined -- [x] Edge cases are identified -- [x] Scope is clearly bounded -- [x] Dependencies and assumptions identified - -## Feature Readiness - -- [x] All functional requirements have clear acceptance criteria -- [x] User scenarios cover primary flows -- [x] Feature meets measurable outcomes defined in Success Criteria -- [x] No implementation details leak into specification - -## Notes - -- All items pass validation. The spec correctly separates Phase 1 (basic clear) from Phase 2 (transaction-aware clear) via assumptions. -- Generation counter is mentioned as a Key Entity (what), not as implementation detail (how). The concept is domain-level — it describes the invalidation mechanism in user terms. -- SC-004 references "pool v1/v2" which is borderline implementation detail but is user-facing since pool selection is via AppContext switch. Accepted. -- Ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/001-pool-clear/spec.md b/specs/001-pool-clear/spec.md index c8d06a3c9c..3ff9488410 100644 --- a/specs/001-pool-clear/spec.md +++ b/specs/001-pool-clear/spec.md @@ -54,7 +54,7 @@ As an application developer, I want multiple/concurrent calls to `ClearPool` to **Acceptance Scenarios**: -1. **Given** a pool with connections, **When** `ClearPool` is called twice rapidly, **Then** both calls complete without error, and only connections predating both clears are invalidated. +1. **Given** a pool with connections, **When** `ClearPool` is called twice rapidly, **Then** both calls complete without error, and connections predating either clear are invalidated. --- @@ -70,7 +70,6 @@ As an application developer, I want multiple/concurrent calls to `ClearPool` to ### Functional Requirements -- **FR-001**: System MUST implement a generation counter that increments atomically on each `Clear()` call. - **FR-002**: System MUST stamp each new connection with the current pool generation at creation time. - **FR-003**: System MUST reject connections whose generation does not match the current pool generation when they are retrieved from the idle channel or returned to the pool. - **FR-004**: System MUST drain all idle connections from the channel on `Clear()`, closing each one. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index e253c655a3..d6db51e5bb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -84,7 +84,7 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool /// rather than returned to the idle channel. /// Must be updated using operations to ensure thread safety. /// - private volatile int _clearCounter; + private volatile int _clearGeneration; /// /// Guard to prevent concurrent operations from draining the idle channel @@ -178,7 +178,7 @@ public void Clear() SqlClientEventSource.Log.TryPoolerTraceEvent( " {0}, Clearing.", Id); - Interlocked.Increment(ref _clearCounter); + Interlocked.Increment(ref _clearGeneration); // If another thread is already draining, skip the drain. The generation counter has // already been incremented, so stale connections will still be caught lazily by @@ -411,7 +411,7 @@ public bool TryGetConnection( if (connection is not null) { - connection.ClearGeneration = _clearCounter; + connection.ClearGeneration = _clearGeneration; } return connection; @@ -445,7 +445,7 @@ private bool IsLiveConnection(DbConnectionInternal connection) } // Connection was created before the last Clear, so it's stale. - if (connection.ClearGeneration != _clearCounter) + if (connection.ClearGeneration != _clearGeneration) { return false; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs index 3a984a5883..8a6ae397ed 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs @@ -105,6 +105,13 @@ internal interface IDbConnectionPool /// /// Clears the connection pool, releasing all connections and resetting the state. /// + /// + /// Clearing the pool is an expensive operation and should only be used if required. + /// This operation may negatively interfere with pool warmup and generate high connection + /// churn as the warmup operation continually opens new connections to attempt + /// to reach min pool size. This situation is especially likely if clear is called in a + /// tight loop. + /// void Clear(); /// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs similarity index 86% rename from src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs rename to src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs index 5ad80ab11e..3a52fc6f11 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs @@ -25,6 +25,7 @@ internal IdleConnectionChannel() { var channel = Channel.CreateUnbounded(); _reader = channel.Reader; + //TODO: the channel should be completed on pool shutdown _writer = channel.Writer; } @@ -40,12 +41,16 @@ internal IdleConnectionChannel() /// if the value was written; otherwise . internal bool TryWrite(DbConnectionInternal? connection) { - if (connection is not null) + if (_writer.TryWrite(connection)) { - Interlocked.Increment(ref _count); + if (connection is not null) + { + Interlocked.Increment(ref _count); + } + return true; } - - return _writer.TryWrite(connection); + + return false; } /// @@ -71,7 +76,7 @@ internal bool TryRead(out DbConnectionInternal? connection) /// Asynchronously reads a value from the channel. /// Decrements the idle count when a non-null connection is read. /// - internal async System.Threading.Tasks.ValueTask ReadAsync(CancellationToken cancellationToken) + internal async ValueTask ReadAsync(CancellationToken cancellationToken) { var connection = await _reader.ReadAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 76bcdc4aa7..c7df9dc2c6 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -751,7 +751,7 @@ out internalConnections[i] } [Fact] - public void Clear_BusyConnections_NotDestroyedImmediately() + public void Clear_BusyConnection_NotDestroyedImmediately() { // Arrange var pool = ConstructPool(SuccessfulConnectionFactory); From 31dd315b20650bb9b509b9c1f3f0e082d86cf9cd Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 22 Apr 2026 15:12:03 -0700 Subject: [PATCH 09/10] Rename test class. Add concurrency test. Co-authored-by: Copilot --- ...elTest.cs => IdleConnectionChannelTest.cs} | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) rename src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/{IdleChannelTest.cs => IdleConnectionChannelTest.cs} (88%) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleConnectionChannelTest.cs similarity index 88% rename from src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs rename to src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleConnectionChannelTest.cs index ed2acc6d5c..56aa9c234a 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleConnectionChannelTest.cs @@ -15,7 +15,7 @@ namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool { - public class IdleChannelTest + public class IdleConnectionChannelTest { #region TryWrite @@ -194,6 +194,36 @@ public void WriteAndReadSequence_CountStaysConsistent() #endregion + #region Multi-threaded Tests + + [Fact] + public async Task ConcurrentWriteAndRead_CountReturnsToZero() + { + var channel = new IdleConnectionChannel(); + var barrier = new Barrier(3); + const int iterations = 1000; + + async Task WriteAndRead() + { + barrier.SignalAndWait(); + + for (int i = 0; i < iterations; i++) + { + channel.TryWrite(new StubDbConnectionInternal()); + await channel.ReadAsync(CancellationToken.None); + } + } + + await Task.WhenAll( + Task.Run(WriteAndRead), + Task.Run(WriteAndRead), + Task.Run(WriteAndRead)); + + Assert.Equal(0, channel.Count); + } + + #endregion + #region Helpers private class StubDbConnectionInternal : DbConnectionInternal From ccdf316c8bf3faf2ca1fa4050f0a39074a95eed3 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 22 Apr 2026 16:15:02 -0700 Subject: [PATCH 10/10] Fix using statement. --- .../Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs index 3a52fc6f11..00748ab184 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. using System.Threading; +using System.Threading.Tasks; using System.Threading.Channels; using Microsoft.Data.ProviderBase; @@ -49,7 +50,7 @@ internal bool TryWrite(DbConnectionInternal? connection) } return true; } - + return false; }