From 6db6b63ef38d359f2378ea6ce56777f4c774bd1a Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 11 Nov 2025 17:22:41 -0800 Subject: [PATCH 1/6] 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 387019910b..3989b6c243 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 @@ -1682,8 +1682,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 17d04c79f00a7755febd42cab0fae9f50260428c Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 12:50:54 -0800 Subject: [PATCH 2/6] 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 ++ 4 files changed, 6 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 80466509a6..cfe229341a 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 @@ -978,7 +978,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 4330e5205b..204e1ece49 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 @@ -984,7 +984,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 d4ea183312..cfb24c4302 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -43,6 +43,8 @@ protected internal override DataTable GetSchema(DbConnectionFactory factory, DbC internal override bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) => base.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 b4dfb4f214..f40cbcd20a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -797,6 +797,8 @@ internal void PrePush(object expectedOwner) internal void RemoveWeakReference(object value) => ReferenceCollection?.Remove(value); + internal abstract void ResetConnection(); + internal void SetInStasis() { IsTxRootWaitingForTxEnd = true; From 5de730e41672d0a4da39a8be5eb2cfaa19b9ef94 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 14:16:58 -0800 Subject: [PATCH 3/6] 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 3989b6c243..cb099e27d3 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 @@ -1694,6 +1694,7 @@ public void PutObjectFromTransactedPool(DbConnectionInternal obj) if (State is Running && obj.CanBePooled) { + obj.ResetConnection(); PutNewObject(obj); } else From 549396f25df3def4c7c61958c84d7dcfc2600f3b Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 12 Nov 2025 14:17:04 -0800 Subject: [PATCH 4/6] 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 cfe229341a..2afde02578 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 @@ -977,7 +977,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 204e1ece49..0f8b67301c 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 @@ -983,7 +983,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 cfb24c4302..f4e87010a6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -32,6 +32,7 @@ internal override void CloseConnection(DbConnection owningObject, DbConnectionFa // not much to do here... } + /// protected override void Deactivate() => ADP.ClosedConnectionError(); public override void EnlistTransaction(System.Transactions.Transaction transaction) => throw ADP.ClosedConnectionError(); @@ -44,6 +45,7 @@ protected internal override DataTable GetSchema(DbConnectionFactory factory, DbC internal override bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) => base.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 f40cbcd20a..eafddd8ae2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -797,6 +797,11 @@ internal void PrePush(object 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() @@ -836,6 +841,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); /// @@ -852,6 +862,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 742c8b2865..0ea6c7c621 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs @@ -301,6 +301,7 @@ override protected DbReferenceCollection CreateReferenceCollection() return new SqlReferenceCollection(); } + /// override protected void Deactivate() { TdsParser bestEffortCleanupTarget = null; From f9017cac8adfedd86df2ae8ffc6cdedd3f78abb1 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 14 Nov 2025 14:23:09 -0800 Subject: [PATCH 5/6] 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 4e90bbf6c7..8cbcde5af6 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 488d937d63233f8a5681464d7aaed2faae3174d8 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 18 Nov 2025 10:58:16 -0800 Subject: [PATCH 6/6] Review changes. --- .../ManualTests/TracingTests/MetricsTest.cs | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 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 8cbcde5af6..bd70084fab 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.Reflection; +using System.Threading.Tasks; using System.Transactions; using Xunit; @@ -239,11 +240,81 @@ public void TransactedConnectionPool_VerifyActiveConnectionCounters() if (SupportsActiveConnectionCounters) { Assert.Equal(initialActiveSoftConnections, SqlClientEventSourceProps.ActiveSoftConnections); - Assert.Equal(initialActiveHardConnections+1, SqlClientEventSourceProps.ActiveHardConnections); + Assert.Equal(initialActiveHardConnections + 1, SqlClientEventSourceProps.ActiveHardConnections); Assert.Equal(initialActiveConnections, SqlClientEventSourceProps.ActiveConnections); } } + #if NET + // Note: DbConnection.CloseAsync is not available in .NET Framework + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] + public async Task TransactedConnectionPool_VerifyActiveConnectionCounters_Async() + { + // 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(TransactionScopeAsyncFlowOption.Enabled)) + { + using (var conn = new SqlConnection(stringBuilder.ToString())) + { + await conn.OpenAsync(); + 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); + } + + await conn.CloseAsync(); + + // 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); + } + } + #endif + [ActiveIssue("https://github.com/dotnet/SqlClient/issues/3031")] [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public void ReclaimedConnectionsCounter_Functional()