From 6db296eeb0f474910303ef450f17d76060d62d89 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 11 Nov 2025 17:22:41 -0800 Subject: [PATCH 1/7] Remove extra connection deactivation. --- .../Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | 2 -- 1 file changed, 2 deletions(-) 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 c588fcd3f7..26e729f23a 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 @@ -1379,8 +1379,6 @@ public void PutObjectFromTransactedPool(DbConnectionInternal obj) Debug.Assert(obj != null, "null pooledObject?"); Debug.Assert(obj.EnlistedTransaction == null, "pooledObject is still enlisted?"); - obj.DeactivateConnection(); - // called by the transacted connection pool , once it's removed the // connection from it's list. We put the connection back in general // circulation. From d30ce4c01d3c84f05865a4c9d8ce9f1e805eb50e Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 12:50:54 -0800 Subject: [PATCH 2/7] Expose ResetConnection as internal method. --- .../src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | 2 +- .../src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | 2 +- .../src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs | 2 ++ .../src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | 2 ++ .../UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | 5 +++++ .../UnitTests/ConnectionPool/ConnectionPoolSlotsTest.cs | 5 +++++ .../UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs | 4 ++++ 7 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index 482551fe79..a16b0cc039 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -959,7 +959,7 @@ protected override void InternalDeactivate() } [SuppressMessage("Microsoft.Globalization", "CA1303:DoNotPassLiteralsAsLocalizedParameters")] // copied from Triaged.cs - private void ResetConnection() + internal override void ResetConnection() { // For implicit pooled connections, if connection reset behavior is specified, // reset the database and language properties back to default. It is important diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index 523f1288e5..8ec60dae99 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -967,7 +967,7 @@ protected override void InternalDeactivate() } [SuppressMessage("Microsoft.Globalization", "CA1303:DoNotPassLiteralsAsLocalizedParameters")] // copied from Triaged.cs - private void ResetConnection() + internal override void ResetConnection() { // For implicit pooled connections, if connection reset behavior is specified, // reset the database and language properties back to default. It is important diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs index 8549c48a8d..bf8f4a64d9 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -55,6 +55,8 @@ internal override bool TryOpenConnection( TaskCompletionSource retry, DbConnectionOptions userOptions) => TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + + internal override void ResetConnection() => throw ADP.ClosedConnectionError(); } internal abstract class DbConnectionBusy : DbConnectionClosed 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 6047a4d836..e23d8833ac 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -795,6 +795,8 @@ internal void PrePush(DbConnection expectedOwner) internal void RemoveWeakReference(object value) => ReferenceCollection?.Remove(value); + internal abstract void ResetConnection(); + internal void SetInStasis() { IsTxRootWaitingForTxEnd = true; diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 5e47d618ed..219bb1fdf6 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -777,6 +777,11 @@ protected override void Deactivate() { return; } + + internal override void ResetConnection() + { + return; + } #endregion } #endregion diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ConnectionPoolSlotsTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ConnectionPoolSlotsTest.cs index 90dd4fd8e1..c4f437d981 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ConnectionPoolSlotsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ConnectionPoolSlotsTest.cs @@ -43,6 +43,11 @@ protected override void Deactivate() { // Mock implementation - do nothing } + + internal override void ResetConnection() + { + // Mock implementation - do nothing + } } [Fact] diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs index 8139767c74..3fc31338cd 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs @@ -731,6 +731,10 @@ protected override void Deactivate() // Mock implementation - do nothing } + internal override void ResetConnection() + { + // Mock implementation - do nothing + } public override string ToString() => $"MockConnection_{MockId}"; } From 909ee2005837661c63b374fa452a90d342209ba2 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 14:16:58 -0800 Subject: [PATCH 3/7] Add call to reset connection. --- .../Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | 1 + 1 file changed, 1 insertion(+) 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 26e729f23a..ae56af3bcd 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 @@ -1391,6 +1391,7 @@ public void PutObjectFromTransactedPool(DbConnectionInternal obj) if (State is Running && obj.CanBePooled) { + obj.ResetConnection(); PutNewObject(obj); } else From 8bf5ed842d05da6f1ca3e9485b227d41fdc0e528 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 14:17:04 -0800 Subject: [PATCH 4/7] Add doc comments. --- .../Data/SqlClient/SqlInternalConnectionTds.cs | 2 +- .../Data/SqlClient/SqlInternalConnectionTds.cs | 2 +- .../Data/ProviderBase/DbConnectionClosed.cs | 2 ++ .../Data/ProviderBase/DbConnectionInternal.cs | 15 +++++++++++++++ .../Data/SqlClient/SqlInternalConnection.cs | 1 + 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index a16b0cc039..d3c489f147 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -958,7 +958,7 @@ protected override void InternalDeactivate() } } - [SuppressMessage("Microsoft.Globalization", "CA1303:DoNotPassLiteralsAsLocalizedParameters")] // copied from Triaged.cs + /// internal override void ResetConnection() { // For implicit pooled connections, if connection reset behavior is specified, diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index 8ec60dae99..e5de672c61 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -966,7 +966,7 @@ protected override void InternalDeactivate() } } - [SuppressMessage("Microsoft.Globalization", "CA1303:DoNotPassLiteralsAsLocalizedParameters")] // copied from Triaged.cs + /// internal override void ResetConnection() { // For implicit pooled connections, if connection reset behavior is specified, diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs index bf8f4a64d9..15c32053d6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -33,6 +33,7 @@ internal override void CloseConnection(DbConnection owningObject, SqlConnectionF // not much to do here... } + /// protected override void Deactivate() => ADP.ClosedConnectionError(); public override void EnlistTransaction(System.Transactions.Transaction transaction) => throw ADP.ClosedConnectionError(); @@ -56,6 +57,7 @@ internal override bool TryOpenConnection( DbConnectionOptions userOptions) => TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + /// internal override void ResetConnection() => throw ADP.ClosedConnectionError(); } 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 e23d8833ac..2039e62929 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -795,6 +795,11 @@ internal void PrePush(DbConnection expectedOwner) internal void RemoveWeakReference(object value) => ReferenceCollection?.Remove(value); + /// + /// Idempotently resets the connection so that it may be recycled without leaking state. + /// May preserve transaction state if the connection is enlisted in a distributed transaction. + /// Should be called before the first action is taken on a recycled connection. + /// internal abstract void ResetConnection(); internal void SetInStasis() @@ -834,6 +839,11 @@ internal virtual bool TryReplaceConnection( #region Protected Methods + /// + /// Activates the connection, preparing it for active use. + /// An activated connection has an owner and is checked out from the connection pool (if pooling is enabled). + /// + /// The transaction in which the connection should enlist. protected abstract void Activate(Transaction transaction); /// @@ -850,6 +860,11 @@ protected virtual DbReferenceCollection CreateReferenceCollection() throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject); } + /// + /// Deactivates the connection, cleaning up any state as necessary. + /// A deactivated connection is one that is no longer in active use and does not have an owner. + /// A deactivated connection may be open (connected to a server) and is checked into the connection pool (if pooling is enabled). + /// protected abstract void Deactivate(); protected internal void DoNotPoolThisConnection() diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs index 3d312430d8..46852a5df4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs @@ -246,6 +246,7 @@ override protected DbReferenceCollection CreateReferenceCollection() return new SqlReferenceCollection(); } + /// override protected void Deactivate() { try From 93b6c9d22b93de02730d11d2f91734c2da3c2b4e Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 14 Nov 2025 14:23:09 -0800 Subject: [PATCH 5/7] Add metric test case. --- .../ManualTests/TracingTests/MetricsTest.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs index debb5e7cce..f0b1b53b48 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs @@ -177,6 +177,73 @@ public void StasisCounters_Functional() Assert.Equal(0, SqlClientEventSourceProps.StasisConnections); } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] + public void TransactedConnectionPool_VerifyActiveConnectionCounters() + { + // This test verifies that the active connection count metric never goes negative + // when connections are returned to the pool while enlisted in a transaction. + // This is a regression test for issue #3640 where an extra DeactivateConnection + // call was causing the active connection count to go negative. + + // Arrange + var stringBuilder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) + { + Pooling = true, + Enlist = false, + MinPoolSize = 0, + MaxPoolSize = 10 + }; + + // Clear pools to start fresh + ClearConnectionPools(); + + long initialActiveSoftConnections = SqlClientEventSourceProps.ActiveSoftConnections; + long initialActiveHardConnections = SqlClientEventSourceProps.ActiveHardConnections; + long initialActiveConnections = SqlClientEventSourceProps.ActiveConnections; + + // Act and Assert + // Verify counters at each step in the lifecycle of a transacted connection + using (var txScope = new TransactionScope()) + { + using (var conn = new SqlConnection(stringBuilder.ToString())) + { + conn.Open(); + conn.EnlistTransaction(System.Transactions.Transaction.Current); + + if (SupportsActiveConnectionCounters) + { + // Connection should be active + Assert.Equal(initialActiveSoftConnections + 1, SqlClientEventSourceProps.ActiveSoftConnections); + Assert.Equal(initialActiveHardConnections + 1, SqlClientEventSourceProps.ActiveHardConnections); + Assert.Equal(initialActiveConnections + 1, SqlClientEventSourceProps.ActiveConnections); + } + + conn.Close(); + + // Connection is returned to pool but still in transaction (stasis) + if (SupportsActiveConnectionCounters) + { + // Connection should be deactivated (returned to pool) + Assert.Equal(initialActiveSoftConnections, SqlClientEventSourceProps.ActiveSoftConnections); + Assert.Equal(initialActiveHardConnections + 1, SqlClientEventSourceProps.ActiveHardConnections); + Assert.Equal(initialActiveConnections, SqlClientEventSourceProps.ActiveConnections); + } + } + + // Completing the transaction after the connection is closed ensures that the connection + // is in the transacted pool at the time the transaction ends. This verifies that the + // transition from the transacted pool back to the main pool properly updates the counters. + txScope.Complete(); + } + + if (SupportsActiveConnectionCounters) + { + Assert.Equal(initialActiveSoftConnections, SqlClientEventSourceProps.ActiveSoftConnections); + Assert.Equal(initialActiveHardConnections+1, SqlClientEventSourceProps.ActiveHardConnections); + Assert.Equal(initialActiveConnections, SqlClientEventSourceProps.ActiveConnections); + } + } + [ActiveIssue("https://github.com/dotnet/SqlClient/issues/3031")] [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public void ReclaimedConnectionsCounter_Functional() From c479b18793f7ce8001f4eb858c2d9a47402a7650 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 14 Nov 2025 14:24:02 -0800 Subject: [PATCH 6/7] Fix conditions. --- .../tests/ManualTests/TracingTests/MetricsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs index f0b1b53b48..c9c956ccc6 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs @@ -156,7 +156,7 @@ public void PooledConnectionsCounters_Functional() } } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public void StasisCounters_Functional() { var stringBuilder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { Pooling = false, Enlist = false }; From 32bacec26a8110e5f358f09529c1cbf0a771a830 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 14 Nov 2025 14:25:24 -0800 Subject: [PATCH 7/7] Fix conditions. --- .../tests/ManualTests/TracingTests/MetricsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs index c9c956ccc6..f0b1b53b48 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs @@ -156,7 +156,7 @@ public void PooledConnectionsCounters_Functional() } } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public void StasisCounters_Functional() { var stringBuilder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { Pooling = false, Enlist = false };