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 00ddb01c26..a87f7af27e 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 @@ -349,6 +349,19 @@ public bool IsRunning public TransactedConnectionPool TransactedConnectionPool => _transactedConnectionPool; + /// + /// Periodic pruning callback invoked by . Destroys idle + /// connections that have aged out of the general pool (those above + /// that have been on _stackOld for a full cleanup + /// period) and promotes the remaining _stackNew entries onto _stackOld + /// so they become eligible for pruning on the next tick. Connections held in the + /// are not touched here — the transaction-end + /// event is responsible for releasing them. + /// + /// + /// Exposed as internal (rather than private) solely so unit tests can + /// invoke a deterministic prune cycle without waiting on the cleanup timer. + /// internal void CleanupCallback(object state) { // If the pool is not Running, skip work. Shutdown disposes the timer, but diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs index 96432c8bd6..e5b66917e3 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs @@ -68,41 +68,5 @@ public static void BasicTransactionPoolTest(string connectionString) Assert.Equal(2, connectionPool.ConnectionCount); } - - /// - /// Checks that connections in the transaction pool are not cleaned out, and the root transaction is put into "stasis" when it ages - /// Synapse: only supports local transaction request. - /// - /// - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] - [ClassData(typeof(ConnectionPoolConnectionStringProvider))] - public static void TransactionCleanupTest(string connectionString) - { - SqlConnection.ClearAllPools(); - ConnectionPoolWrapper connectionPool = null; - - using (TransactionScope transScope = new()) - { - using SqlConnection connection1 = new(connectionString); - using SqlConnection connection2 = new(connectionString); - connection1.Open(); - connection2.Open(); - InternalConnectionWrapper internalConnection1 = new(connection1); - connectionPool = new ConnectionPoolWrapper(connection1); - - connectionPool.Cleanup(); - Assert.Equal(2, connectionPool.ConnectionCount); - - connection1.Close(); - connection2.Close(); - connectionPool.Cleanup(); - Assert.Equal(2, connectionPool.ConnectionCount); - - connectionPool.Cleanup(); - Assert.Equal(2, connectionPool.ConnectionCount); - - transScope.Complete(); - } - } } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index 712708375a..d376d8675e 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -896,6 +896,56 @@ public void SequentialTransactions_CanReuseConnections() #endregion + #region Pruning Tests + + [Fact] + public void Pruning_IgnoresTransactedConnections() + { + // Arrange - place a connection into the transacted pool by returning it + // while a transaction is active. The connection lives in + // TransactedConnectionPool, not in the general pool's _stackOld/_stackNew. + using var scope = new TransactionScope(); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + // Sanity: connection sits in the transacted pool, not the general pool. + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + Assert.Equal(0, _pool.IdleCount); + int poolCountBefore = _pool.Count; + + // Act - invoke pruning twice. The cleanup pass moves connections from + // _stackNew to _stackOld on one tick and destroys aged entries on the + // next, so running it twice mirrors a full prune cycle. + var waitHandlePool = (WaitHandleDbConnectionPool)_pool; + waitHandlePool.CleanupCallback(null!); + waitHandlePool.CleanupCallback(null!); + + // Assert - the transacted connection must still be tracked in the + // transacted pool and must not have been destroyed. + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + Assert.True(_pool.TransactedConnectionPool.TransactedConnections.ContainsKey(transaction)); + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + Assert.Equal(poolCountBefore, _pool.Count); + Assert.False(conn.IsConnectionDoomed, + "Transacted connection should not be doomed by the pruning process."); + + // The transacted connection must still be reusable for the same transaction. + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.Same(conn, conn2); + ReturnConnection(conn2, owner2); + + scope.Complete(); + } + + #endregion + #region Mock Classes internal class MockSqlConnectionFactory : SqlConnectionFactory diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs index 566d3d0cef..a75aee1b1e 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs @@ -760,6 +760,12 @@ public async Task TransientFault_Async_ShouldConnectToPrimary_NotFailover(uint e [InlineData(40613)] [InlineData(42108)] [InlineData(42109)] + // Quarantined due to intermittent failure: + // Assert.Equal() Failure: Strings differ + // ↓ (pos 14) + // Expected: "localhost,56862" + // Actual: "localhost,56861" + [Trait("Category", "flaky")] public async Task TransientFault_WithUserProvidedPartner_Async_ShouldConnectToPrimary_NotFailover(uint errorCode) { // Async parity for TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs index ef058c881a..74171300fd 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs @@ -79,6 +79,7 @@ public async Task RequestEncryption_ServerDoesNotSupportEncryption_ShouldFail() [InlineData(40613)] [InlineData(42108)] [InlineData(42109)] + [Trait("Category", "flaky")] public async Task TransientFault_RetryEnabled_ShouldSucceed_Async(uint errorCode) { using TransientTdsErrorTdsServer server = new( @@ -161,6 +162,11 @@ public async Task TransientFault_RetryDisabled_ShouldFail_Async(uint errorCode) [InlineData(40613)] [InlineData(42108)] [InlineData(42109)] + // Quarantined due to intermittent failure: + // Assert.Equal() Failure: Values differ + // Expected: 40613 + // Actual: 42108 + [Trait("Category", "flaky")] public void TransientFault_RetryDisabled_ShouldFail(uint errorCode) { using TransientTdsErrorTdsServer server = new(