From 641d3edc17ab4ea315845dcbeaa3826e38e7f211 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 23 Sep 2026 11:16:14 -0700 Subject: [PATCH] Preserve in-flight opens when clearing connection pools (#4718) * Fix OpenAsync retry after pool clear Route pending async opens on a cleared wait-handle pool back through the connection factory so they can complete from the replacement pool instead of surfacing a misleading pool timeout. Add a regression test that parks an async pending open, clears the pool group, and verifies completion from a replacement pool. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Preserve in-flight opens when clearing wait-handle pools Remove shutdown interruption and retry routing. Let admitted requests finish on the retired pool and dispose their connections on return. Preserve error expiry for remaining waiters. Cover sync and async clearing, cancellation, timeouts, and error expiry. Add a SQL Server regression workload for #4714. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Make pool-clear regression coverage deterministic Remove the concurrent stress workload and elapsed-time assertions. Gate physical creation and assert exact request ordering and creation counts around pool shutdown. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Remove pool-clearing documentation changes Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Leave retired pool clearing to the factory Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify gated pool shutdown test scenario Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Separate pool-clear scenario from test plumbing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Organize pool regression tests as arrange act assert Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Explain pool regression test interleavings inline Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Trim blocking-period test commentary Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Explain regression conditions and assertion evidence Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify shutdown test walkthrough Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Match pool-clear comments to shutdown walkthrough Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../WaitHandleDbConnectionPool.cs | 70 +----- ...andleDbConnectionPoolBlockingPeriodTest.cs | 28 +++ .../WaitHandleDbConnectionPoolShutdownTest.cs | 234 +++++++++++++----- .../PoolClearDuringOpenTests.cs | 205 +++++++++++++++ 4 files changed, 413 insertions(+), 124 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/PoolClearDuringOpenTests.cs 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 afaa827fce..887bcd5c35 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 @@ -900,12 +900,8 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource {0}, Pool is shutting down; abandoning wait.", Id); - if (waitResult == SEMAPHORE_HANDLE || waitResult == WAIT_ABANDONED + SEMAPHORE_HANDLE) - { - try - { - _waitHandles.PoolSemaphore.Release(1); - } - catch (SemaphoreFullException) - { - // Pool semaphore was already saturated by Shutdown's bulk release; safe to ignore. - } - } - Interlocked.Decrement(ref _waitCount); - connection = null; - return false; - } - // From the WaitAny docs: "If more than one object became signaled during // the call, this is the array index of the signaled object with the // smallest index value of all the signaled objects." This is important @@ -1597,37 +1565,17 @@ public void Shutdown() } State = ShuttingDown; - // Dispose all background timers so they no longer schedule new work. - // Note that any timer callback already in flight may still observe State == ShuttingDown - // and short-circuit (see CleanupCallback / ErrorCallback). + // Stop maintenance, but let admitted requests finish. Their connections are + // destroyed by DeactivateObject when returned to this retired pool. Timer cleanup = Interlocked.Exchange(ref _cleanupTimer, null); cleanup?.Dispose(); - _errorState.Dispose(); - - // Wake any threads parked in WaitHandle.WaitAny by releasing as many semaphore - // slots as there are recorded waiters. Using _waitCount (rather than MaxPoolSize) - // avoids ArgumentOutOfRangeException when MaxPoolSize == 0 (unlimited) and ensures - // we wake every parked waiter even when _waitCount exceeds MaxPoolSize. Waiters - // observe State is not Running after wake-up and bail. - int waitersToWake = Volatile.Read(ref _waitCount); - if (waitersToWake > 0) - { - try - { - _waitHandles.PoolSemaphore.Release(waitersToWake); - } - catch (SemaphoreFullException) - { - // Semaphore already saturated; nothing to do. - } - } + // Keep the cached error and its expiry timer available to admitted waiters. + // Disposing the error state here leaves ErrorEvent signaled without an error. - // Reuse Clear() to doom every connection (including active checked-out ones), drain - // both idle stacks, and reclaim emancipated objects. Active connections destroy - // themselves on return either via the doom flag or via DeactivateObject's - // State == ShuttingDown branch. - Clear(); + // Leave Clear() to the factory's explicit-clear or deferred-pruning path. + // Shutdown can run under the pool-group lock, where reclamation and connection + // disposal must not be added before the pool is queued for release. } // TransactionEnded merely provides the plumbing for DbConnectionInternal to access the transacted pool diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs index c5d628c618..163023a0f4 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs @@ -130,6 +130,34 @@ public void TryGetConnection_WhenFactoryThrows_EntersBlockingPeriod() Assert.Equal(1, factory.CreateConnectionCallCount); } + /// + /// Shutdown preserves the cached error for admitted waiters and lets its timer expire. + /// + [Fact] + public void Shutdown_WhileBlocked_PreservesErrorUntilExpiry() + { + // Arrange + var clock = new FakeTimeProvider(); + SqlException failure = SqlExceptionHelper.CreateSqlException("server unreachable"); + var factory = new ConfigurableSqlConnectionFactory(_ => throw failure); + var pool = CreatePool(factory, timeProvider: clock); + using var owner = new SqlConnection(); + + Assert.Throws(() => TryGetConnectionSync(pool, owner, out _)); + + // Act + pool.Shutdown(); + + // Assert + Assert.True(pool.ErrorOccurred); + + // Act: expire the preserved blocking period. + clock.Advance(TimeSpan.FromSeconds(5)); + + // Assert + Assert.False(pool.ErrorOccurred); + } + /// /// Verifies that once the pool is in the blocking period, a subsequent request fast-fails /// with the cached exception without invoking the connection factory again. The first throw diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolShutdownTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolShutdownTest.cs index 17ed3e63f8..c0be93f156 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolShutdownTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolShutdownTest.cs @@ -3,6 +3,7 @@ // 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 Microsoft.Data.Common.ConnectionString; @@ -17,7 +18,7 @@ namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool /// public class WaitHandleDbConnectionPoolShutdownTest { - private static WaitHandleDbConnectionPool CreatePool(int maxPoolSize = 5) + private static WaitHandleDbConnectionPool CreatePool(int maxPoolSize = 5, SqlConnectionFactory? factory = null) { var poolGroupOptions = new DbConnectionPoolGroupOptions( poolByIdentity: false, @@ -34,7 +35,7 @@ private static WaitHandleDbConnectionPool CreatePool(int maxPoolSize = 5) poolGroupOptions); var pool = new WaitHandleDbConnectionPool( - new WaitHandleDbConnectionPoolTransactionTest.MockSqlConnectionFactory(), + factory ?? new WaitHandleDbConnectionPoolTransactionTest.MockSqlConnectionFactory(), dbConnectionPoolGroup, DbConnectionPoolIdentity.NoIdentity, new DbConnectionPoolProviderInfo()); @@ -67,13 +68,17 @@ public void Shutdown_DisposesCleanupTimer() Assert.Null(pool._cleanupTimer); } - // Drains idle stacks. + /// + /// Leaves idle connections for the factory's explicit or deferred Clear call. + /// [Fact] - public void Shutdown_DrainsIdleStacks() + public void Shutdown_LeavesIdleConnectionsUntilClear() { + // Arrange var pool = CreatePool(); - // Vend a few connections then return them so they sit in _stackNew. + // An empty pool would pass even if Shutdown still called Clear internally. + // Keep idle inventory so the two lifecycle operations have distinguishable effects. var owner1 = new SqlConnection(); var owner2 = new SqlConnection(); pool.TryGetConnection(owner1, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? c1); @@ -86,10 +91,32 @@ public void Shutdown_DrainsIdleStacks() Assert.Equal(2, pool.IdleCount); Assert.Equal(2, pool.Count); - pool.Shutdown(); - - Assert.Equal(0, pool.IdleCount); - Assert.Equal(0, pool.Count); + try + { + // Act + pool.Shutdown(); + + // Assert: unchanged inventory and poolability detect both effects of an + // unintended Clear: draining idle objects and marking them non-poolable. + Assert.False(pool.IsRunning); + Assert.Equal(2, pool.IdleCount); + Assert.Equal(2, pool.Count); + Assert.True(c1!.CanBePooled); + Assert.True(c2!.CanBePooled); + + // Act: perform the Clear that the factory owns, separately from shutdown. + pool.Clear(); + + // Assert + Assert.Equal(0, pool.IdleCount); + Assert.Equal(0, pool.Count); + } + finally + { + // Cleanup + pool.Shutdown(); + pool.Clear(); + } } // Shutdown is idempotent. @@ -160,70 +187,151 @@ public void TryGetConnection_Async_AfterShutdown_ShortCircuits_NoPendingOpenSche Assert.Equal(0, Volatile.Read(ref pool._waitCount)); } - // Shutdown wakes up a thread parked in WaitHandle.WaitAny. - [Trait("category", "flaky")] - // Failed Microsoft.Data.SqlClient.UnitTests.ConnectionPool.WaitHandleDbConnectionPoolShutdownTest.Shutdown_UnblocksSyncWaiter [5 s] - // ##[error]EXEC(0,0): Error Message: - // EXEC : error Message: [D:\a\_work\1\s\build.proj] - // Waiter did not park within 5s. - // Stack Trace: - // at Microsoft.Data.SqlClient.UnitTests.ConnectionPool.WaitHandleDbConnectionPoolShutdownTest.Shutdown_UnblocksSyncWaiter() in D:\a\_work\1\s\src\Microsoft.Data.SqlClient\tests\UnitTests\ConnectionPool\WaitHandleDbConnectionPoolShutdownTest.cs:line 207 - [Fact] - public void Shutdown_UnblocksSyncWaiter() + /// + /// Starts with no idle connections. Pauses the first physical connection creation + /// while it holds the creation semaphore, then admits a second request that waits + /// for that semaphore. Shuts down the pool before releasing the first creation. + /// Both requests create their own connections on the retired pool, and both + /// connections are destroyed when returned. In the cancellation case, the worker + /// returns and destroys the second connection instead of delivering it to the caller. + /// + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Shutdown_InFlightRequest_CompletesOnRetiredPool(bool async, bool cancel) { - var pool = CreatePool(maxPoolSize: 1); + // Arrange + // Initiate a request to the pool. It will be blocked by the gated connection factory. + using var factory = new GatedConnectionFactory(); + var pool = CreatePool(maxPoolSize: 2, factory: factory); + using var firstOwner = new SqlConnection(); + using var pendingOwner = new SqlConnection(); + var completion = new TaskCompletionSource(); + Task first = Acquire(pool, firstOwner, completion: null); + Task? pending = null; + + // Make sure the first request is blocked in the connection factory. Then, initiate a second request. + // The second request will be blocked on the create semaphore in the pool. + Assert.True(factory.Entered.Wait(TimeSpan.FromSeconds(10))); + pending = Acquire(pool, pendingOwner, async ? completion : null); + // Make sure the second request is also blocked. + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref pool._waitCount) == 2, TimeSpan.FromSeconds(10))); + Assert.Equal(1, factory.CreateCount); + Assert.False(first.IsCompleted); + Assert.False(pending.IsCompleted); + + // Act + // Now, shut down the pool. New requests will no longer be accepted, but in-flight requests should proceed. + pool.Shutdown(); + if (cancel) + { + completion.SetCanceled(); + } - // Saturate the pool. - var owner = new SqlConnection(); - Assert.True(pool.TryGetConnection(owner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? blocking)); - Assert.NotNull(blocking); + // Unblock the first request, allowing both to proceed, in turn. + factory.Release.Set(); - // Park a sync waiter on a worker thread with a long creation timeout. - DbConnectionInternal? waiterResult = null; - bool waiterCompleted = false; - Exception? waiterEx = null; + // Assert + // Wait for the first request to complete + Assert.False(pool.IsRunning); + Assert.Same(first, await Task.WhenAny(first, Task.Delay(TimeSpan.FromSeconds(10)))); + DbConnectionInternal? firstConnection = await first; + Assert.NotNull(firstConnection); + Assert.Same(pool, firstConnection.Pool); + + // Wait for the second request to complete + Assert.Same(pending, await Task.WhenAny(pending, Task.Delay(TimeSpan.FromSeconds(10)))); + if (cancel) + { + await Assert.ThrowsAnyAsync(() => pending); + Assert.True(SpinWait.SpinUntil(() => pool.Count == 1 && Volatile.Read(ref pool._waitCount) == 0, TimeSpan.FromSeconds(10))); + } + else + { + DbConnectionInternal? pendingConnection = await pending; + Assert.NotNull(pendingConnection); + Assert.Same(pool, pendingConnection.Pool); + Assert.Equal(0, Volatile.Read(ref pool._waitCount)); + } + + // Assert that both requests created new connections + Assert.Equal(2, factory.CreateCount); - var t = new Thread(() => + // Cleanup + factory.Release.Set(); + pool.Shutdown(); + await ReturnWhenCompleted(pool, firstOwner, first); + if (pending is not null) { - try - { - waiterCompleted = pool.TryGetConnection( - new SqlConnection(), - taskCompletionSource: null, - TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), - out waiterResult); - } - catch (Exception ex) + await ReturnWhenCompleted(pool, pendingOwner, pending); + } + + // Assert: returned connections were destroyed rather than pooled. + Assert.Equal(0, pool.IdleCount); + Assert.Equal(0, pool.Count); + } + + /// Starts a sync acquisition on a dedicated thread or queues an async acquisition. + private static Task Acquire(WaitHandleDbConnectionPool pool, SqlConnection owner, + TaskCompletionSource? completion) + { + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)); + if (completion is null) + { + return Task.Factory.StartNew(() => { - waiterEx = ex; - } - }) - { IsBackground = true }; - t.Start(); - - // Wait deterministically until the worker has incremented _waitCount, which - // happens immediately before it enters WaitHandle.WaitAny. Polling avoids the - // CI-flakiness of a fixed Thread.Sleep on slow agents. Volatile.Read ensures - // we see the worker's Interlocked.Increment without depending on CPU memory - // ordering of plain int reads. - var deadline = DateTime.UtcNow.AddSeconds(5); - while (DateTime.UtcNow < deadline && Volatile.Read(ref pool._waitCount) < 1) + Assert.True(pool.TryGetConnection(owner, null, timer, out DbConnectionInternal connection)); + return (DbConnectionInternal?)connection; + }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + } + // A false return here means async acquisition was queued, not that it failed. + // Its eventual result is delivered through completion.Task. + Assert.False(pool.TryGetConnection(owner, completion, timer, out DbConnectionInternal pending)); + Assert.Null(pending); + return completion.Task!; + } + + /// Drains test work and returns successful acquisitions even when an assertion failed. + private static async Task ReturnWhenCompleted(WaitHandleDbConnectionPool pool, SqlConnection owner, Task task) + { + Assert.Same(task, await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(20)))); + if (task.Status == TaskStatus.RanToCompletion && task.Result is { } connection) + { + pool.ReturnInternalConnection(connection, owner); + } + else if (task.IsFaulted) { - Thread.Yield(); + // Observe failures during assertion cleanup without replacing the original failure. + _ = task.Exception; } - Assert.True(Volatile.Read(ref pool._waitCount) >= 1, "Waiter did not park within 5s."); - Assert.True(t.IsAlive, "Waiter should be parked, but thread already exited."); + } - pool.Shutdown(); + /// Holds the first physical creation so a second acquisition waits on its semaphore. + private sealed class GatedConnectionFactory : WaitHandleDbConnectionPoolTransactionTest.MockSqlConnectionFactory, IDisposable + { + internal readonly ManualResetEventSlim Entered = new(); + internal readonly ManualResetEventSlim Release = new(); + private int _calls; - Assert.True(t.Join(TimeSpan.FromSeconds(5)), "Waiter did not unblock within 5s of Shutdown."); - // Acceptable outcomes: either returned false/null (timed out / abandoned) or - // returned true/null (state-check short-circuit). Either way, it must NOT block - // forever, and it must NOT vend a real connection from a shut-down pool. - Assert.Null(waiterResult); - Assert.Null(waiterEx); - // Suppress unused warning - presence of waiterCompleted just documents the contract. - _ = waiterCompleted; + internal int CreateCount => Volatile.Read(ref _calls); + + protected override DbConnectionInternal CreateConnection(SqlConnectionOptions options, ConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection, TimeoutTimer timeout) + { + if (Interlocked.Increment(ref _calls) == 1) + { + Entered.Set(); + Assert.True(Release.Wait(TimeSpan.FromSeconds(15)), "Physical creation was not released."); + } + return base.CreateConnection(options, poolKey, poolGroupProviderInfo, pool, owningConnection, timeout); + } + + public void Dispose() + { + Entered.Dispose(); + Release.Dispose(); + } } // Startup() must be a no-op when the pool has already been shut down. Without the diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/PoolClearDuringOpenTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/PoolClearDuringOpenTests.cs new file mode 100644 index 0000000000..9b6bf1f2b4 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/PoolClearDuringOpenTests.cs @@ -0,0 +1,205 @@ +// 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; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Data.SqlClient.ConnectionPool; +using Microsoft.Data.SqlClient.Tests.Common; +using Microsoft.SqlServer.TDS.Servers; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests; + +/// +/// Exercises public pool clearing while one login is in flight and another open is waiting. +/// +[Collection(SimulatedServerTestCollection.Name)] +public class PoolClearDuringOpenTests +{ + /// + /// Blocks the first login, admits a second open, then clears the pool before releasing + /// the login. Both opens finish on the retired pool and their connections are destroyed + /// on close. A subsequent open uses a replacement pool. + /// + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task ClearDuringOpen_InFlightConnectionsAreDestroyedOnClose(bool async, bool clearAll) + { + // Arrange + // Initiate an open using the wait-handle pool. Its login will be blocked by the gated server. + using var switches = new LocalAppContextSwitchesHelper { UseConnectionPoolV2 = false }; + using var server = new GatedLoginServer(); + using var first = new SqlConnection(server.ConnectionString); + using var pending = new SqlConnection(server.ConnectionString); + using var replacement = new SqlConnection(server.ConnectionString); + Task opens = Open(first, async); + + try + { + // Make sure the first open is blocked during login. Then, initiate a second open. + // The second open must wait for the first creation to finish. + server.WaitForFirstLogin(); + var retiredPool = Assert.IsType(first.PoolGroup.GetConnectionPool(SqlConnectionFactory.Instance)); + Assert.False(opens.IsCompleted); + Task pendingOpen = Open(pending, async); + opens = Task.WhenAll(opens, pendingOpen); + + // Make sure the second open is pending on the same pool and has not started its login. + AssertPendingOpen(pending, pendingOpen, retiredPool, async); + Assert.Same(first.PoolGroup, pending.PoolGroup); + Assert.Equal(1, server.LoginCount); + + // Act + // Now, clear the pool. New opens should use a new pool, but these in-flight opens should proceed. + if (clearAll) + { + SqlConnection.ClearAllPools(); + } + else + { + SqlConnection.ClearPool(first); + } + + // Unblock the first login, allowing both opens to proceed, in turn. + server.ReleaseFirstLogin(); + + // Assert + // Wait for both opens to complete, then confirm they succeeded on the original pool. + await AssertCompletes(opens); + await opens; + + Assert.False(retiredPool.IsRunning); + Assert.Equal(ConnectionState.Open, first.State); + Assert.Equal(ConnectionState.Open, pending.State); + Assert.Same(retiredPool, first.InnerConnection.Pool); + Assert.Same(retiredPool, pending.InnerConnection.Pool); + + // Assert that both opens created new connections. + Assert.Equal(2, retiredPool.Count); + Assert.Equal(2, server.LoginCount); + + // Act + // Initiate another open after clearing. This one should use a new pool. + await Open(replacement, async); + + // Assert + // Make sure the new open used a different, running pool and performed another login. + Assert.NotSame(retiredPool, replacement.InnerConnection.Pool); + Assert.True(replacement.InnerConnection.Pool.IsRunning); + Assert.Equal(3, server.LoginCount); + + // Act + // Close the connections that completed on the retired pool. They should be destroyed, not reused. + first.Close(); + pending.Close(); + + // Assert: the retired pool has no connections or pending requests left. + Assert.Equal(0, retiredPool.Count); + Assert.Equal(0, retiredPool.IdleCount); + Assert.Equal(0, Volatile.Read(ref retiredPool._waitCount)); + } + finally + { + // Cleanup + server.ReleaseFirstLogin(); + await AssertCompletes(opens); + if (opens.IsFaulted) + { + // Observe background failures without replacing an earlier assertion failure. + _ = opens.Exception; + } + first.Close(); + pending.Close(); + replacement.Close(); + SqlConnection.ClearPool(first); + } + } + + /// Confirms the second open is admitted before the test clears its pool. + private static void AssertPendingOpen(SqlConnection pending, Task open, WaitHandleDbConnectionPool pool, bool async) + { + if (async) + { + // The second async open is queued behind the worker handling the first login. + // It has not entered the pool's wait loop yet, so _waitCount will still be 1. + Assert.Equal(ConnectionState.Connecting, pending.State); + } + else + { + // The sync opens run on separate threads. Wait until the second has entered + // the pool and is blocked on the creation semaphore before allowing the test to clear it. + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref pool._waitCount) == 2, TimeSpan.FromSeconds(10)), + "Second open did not wait for the creation semaphore."); + } + Assert.False(open.IsCompleted); + } + + /// + /// Prevents a stuck open from hanging the test. The caller awaits the completed task + /// separately to check whether the opens succeeded. + /// + private static async Task AssertCompletes(Task task) => + Assert.Same(task, await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(20)))); + + /// Runs synchronous opens on a dedicated thread so the test can release the login. + private static Task Open(SqlConnection connection, bool async) => + async + ? connection.OpenAsync() + : Task.Factory.StartNew(connection.Open, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + + /// Owns the simulated server and a gate that blocks only its first login. + private sealed class GatedLoginServer : IDisposable + { + private readonly TdsServer _server = new(); + private readonly ManualResetEventSlim _loginEntered = new(); + private readonly ManualResetEventSlim _releaseLogin = new(); + private int _logins; + + internal int LoginCount => Volatile.Read(ref _logins); + internal string ConnectionString { get; } + + /// Starts an isolated server with room for both admitted connections. + internal GatedLoginServer() + { + _server.OnLogin7Validated = _ => + { + if (Interlocked.Increment(ref _logins) == 1) + { + _loginEntered.Set(); + Assert.True(_releaseLogin.Wait(TimeSpan.FromSeconds(15)), "Login was not released."); + } + }; + _server.Start(); + ConnectionString = new SqlConnectionStringBuilder + { + DataSource = $"localhost,{_server.EndPoint.Port}", + Encrypt = SqlConnectionEncryptOption.Optional, + Pooling = true, + MaxPoolSize = 2, + ConnectTimeout = 15, + }.ConnectionString; + } + + /// Waits until the first login reaches the gate. + internal void WaitForFirstLogin() => + Assert.True(_loginEntered.Wait(TimeSpan.FromSeconds(10)), "First login did not start."); + + /// Allows the first login to finish, including during failure cleanup. + internal void ReleaseFirstLogin() => _releaseLogin.Set(); + + /// Releases the gate and disposes the server before its synchronization objects. + public void Dispose() + { + ReleaseFirstLogin(); + _server.Dispose(); + _loginEntered.Dispose(); + _releaseLogin.Dispose(); + } + } +}