From 3ae610474341653cdc26102bde147160b2958de4 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 14:40:50 -0500 Subject: [PATCH 01/19] Move netfx version of DbConnectionInternal.cs --- .../src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Microsoft.Data.SqlClient/{netfx => }/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs (100%) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs similarity index 100% rename from src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs rename to src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs From 67666c022701d72429f3e4f524cdc2c8bd34e172 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 15:06:20 -0500 Subject: [PATCH 02/19] Merge the three files into the common one: * netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs * netcore/src/Common/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs * netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs --- .../Data/ProviderBase/DbConnectionInternal.cs | 136 ++++++++++++++---- 1 file changed, 105 insertions(+), 31 deletions(-) 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 336cb876ab..57d28b92f4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -2,9 +2,19 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +//netcore--- +using System; +using System.Data.Common; +using System.Diagnostics; +using System.Threading; +using System.Transactions; +using Microsoft.Data.Common; +using Microsoft.Data.SqlClient; +//---netcore + namespace Microsoft.Data.ProviderBase { - + //netfx--- using System; using System.Data; using System.Data.Common; @@ -16,6 +26,7 @@ namespace Microsoft.Data.ProviderBase using Microsoft.Data.Common; using Microsoft.Data.SqlClient; using System.Transactions; + //---netfx internal abstract class DbConnectionInternal { @@ -23,6 +34,7 @@ internal abstract class DbConnectionInternal internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; + //netfx--- internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); @@ -39,6 +51,7 @@ internal abstract class DbConnectionInternal private bool _connectionIsDoomed; // true when the connection should no longer be used. private bool _cannotBePooled; // true when the connection should no longer be pooled. + //---netfx private bool _isInStasis; private DateTime _createTime; // when the connection was created. @@ -56,7 +69,7 @@ internal abstract class DbConnectionInternal #endif //DEBUG protected DbConnectionInternal() : this(ConnectionState.Open, true, false) - { // V1.1.3300 + { //netfx // V1.1.3300 } // Constructor for internal connections @@ -219,6 +232,7 @@ internal bool IsTxRootWaitingForTxEnd } } + //netfx--- /// /// Get boolean that specifies whether an enlisted transaction can be unbound from /// the connection when that transaction completes. @@ -268,7 +282,7 @@ internal bool IsEmancipated // (DbConnectionPool.Clear and ReclaimEmancipatedObjects // do this for us) - // Remember how this works (I keep getting confused...) + // The functionality is as follows: // // _pooledCount is incremented when the connection is pushed into the pool // _pooledCount is decremented when the connection is popped from the pool @@ -276,17 +290,18 @@ internal bool IsEmancipated // // That means that: // - // _pooledCount > 1 connection is in the pool multiple times (this is a serious bug...) + // _pooledCount > 1 connection is in the pool multiple times (This should not happen) // _pooledCount == 1 connection is in the pool // _pooledCount == 0 connection is out of the pool // _pooledCount == -1 connection is not a pooled connection; we shouldn't be here for non-pooled connections. - // _pooledCount < -1 connection out of the pool multiple times (not sure how this could happen...) + // _pooledCount < -1 connection out of the pool multiple times // // Now, our job is to return TRUE when the connection is out // of the pool and it's owning object is no longer around to // return it. - return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + //netfx return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + //netcore return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); } } @@ -329,6 +344,7 @@ internal DbConnectionPool Pool } } + //netfx--- protected DbConnectionPoolCounters PerformanceCounters { get @@ -336,6 +352,33 @@ protected DbConnectionPoolCounters PerformanceCounters return _performanceCounters; } } + //---netfx + //netcore--- + virtual protected bool UnbindOnTransactionCompletion + { + get + { + return true; + } + } + + // Is this a connection that must be put in stasis (or is already in stasis) pending the end of it's transaction? + virtual protected internal bool IsNonPoolableTransactionRoot + { + get + { + return false; // if you want to have delegated transactions that are non-poolable, you better override this... + } + } + + virtual internal bool IsTransactionRoot + { + get + { + return false; // if you want to have delegated transactions, you better override this... + } + } + //---netcore virtual protected bool ReadyToPrepareTransaction { @@ -399,7 +442,8 @@ internal void ActivateConnection(Transaction transaction) Activate(transaction); - PerformanceCounters.NumberOfActiveConnections.Increment(); + //netfx PerformanceCounters.NumberOfActiveConnections.Increment(); + //netcore SqlClientEventSource.Log.EnterActiveConnection(); } internal void AddWeakReference(object value, int tag) @@ -419,7 +463,8 @@ internal void AddWeakReference(object value, int tag) virtual public void ChangeDatabase(string value) { - throw ADP.MethodNotImplemented("ChangeDatabase"); + //netfx throw ADP.MethodNotImplemented("ChangeDatabase"); + //netcore throw ADP.MethodNotImplemented(); } internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) @@ -476,7 +521,8 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac lock (this) { - object lockToken = ObtainAdditionalLocksForClose(); + //netfx object lockToken = ObtainAdditionalLocksForClose(); + //netcore bool lockToken = ObtainAdditionalLocksForClose(); try { PrepareForCloseConnection(); @@ -492,15 +538,16 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac if (connectionPool != null) { connectionPool.PutObject(this, owningObject); // PutObject calls Deactivate for us... - // NOTE: Before we leave the PutObject call, another - // thread may have already popped the connection from - // the pool, so don't expect to be able to verify it. + // NOTE: Before we leave the PutObject call, another + // thread may have already popped the connection from + // the pool, so don't expect to be able to verify it. } else { Deactivate(); // ensure we de-activate non-pooled connections, or the data readers and transactions may not get cleaned up... - PerformanceCounters.HardDisconnectsPerSecond.Increment(); + //netfx PerformanceCounters.HardDisconnectsPerSecond.Increment(); + //netcore SqlClientEventSource.Log.HardDisconnectRequest(); // To prevent an endless recursion, we need to clear // the owning object before we call dispose so that @@ -517,11 +564,17 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac } else { + //netfx--- PerformanceCounters.NumberOfNonPooledConnections.Decrement(); if (this.GetType() != typeof(Microsoft.Data.SqlClient.SqlInternalConnectionSmi)) { Dispose(); } + //---netfx + //netcore--- + SqlClientEventSource.Log.ExitNonPooledConnection(); + Dispose(); + //---netcore } } } @@ -546,12 +599,21 @@ virtual protected void PrepareForCloseConnection() // By default, there is no preparation required } + //netfx--- virtual protected object ObtainAdditionalLocksForClose() { return null; // no additional locks in default implementation } + //---netfx + //netcore--- + virtual protected bool ObtainAdditionalLocksForClose() + { + return false; // no additional locks in default implementation + } + //---netcore - virtual protected void ReleaseAdditionalLocksForClose(object lockToken) + //netfx virtual protected void ReleaseAdditionalLocksForClose(object lockToken) + //netcore virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) { // no additional locks in default implementation } @@ -570,20 +632,25 @@ internal void DeactivateConnection() SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Deactivating", ObjectID); #if DEBUG int activateCount = Interlocked.Decrement(ref _activateCount); - Debug.Assert(0 == activateCount, "activated multiple times?"); + //netfx Debug.Assert(0 == activateCount, "activated multiple times?"); #endif // DEBUG + //netfx--- if (PerformanceCounters != null) { // Pool.Clear will DestroyObject that will clean performanceCounters before going here PerformanceCounters.NumberOfActiveConnections.Decrement(); } + //---netfx + //netcore--- + SqlClientEventSource.Log.ExitActiveConnection(); + //---netcore if (!_connectionIsDoomed && Pool.UseLoadBalancing) { // If we're not already doomed, check the connection's lifetime and // doom it if it's lifetime has elapsed. - DateTime now = DateTime.UtcNow; // WebData 111116 + DateTime now = DateTime.UtcNow; if ((now.Ticks - _createTime.Ticks) > Pool.LoadBalanceTimeout.Ticks) { DoNotPoolThisConnection(); @@ -635,7 +702,8 @@ virtual internal void DelegatedTransactionEnded() // once and for all, or the server will have fits about us // leaving connections open until the client-side GC kicks // in. - PerformanceCounters.NumberOfNonPooledConnections.Decrement(); + //netfx PerformanceCounters.NumberOfNonPooledConnections.Decrement(); + //netcore SqlClientEventSource.Log.ExitNonPooledConnection(); Dispose(); } // When _pooledCount is 0, the connection is a pooled connection @@ -647,7 +715,7 @@ virtual internal void DelegatedTransactionEnded() public virtual void Dispose() { _connectionPool = null; - _performanceCounters = null; + //netfx _performanceCounters = null; _connectionIsDoomed = true; _enlistedTransactionOriginal = null; // should not be disposed @@ -668,7 +736,7 @@ protected internal void DoNotPoolThisConnection() } /// Ensure that this connection cannot be put back into the pool. - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + //netfx [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] protected internal void DoomThisConnection() { _connectionIsDoomed = true; @@ -683,7 +751,8 @@ protected internal void UnDoomThisConnection() abstract public void EnlistTransaction(Transaction transaction); - virtual protected internal DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + //netfx virtual protected internal DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + //netcore protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) { Debug.Assert(outerConnection != null, "outerConnection may not be null."); @@ -693,13 +762,14 @@ virtual protected internal DataTable GetSchema(DbConnectionFactory factory, DbCo return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); } - internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters) + //netfx internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters) + //netcore internal void MakeNonPooledObject(DbConnection owningObject) { // Used by DbConnectionFactory to indicate that this object IS NOT part of // a connection pool. _connectionPool = null; - _performanceCounters = performanceCounters; + //netfx _performanceCounters = performanceCounters; _owningObject.SetTarget(owningObject); _pooledCount = -1; } @@ -709,11 +779,12 @@ internal void MakePooledConnection(DbConnectionPool connectionPool) // Used by DbConnectionFactory to indicate that this object IS part of // a connection pool. - // TODO: consider using ADP.TimerCurrent() for this. - _createTime = DateTime.UtcNow; // WebData 111116 + //netfx // TODO: consider using ADP.TimerCurrent() for this. + //netfx _createTime = DateTime.UtcNow; // WebData 111116 + //netcore _createTime = DateTime.UtcNow; _connectionPool = connectionPool; - _performanceCounters = connectionPool.PerformanceCounters; + //netfx _performanceCounters = connectionPool.PerformanceCounters; } internal void NotifyWeakReference(int message) @@ -745,7 +816,8 @@ internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnecti internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { - throw ADP.MethodNotImplemented("TryReplaceConnection"); + //netfx throw ADP.MethodNotImplemented("TryReplaceConnection"); + //netcore throw ADP.MethodNotImplemented(); } protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) @@ -836,7 +908,7 @@ internal void PostPop(DbConnection newOwner) _owningObject.SetTarget(newOwner); _pooledCount--; - //DbConnection x = (newOwner as DbConnection); + //netfx //DbConnection x = (newOwner as DbConnection); SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); //3 // The following tests are retail assertions of things we can't allow to happen. @@ -945,8 +1017,8 @@ void TransactionCompletedEvent(object sender, TransactionEventArgs e) } - // TODO: Review whether we need the unmanaged code permission when we have the new object model available. - [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] + //netfx // TODO: Review whether we need the unmanaged code permission when we have the new object model available. + //netfx [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] private void TransactionOutcomeEnlist(Transaction transaction) { _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); @@ -957,7 +1029,8 @@ internal void SetInStasis() { _isInStasis = true; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); - PerformanceCounters.NumberOfStasisConnections.Increment(); + //netfx PerformanceCounters.NumberOfStasisConnections.Increment(); + //netcore SqlClientEventSource.Log.EnterStasisConnection(); } private void TerminateStasis(bool returningToPool) @@ -971,7 +1044,8 @@ private void TerminateStasis(bool returningToPool) SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed/leaked. Disposing.", ObjectID); } - PerformanceCounters.NumberOfStasisConnections.Decrement(); + //netfx PerformanceCounters.NumberOfStasisConnections.Decrement(); + //netcore SqlClientEventSource.Log.ExitStasisConnection(); _isInStasis = false; } From 94b4c594d565c312b4b84b746fce2fd08d0fdcba Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 15:08:48 -0500 Subject: [PATCH 03/19] Ensure System.Transactions is included in the common project --- .../src/Microsoft.Data.SqlClient.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj index d8e41155af..d72711b7e4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj @@ -4,5 +4,8 @@ net6.0;net8.0;net462 + + + From 5274f73d746a9326c85650f66f915b3fa2047806 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 15:15:28 -0500 Subject: [PATCH 04/19] Bring in the DbConnectionPoolCounters from netfx project --- .../Data/ProviderBase/DbConnectionPoolCounters.netfx.cs} | 4 ++++ 1 file changed, 4 insertions(+) rename src/Microsoft.Data.SqlClient/{netfx/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.cs => src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.netfx.cs} (99%) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.netfx.cs similarity index 99% rename from src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.cs rename to src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.netfx.cs index 72823f2c5a..e20bc410ca 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPoolCounters.netfx.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#if NETFRAMEWORK + namespace Microsoft.Data.ProviderBase { @@ -353,3 +355,5 @@ private DbConnectionPoolCountersNoCounters() : base() } } } + +#endif From 95eab471578d280755c6763f0821a5303b51a52c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 15:42:38 -0500 Subject: [PATCH 05/19] Add stubs for types used in DbConnectionInternal --- .../ProviderBase/DbConnectionPool.stub.cs | 31 +++++++++++++++++++ .../SqlInternalConnectionSmi.stub.cs | 16 ++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs new file mode 100644 index 0000000000..a548d3d4d8 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs @@ -0,0 +1,31 @@ +// 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.Transactions; + +namespace Microsoft.Data.ProviderBase +{ + // DO NOT USE THIS FILE IN ANY PROJECT! + // This is a temporary stub to enable migrating DbConnectionInternal to the common project. + internal class DbConnectionPool + { + internal TimeSpan LoadBalanceTimeout => throw new NotImplementedException("STUB"); + + #if NETFRAMEWORK + internal DbConnectionPoolCounters PerformanceCounters => throw new NotImplementedException("STUB"); + #endif + + internal bool UseLoadBalancing => throw new NotImplementedException("STUB"); + + internal void PutObject(DbConnectionInternal obj, object owningObject) => + throw new NotImplementedException("STUB"); + + internal void PutObjectFromTransactedPool(DbConnectionInternal obj) => + throw new NotImplementedException("STUB"); + + internal void TransactionEnded(Transaction transaction, DbConnectionInternal transactedObject) => + throw new NotImplementedException("STUB"); + } +} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs new file mode 100644 index 0000000000..aaafc74b5a --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs @@ -0,0 +1,16 @@ +// 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. + +#if NETFRAMEWORK + +namespace Microsoft.Data.SqlClient +{ + // DO NOT USE THIS FILE IN ANY PROJECT! + // This is a temporary stub to enable migrating DbConnectionInternal to the common project. + internal class SqlInternalConnectionSmi + { + } +} + +#endif From 9be0990b8fc5c88795ede6f6269e183d8af4be39 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 15:49:01 -0500 Subject: [PATCH 06/19] Resolve conflicts from merge file --- .../Data/ProviderBase/DbConnectionInternal.cs | 192 ++++++++---------- 1 file changed, 88 insertions(+), 104 deletions(-) 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 57d28b92f4..13d191555c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -2,39 +2,29 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -//netcore--- using System; +using System.Data; using System.Data.Common; using System.Diagnostics; using System.Threading; +using System.Threading.Tasks; using System.Transactions; using Microsoft.Data.Common; using Microsoft.Data.SqlClient; -//---netcore + +#if NETFRAMEWORK +using System.Runtime.ConstrainedExecution; +using System.Security.Permissions; +#endif namespace Microsoft.Data.ProviderBase { - //netfx--- - using System; - using System.Data; - using System.Data.Common; - using System.Diagnostics; - using System.Runtime.ConstrainedExecution; - using System.Security.Permissions; - using System.Threading; - using System.Threading.Tasks; - using Microsoft.Data.Common; - using Microsoft.Data.SqlClient; - using System.Transactions; - //---netfx - internal abstract class DbConnectionInternal { private static int _objectTypeCount; internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; - //netfx--- internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); @@ -45,13 +35,11 @@ internal abstract class DbConnectionInternal private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only) - private DbConnectionPoolCounters _performanceCounters; // the performance counters we're supposed to update private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated private int _pooledCount; // [usage must be thread safe] the number of times this object has been pushed into the pool less the number of times it's been popped (0 != inPool) private bool _connectionIsDoomed; // true when the connection should no longer be used. private bool _cannotBePooled; // true when the connection should no longer be pooled. - //---netfx private bool _isInStasis; private DateTime _createTime; // when the connection was created. @@ -64,12 +52,16 @@ internal abstract class DbConnectionInternal // Also, this reference should not be disposed, since we aren't taking ownership of it. private Transaction _enlistedTransactionOriginal; -#if DEBUG + #if NETFRAMEWORK + private DbConnectionPoolCounters _performanceCounters; // the performance counters we're supposed to update + #endif + + #if DEBUG private int _activateCount; // debug only counter to verify activate/deactivates are in sync. -#endif //DEBUG + #endif protected DbConnectionInternal() : this(ConnectionState.Open, true, false) - { //netfx // V1.1.3300 + { } // Constructor for internal connections @@ -232,7 +224,6 @@ internal bool IsTxRootWaitingForTxEnd } } - //netfx--- /// /// Get boolean that specifies whether an enlisted transaction can be unbound from /// the connection when that transaction completes. @@ -300,8 +291,11 @@ internal bool IsEmancipated // of the pool and it's owning object is no longer around to // return it. - //netfx return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - //netcore return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + #if NETFRAMEWORK + return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + #else + return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + #endif } } @@ -344,7 +338,7 @@ internal DbConnectionPool Pool } } - //netfx--- + #if NETFRAMEWORK protected DbConnectionPoolCounters PerformanceCounters { get @@ -352,33 +346,7 @@ protected DbConnectionPoolCounters PerformanceCounters return _performanceCounters; } } - //---netfx - //netcore--- - virtual protected bool UnbindOnTransactionCompletion - { - get - { - return true; - } - } - - // Is this a connection that must be put in stasis (or is already in stasis) pending the end of it's transaction? - virtual protected internal bool IsNonPoolableTransactionRoot - { - get - { - return false; // if you want to have delegated transactions that are non-poolable, you better override this... - } - } - - virtual internal bool IsTransactionRoot - { - get - { - return false; // if you want to have delegated transactions, you better override this... - } - } - //---netcore + #endif virtual protected bool ReadyToPrepareTransaction { @@ -442,8 +410,11 @@ internal void ActivateConnection(Transaction transaction) Activate(transaction); - //netfx PerformanceCounters.NumberOfActiveConnections.Increment(); - //netcore SqlClientEventSource.Log.EnterActiveConnection(); + #if NETFRAMEWORK + PerformanceCounters.NumberOfActiveConnections.Increment(); + #else + SqlClientEventSource.Log.EnterActiveConnection(); + #endif } internal void AddWeakReference(object value, int tag) @@ -463,8 +434,7 @@ internal void AddWeakReference(object value, int tag) virtual public void ChangeDatabase(string value) { - //netfx throw ADP.MethodNotImplemented("ChangeDatabase"); - //netcore throw ADP.MethodNotImplemented(); + throw ADP.MethodNotImplemented(); } internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) @@ -520,9 +490,7 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac // Lock to prevent race condition with cancellation lock (this) { - - //netfx object lockToken = ObtainAdditionalLocksForClose(); - //netcore bool lockToken = ObtainAdditionalLocksForClose(); + bool lockToken = ObtainAdditionalLocksForClose(); try { PrepareForCloseConnection(); @@ -546,8 +514,11 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac { Deactivate(); // ensure we de-activate non-pooled connections, or the data readers and transactions may not get cleaned up... - //netfx PerformanceCounters.HardDisconnectsPerSecond.Increment(); - //netcore SqlClientEventSource.Log.HardDisconnectRequest(); + #if NETFRAMEWORK + PerformanceCounters.HardDisconnectsPerSecond.Increment(); + #else + SqlClientEventSource.Log.HardDisconnectRequest(); + #endif // To prevent an endless recursion, we need to clear // the owning object before we call dispose so that @@ -564,17 +535,16 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac } else { - //netfx--- + #if NETFRAMEWORK PerformanceCounters.NumberOfNonPooledConnections.Decrement(); if (this.GetType() != typeof(Microsoft.Data.SqlClient.SqlInternalConnectionSmi)) { Dispose(); } - //---netfx - //netcore--- + #else SqlClientEventSource.Log.ExitNonPooledConnection(); Dispose(); - //---netcore + #endif } } } @@ -599,21 +569,12 @@ virtual protected void PrepareForCloseConnection() // By default, there is no preparation required } - //netfx--- - virtual protected object ObtainAdditionalLocksForClose() - { - return null; // no additional locks in default implementation - } - //---netfx - //netcore--- virtual protected bool ObtainAdditionalLocksForClose() { return false; // no additional locks in default implementation } - //---netcore - //netfx virtual protected void ReleaseAdditionalLocksForClose(object lockToken) - //netcore virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) + virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) { // no additional locks in default implementation } @@ -632,18 +593,19 @@ internal void DeactivateConnection() SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Deactivating", ObjectID); #if DEBUG int activateCount = Interlocked.Decrement(ref _activateCount); - //netfx Debug.Assert(0 == activateCount, "activated multiple times?"); + #if NETFRAMEWORK + Debug.Assert(0 == activateCount, "activated multiple times?"); + #endif #endif // DEBUG - //netfx--- + #if NETFRAMEWORK if (PerformanceCounters != null) { // Pool.Clear will DestroyObject that will clean performanceCounters before going here PerformanceCounters.NumberOfActiveConnections.Decrement(); } - //---netfx - //netcore--- + #else SqlClientEventSource.Log.ExitActiveConnection(); - //---netcore + #endif if (!_connectionIsDoomed && Pool.UseLoadBalancing) { @@ -702,8 +664,12 @@ virtual internal void DelegatedTransactionEnded() // once and for all, or the server will have fits about us // leaving connections open until the client-side GC kicks // in. - //netfx PerformanceCounters.NumberOfNonPooledConnections.Decrement(); - //netcore SqlClientEventSource.Log.ExitNonPooledConnection(); + #if NETFRAMEWORK + PerformanceCounters.NumberOfNonPooledConnections.Decrement(); + #else + SqlClientEventSource.Log.ExitNonPooledConnection(); + #endif + Dispose(); } // When _pooledCount is 0, the connection is a pooled connection @@ -715,10 +681,13 @@ virtual internal void DelegatedTransactionEnded() public virtual void Dispose() { _connectionPool = null; - //netfx _performanceCounters = null; _connectionIsDoomed = true; _enlistedTransactionOriginal = null; // should not be disposed + #if NETFRAMEWORK + _performanceCounters = null; + #endif + // Dispose of the _enlistedTransaction since it is a clone // of the original reference. // VSDD 780271 - _enlistedTransaction can be changed by another thread (TX end event) @@ -736,7 +705,9 @@ protected internal void DoNotPoolThisConnection() } /// Ensure that this connection cannot be put back into the pool. - //netfx [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + #if NETFRAMEWORK + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + #endif protected internal void DoomThisConnection() { _connectionIsDoomed = true; @@ -751,8 +722,7 @@ protected internal void UnDoomThisConnection() abstract public void EnlistTransaction(Transaction transaction); - //netfx virtual protected internal DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) - //netcore protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) { Debug.Assert(outerConnection != null, "outerConnection may not be null."); @@ -762,14 +732,20 @@ protected internal void UnDoomThisConnection() return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); } - //netfx internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters) - //netcore internal void MakeNonPooledObject(DbConnection owningObject) + #if NETFRAMEWORK + internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters) + #else + internal void MakeNonPooledObject(DbConnection owningObject) + #endif { // Used by DbConnectionFactory to indicate that this object IS NOT part of // a connection pool. + #if NETFRAMEWORK + _performanceCounters = performanceCounters; + #endif + _connectionPool = null; - //netfx _performanceCounters = performanceCounters; _owningObject.SetTarget(owningObject); _pooledCount = -1; } @@ -778,13 +754,13 @@ internal void MakePooledConnection(DbConnectionPool connectionPool) { // Used by DbConnectionFactory to indicate that this object IS part of // a connection pool. - - //netfx // TODO: consider using ADP.TimerCurrent() for this. - //netfx _createTime = DateTime.UtcNow; // WebData 111116 - //netcore _createTime = DateTime.UtcNow; + _createTime = DateTime.UtcNow; _connectionPool = connectionPool; - //netfx _performanceCounters = connectionPool.PerformanceCounters; + + #if NETFRAMEWORK + _performanceCounters = connectionPool.PerformanceCounters; + #endif } internal void NotifyWeakReference(int message) @@ -816,8 +792,7 @@ internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnecti internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { - //netfx throw ADP.MethodNotImplemented("TryReplaceConnection"); - //netcore throw ADP.MethodNotImplemented(); + throw ADP.MethodNotImplemented(); } protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) @@ -908,7 +883,6 @@ internal void PostPop(DbConnection newOwner) _owningObject.SetTarget(newOwner); _pooledCount--; - //netfx //DbConnection x = (newOwner as DbConnection); SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); //3 // The following tests are retail assertions of things we can't allow to happen. @@ -1017,8 +991,10 @@ void TransactionCompletedEvent(object sender, TransactionEventArgs e) } - //netfx // TODO: Review whether we need the unmanaged code permission when we have the new object model available. - //netfx [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] + #if NETFRAMEWORK + // TODO: Review whether we need the unmanaged code permission when we have the new object model available. + [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] + #endif private void TransactionOutcomeEnlist(Transaction transaction) { _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); @@ -1029,8 +1005,12 @@ internal void SetInStasis() { _isInStasis = true; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); - //netfx PerformanceCounters.NumberOfStasisConnections.Increment(); - //netcore SqlClientEventSource.Log.EnterStasisConnection(); + + #if NETFRAMEWORK + PerformanceCounters.NumberOfStasisConnections.Increment(); + #else + SqlClientEventSource.Log.EnterStasisConnection(); + #endif } private void TerminateStasis(bool returningToPool) @@ -1044,8 +1024,12 @@ private void TerminateStasis(bool returningToPool) SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed/leaked. Disposing.", ObjectID); } - //netfx PerformanceCounters.NumberOfStasisConnections.Decrement(); - //netcore SqlClientEventSource.Log.ExitStasisConnection(); + #if NETFRAMEWORK + PerformanceCounters.NumberOfStasisConnections.Decrement(); + #else + SqlClientEventSource.Log.ExitStasisConnection(); + #endif + _isInStasis = false; } From 518328e4bc6a94775d5d127183d74908d0bcd2f7 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 16:24:32 -0500 Subject: [PATCH 07/19] External changes following merge * Just use a bool as the lock token --- .../Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 96b9f9635a..d0c64d07e3 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 @@ -2333,7 +2333,7 @@ internal void FailoverPermissionDemand() // PREPARED COMMAND METHODS //////////////////////////////////////////////////////////////////////////////////////// - protected override object ObtainAdditionalLocksForClose() + protected override bool ObtainAdditionalLocksForClose() { bool obtainParserLock = !ThreadHasParserLockForClose; Debug.Assert(obtainParserLock || _parserLock.ThreadMayHaveLock(), "Thread claims to have lock, but lock is not taken"); @@ -2345,10 +2345,9 @@ protected override object ObtainAdditionalLocksForClose() return obtainParserLock; } - protected override void ReleaseAdditionalLocksForClose(object lockToken) + protected override void ReleaseAdditionalLocksForClose(bool lockToken) { - Debug.Assert(lockToken is bool, "Lock token should be boolean"); - if ((bool)lockToken) + if (lockToken) { ThreadHasParserLockForClose = false; _parserLock.Release(); From 3146c6a59ad7fbcee731ca7f7776731b8270718d Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 16:24:50 -0500 Subject: [PATCH 08/19] Remove netcore version of the files --- .../Data/ProviderBase/DbConnectionInternal.cs | 454 ---------------- .../Data/ProviderBase/DbConnectionInternal.cs | 512 ------------------ 2 files changed, 966 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Common/Microsoft/Data/ProviderBase/DbConnectionInternal.cs delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Common/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/netcore/src/Common/Microsoft/Data/ProviderBase/DbConnectionInternal.cs deleted file mode 100644 index fd5e3e2848..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Common/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ /dev/null @@ -1,454 +0,0 @@ -// 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 Microsoft.Data.Common; -using Microsoft.Data.SqlClient; -using System; -using System.Data; -using System.Data.Common; -using System.Diagnostics; -using System.Threading; -using System.Threading.Tasks; -using System.Transactions; - - -namespace Microsoft.Data.ProviderBase -{ - internal abstract partial class DbConnectionInternal - { - internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); - internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); - - private readonly bool _allowSetConnectionString; - private readonly bool _hidePassword; - private readonly ConnectionState _state; - - private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) - - private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only) - private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated - private int _pooledCount; // [usage must be thread safe] the number of times this object has been pushed into the pool less the number of times it's been popped (0 != inPool) - - private bool _connectionIsDoomed; // true when the connection should no longer be used. - private bool _cannotBePooled; // true when the connection should no longer be pooled. - - private DateTime _createTime; // when the connection was created. - -#if DEBUG - private int _activateCount; // debug only counter to verify activate/deactivates are in sync. -#endif //DEBUG - - protected DbConnectionInternal() : this(ConnectionState.Open, true, false) - { - } - - // Constructor for internal connections - internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool allowSetConnectionString) - { - _allowSetConnectionString = allowSetConnectionString; - _hidePassword = hidePassword; - _state = state; - } - - internal bool AllowSetConnectionString - { - get - { - return _allowSetConnectionString; - } - } - - internal bool CanBePooled - { - get - { - return (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out _)); - } - } - - protected internal bool IsConnectionDoomed - { - get - { - return _connectionIsDoomed; - } - } - - internal bool IsEmancipated - { - get - { - // NOTE: There are race conditions between PrePush, PostPop and this - // property getter -- only use this while this object is locked; - // (DbConnectionPool.Clear and ReclaimEmancipatedObjects - // do this for us) - - // The functionality is as follows: - // - // _pooledCount is incremented when the connection is pushed into the pool - // _pooledCount is decremented when the connection is popped from the pool - // _pooledCount is set to -1 when the connection is not pooled (just in case...) - // - // That means that: - // - // _pooledCount > 1 connection is in the pool multiple times (This should not happen) - // _pooledCount == 1 connection is in the pool - // _pooledCount == 0 connection is out of the pool - // _pooledCount == -1 connection is not a pooled connection; we shouldn't be here for non-pooled connections. - // _pooledCount < -1 connection out of the pool multiple times - // - // Now, our job is to return TRUE when the connection is out - // of the pool and it's owning object is no longer around to - // return it. - - return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - } - } - - internal bool IsInPool - { - get - { - Debug.Assert(_pooledCount <= 1 && _pooledCount >= -1, "Pooled count for object is invalid"); - return (_pooledCount == 1); - } - } - - - protected internal DbConnection Owner - { - // We use a weak reference to the owning object so we can identify when - // it has been garbage collected without throwing exceptions. - get - { - if (_owningObject.TryGetTarget(out DbConnection connection)) - { - return connection; - } - return null; - } - } - - internal DbConnectionPool Pool - { - get - { - return _connectionPool; - } - } - - protected internal DbReferenceCollection ReferenceCollection - { - get - { - return _referenceCollection; - } - } - - abstract public string ServerVersion - { - get; - } - - // this should be abstract but until it is added to all the providers virtual will have to do - virtual public string ServerVersionNormalized - { - get - { - throw ADP.NotSupported(); - } - } - - public bool ShouldHidePassword - { - get - { - return _hidePassword; - } - } - - public ConnectionState State - { - get - { - return _state; - } - } - - internal void AddWeakReference(object value, int tag) - { - if (_referenceCollection == null) - { - _referenceCollection = CreateReferenceCollection(); - if (_referenceCollection == null) - { - throw ADP.InternalError(ADP.InternalErrorCode.CreateReferenceCollectionReturnedNull); - } - } - _referenceCollection.Add(value, tag); - } - - abstract public DbTransaction BeginTransaction(System.Data.IsolationLevel il); - - virtual public void ChangeDatabase(string value) - { - throw ADP.MethodNotImplemented(); - } - - virtual internal void PrepareForReplaceConnection() - { - // By default, there is no preparation required - } - - virtual protected void PrepareForCloseConnection() - { - // By default, there is no preparation required - } - - virtual protected bool ObtainAdditionalLocksForClose() - { - return false; // no additional locks in default implementation - } - - virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) - { - // no additional locks in default implementation - } - - virtual protected DbReferenceCollection CreateReferenceCollection() - { - throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject); - } - - abstract protected void Deactivate(); - - internal void DeactivateConnection() - { - // Internal method called from the connection pooler so we don't expose - // the Deactivate method publicly. - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Deactivating", ObjectID); - -#if DEBUG - int activateCount = Interlocked.Decrement(ref _activateCount); -#endif // DEBUG - - SqlClientEventSource.Log.ExitActiveConnection(); - - if (!_connectionIsDoomed && Pool.UseLoadBalancing) - { - // If we're not already doomed, check the connection's lifetime and - // doom it if it's lifetime has elapsed. - - DateTime now = DateTime.UtcNow; - if ((now.Ticks - _createTime.Ticks) > Pool.LoadBalanceTimeout.Ticks) - { - DoNotPoolThisConnection(); - } - } - Deactivate(); - } - - protected internal void DoNotPoolThisConnection() - { - _cannotBePooled = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Marking pooled object as non-poolable so it will be disposed", ObjectID); - } - - /// Ensure that this connection cannot be put back into the pool. - protected internal void DoomThisConnection() - { - _connectionIsDoomed = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Dooming", ObjectID); - } - - // Reset connection doomed status so it can be re-connected and pooled. - protected internal void UnDoomThisConnection() - { - _connectionIsDoomed = false; - } - - protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) - { - Debug.Assert(outerConnection != null, "outerConnection may not be null."); - - DbMetaDataFactory metaDataFactory = factory.GetMetaDataFactory(poolGroup, this); - Debug.Assert(metaDataFactory != null, "metaDataFactory may not be null."); - - return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); - } - - internal void MakeNonPooledObject(DbConnection owningObject) - { - // Used by DbConnectionFactory to indicate that this object IS NOT part of - // a connection pool. - - _connectionPool = null; - _owningObject.SetTarget(owningObject); - _pooledCount = -1; - } - - internal void MakePooledConnection(DbConnectionPool connectionPool) - { - // Used by DbConnectionFactory to indicate that this object IS part of - // a connection pool. - _createTime = DateTime.UtcNow; - - _connectionPool = connectionPool; - } - - internal void NotifyWeakReference(int message) - { - DbReferenceCollection referenceCollection = ReferenceCollection; - if (referenceCollection != null) - { - referenceCollection.Notify(message); - } - } - - internal virtual void OpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory) - { - if (!TryOpenConnection(outerConnection, connectionFactory, null, null)) - { - throw ADP.InternalError(ADP.InternalErrorCode.SynchronousConnectReturnedPending); - } - } - - /// The default implementation is for the open connection objects, and - /// it simply throws. Our private closed-state connection objects - /// override this and do the correct thing. - // User code should either override DbConnectionInternal.Activate when it comes out of the pool - // or override DbConnectionFactory.CreateConnection when the connection is created for non-pooled connections - internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - throw ADP.ConnectionAlreadyOpen(State); - } - - internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - throw ADP.MethodNotImplemented(); - } - - protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - // ?->Connecting: prevent set_ConnectionString during Open - if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) - { - DbConnectionInternal openConnection = null; - try - { - connectionFactory.PermissionDemand(outerConnection); - if (!connectionFactory.TryGetConnection(outerConnection, retry, userOptions, this, out openConnection)) - { - return false; - } - } - catch - { - // This should occur for all exceptions, even ADP.UnCatchableExceptions. - connectionFactory.SetInnerConnectionTo(outerConnection, this); - throw; - } - if (openConnection == null) - { - connectionFactory.SetInnerConnectionTo(outerConnection, this); - throw ADP.InternalConnectionError(ADP.ConnectionError.GetConnectionReturnsNull); - } - connectionFactory.SetInnerConnectionEvent(outerConnection, openConnection); - } - - return true; - } - - internal void PrePush(object expectedOwner) - { - // Called by DbConnectionPool when we're about to be put into it's pool, we - // take this opportunity to ensure ownership and pool counts are legit. - - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. - - //3 // The following tests are retail assertions of things we can't allow to happen. - bool isAlive = _owningObject.TryGetTarget(out DbConnection connection); - if (expectedOwner == null) - { - if (isAlive) - { - throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasOwner); // new unpooled object has an owner - } - } - else if (isAlive && connection != expectedOwner) - { - throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasWrongOwner); // unpooled object has incorrect owner - } - if (0 != _pooledCount) - { - throw ADP.InternalError(ADP.InternalErrorCode.PushingObjectSecondTime); // pushing object onto stack a second time - } - - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to push into pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); - _pooledCount++; - _owningObject.SetTarget(null); // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2% - } - - internal void PostPop(DbConnection newOwner) - { - // Called by DbConnectionPool right after it pulls this from it's pool, we - // take this opportunity to ensure ownership and pool counts are legit. - - Debug.Assert(!IsEmancipated, "pooled object not in pool"); - - // When another thread is clearing this pool, it - // will doom all connections in this pool without prejudice which - // causes the following assert to fire, which really mucks up stress - // against checked bits. The assert is benign, so we're commenting - // it out. - //Debug.Assert(CanBePooled, "pooled object is not poolable"); - - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. - if (_owningObject.TryGetTarget(out _)) - { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner! - } - _owningObject.SetTarget(newOwner); - _pooledCount--; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); - - //3 // The following tests are retail assertions of things we can't allow to happen. - if (Pool != null) - { - if (0 != _pooledCount) - { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectInPoolMoreThanOnce); // popping object off stack with multiple pooledCount - } - } - else if (-1 != _pooledCount) - { - throw ADP.InternalError(ADP.InternalErrorCode.NonPooledObjectUsedMoreThanOnce); // popping object off stack with multiple pooledCount - } - } - - internal void RemoveWeakReference(object value) - { - DbReferenceCollection referenceCollection = ReferenceCollection; - if (referenceCollection != null) - { - referenceCollection.Remove(value); - } - } - - /// - /// When overridden in a derived class, will check if the underlying connection is still actually alive - /// - /// If true an exception will be thrown if the connection is dead instead of returning true\false - /// (this allows the caller to have the real reason that the connection is not alive (e.g. network error, etc)) - /// True if the connection is still alive, otherwise false (If not overridden, then always true) - internal virtual bool IsConnectionAlive(bool throwOnException = false) - { - return true; - } - } -} diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs deleted file mode 100644 index 6458d11f48..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ /dev/null @@ -1,512 +0,0 @@ -// 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.Diagnostics; -using System.Threading; -using System.Transactions; -using Microsoft.Data.Common; -using Microsoft.Data.SqlClient; - -namespace Microsoft.Data.ProviderBase -{ - internal abstract partial class DbConnectionInternal - { - private static int _objectTypeCount; - internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); - private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; - - private bool _isInStasis; - - private Transaction _enlistedTransaction; // [usage must be thread-safe] the transaction that we're enlisted in, either manually or automatically - - // _enlistedTransaction is a clone, so that transaction information can be queried even if the original transaction object is disposed. - // However, there are times when we need to know if the original transaction object was disposed, so we keep a reference to it here. - // This field should only be assigned a value at the same time _enlistedTransaction is updated. - // Also, this reference should not be disposed, since we aren't taking ownership of it. - private Transaction _enlistedTransactionOriginal; - - protected internal Transaction EnlistedTransaction - { - get - { - return _enlistedTransaction; - } - set - { - Transaction currentEnlistedTransaction = _enlistedTransaction; - if ((currentEnlistedTransaction == null && value != null) - || (currentEnlistedTransaction != null && !currentEnlistedTransaction.Equals(value))) - { // WebData 20000024 - - // Pay attention to the order here: - // 1) defect from any notifications - // 2) replace the transaction - // 3) re-enlist in notifications for the new transaction - - // SQLBUDT #230558 we need to use a clone of the transaction - // when we store it, or we'll end up keeping it past the - // duration of the using block of the TransactionScope - Transaction valueClone = null; - Transaction previousTransactionClone = null; - try - { - if (value != null) - { - valueClone = value.Clone(); - } - - // NOTE: rather than take locks around several potential round- - // trips to the server, and/or virtual function calls, we simply - // presume that you aren't doing something illegal from multiple - // threads, and check once we get around to finalizing things - // inside a lock. - - lock (this) - { - // NOTE: There is still a race condition here, when we are - // called from EnlistTransaction (which cannot re-enlist) - // instead of EnlistDistributedTransaction (which can), - // however this should have been handled by the outer - // connection which checks to ensure that it's OK. The - // only case where we have the race condition is multiple - // concurrent enlist requests to the same connection, which - // is a bit out of line with something we should have to - // support. - - // enlisted transaction can be nullified in Dispose call without lock - previousTransactionClone = Interlocked.Exchange(ref _enlistedTransaction, valueClone); - _enlistedTransactionOriginal = value; - value = valueClone; - valueClone = null; // we've stored it, don't dispose it. - } - } - finally - { - // we really need to dispose our clones; they may have - // native resources and GC may not happen soon enough. - // VSDevDiv 479564: don't dispose if still holding reference in _enlistedTransaction - if (previousTransactionClone != null && - !object.ReferenceEquals(previousTransactionClone, _enlistedTransaction)) - { - previousTransactionClone.Dispose(); - } - if (valueClone != null && !object.ReferenceEquals(valueClone, _enlistedTransaction)) - { - valueClone.Dispose(); - } - } - - // I don't believe that we need to lock to protect the actual - // enlistment in the transaction; it would only protect us - // against multiple concurrent calls to enlist, which really - // isn't supported anyway. - - if (value != null) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction {1}, Enlisting.", ObjectID, value.GetHashCode()); - TransactionOutcomeEnlist(value); - } - } - } - } - - /// - /// Get boolean value that indicates whether the enlisted transaction has been disposed. - /// - /// - /// True if there is an enlisted transaction, and it has been disposed. - /// False if there is an enlisted transaction that has not been disposed, or if the transaction reference is null. - /// - /// - /// This method must be called while holding a lock on the DbConnectionInternal instance. - /// - protected bool EnlistedTransactionDisposed - { - get - { - // Until the Transaction.Disposed property is public it is necessary to access a member - // that throws if the object is disposed to determine if in fact the transaction is disposed. - try - { - bool disposed; - - Transaction currentEnlistedTransactionOriginal = _enlistedTransactionOriginal; - if (currentEnlistedTransactionOriginal != null) - { - disposed = currentEnlistedTransactionOriginal.TransactionInformation == null; - } - else - { - // Don't expect to get here in the general case, - // Since this getter is called by CheckEnlistedTransactionBinding - // after checking for a non-null enlisted transaction (and it does so under lock). - disposed = false; - } - - return disposed; - } - catch (ObjectDisposedException) - { - return true; - } - } - } - - internal bool IsTxRootWaitingForTxEnd - { - get - { - return _isInStasis; - } - } - - internal int ObjectID - { - get - { - return _objectID; - } - } - - virtual protected bool UnbindOnTransactionCompletion - { - get - { - return true; - } - } - - // Is this a connection that must be put in stasis (or is already in stasis) pending the end of it's transaction? - virtual protected internal bool IsNonPoolableTransactionRoot - { - get - { - return false; // if you want to have delegated transactions that are non-poolable, you better override this... - } - } - - virtual internal bool IsTransactionRoot - { - get - { - return false; // if you want to have delegated transactions, you better override this... - } - } - - virtual protected bool ReadyToPrepareTransaction - { - get - { - return true; - } - } - - internal virtual bool IsAccessTokenExpired => false; - - abstract protected void Activate(Transaction transaction); - - internal void ActivateConnection(Transaction transaction) - { - // Internal method called from the connection pooler so we don't expose - // the Activate method publicly. - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Activating", ObjectID); - -#if DEBUG - int activateCount = Interlocked.Increment(ref _activateCount); - Debug.Assert(1 == activateCount, "activated multiple times?"); -#endif // DEBUG - - Activate(transaction); - SqlClientEventSource.Log.EnterActiveConnection(); - } - - internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) - { - // The implementation here is the implementation required for the - // "open" internal connections, since our own private "closed" - // singleton internal connection objects override this method to - // prevent anything funny from happening (like disposing themselves - // or putting them into a connection pool) - // - // Derived class should override DbConnectionInternal.Deactivate and DbConnectionInternal.Dispose - // for cleaning up after DbConnection.Close - // protected override void Deactivate() { // override DbConnectionInternal.Close - // // do derived class connection deactivation for both pooled & non-pooled connections - // } - // public override void Dispose() { // override DbConnectionInternal.Close - // // do derived class cleanup - // base.Dispose(); - // } - // - // overriding DbConnection.Close is also possible, but must provider for their own synchronization - // public override void Close() { // override DbConnection.Close - // base.Close(); - // // do derived class outer connection for both pooled & non-pooled connections - // // user must do their own synchronization here - // } - // - // if the DbConnectionInternal derived class needs to close the connection it should - // delegate to the DbConnection if one exists or directly call dispose - // DbConnection owningObject = (DbConnection)Owner; - // if (owningObject != null) { - // owningObject.Close(); // force the closed state on the outer object. - // } - // else { - // Dispose(); - // } - // - //////////////////////////////////////////////////////////////// - // DON'T MESS WITH THIS CODE UNLESS YOU KNOW WHAT YOU'RE DOING! - //////////////////////////////////////////////////////////////// - Debug.Assert(owningObject != null, "null owningObject"); - Debug.Assert(connectionFactory != null, "null connectionFactory"); - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0} Closing.", ObjectID); - - // if an exception occurs after the state change but before the try block - // the connection will be stuck in OpenBusy state. The commented out try-catch - // block doesn't really help because a ThreadAbort during the finally block - // would just revert the connection to a bad state. - // Open->Closed: guarantee internal connection is returned to correct pool - if (connectionFactory.SetInnerConnectionFrom(owningObject, DbConnectionOpenBusy.SingletonInstance, this)) - { - // Lock to prevent race condition with cancellation - lock (this) - { - bool lockToken = ObtainAdditionalLocksForClose(); - try - { - PrepareForCloseConnection(); - - DbConnectionPool connectionPool = Pool; - - // Detach from enlisted transactions that are no longer active on close - DetachCurrentTransactionIfEnded(); - - // The singleton closed classes won't have owners and - // connection pools, and we won't want to put them back - // into the pool. - if (connectionPool != null) - { - connectionPool.PutObject(this, owningObject); // PutObject calls Deactivate for us... - // NOTE: Before we leave the PutObject call, another - // thread may have already popped the connection from - // the pool, so don't expect to be able to verify it. - } - else - { - Deactivate(); // ensure we de-activate non-pooled connections, or the data readers and transactions may not get cleaned up... - SqlClientEventSource.Log.HardDisconnectRequest(); - - // To prevent an endless recursion, we need to clear - // the owning object before we call dispose so that - // we can't get here a second time... Ordinarily, I - // would call setting the owner to null a hack, but - // this is safe since we're about to dispose the - // object and it won't have an owner after that for - // certain. - _owningObject.SetTarget(null); - - if (IsTransactionRoot) - { - SetInStasis(); - } - else - { - SqlClientEventSource.Log.ExitNonPooledConnection(); - Dispose(); - } - } - } - finally - { - ReleaseAdditionalLocksForClose(lockToken); - // if a ThreadAbort puts us here then its possible the outer connection will not reference - // this and this will be orphaned, not reclaimed by object pool until outer connection goes out of scope. - connectionFactory.SetInnerConnectionEvent(owningObject, DbConnectionClosedPreviouslyOpened.SingletonInstance); - } - } - } - } - - virtual internal void DelegatedTransactionEnded() - { - // Called by System.Transactions when the delegated transaction has - // completed. We need to make closed connections that are in stasis - // available again, or disposed closed/leaked non-pooled connections. - - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction Completed.", ObjectID); - - if (1 == _pooledCount) - { - // When _pooledCount is 1, it indicates a closed, pooled, - // connection so it is ready to put back into the pool for - // general use. - - TerminateStasis(true); - - Deactivate(); // call it one more time just in case - - DbConnectionPool pool = Pool; - - if (pool == null) - { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectWithoutPool); // pooled connection does not have a pool - } - pool.PutObjectFromTransactedPool(this); - } - else if (-1 == _pooledCount && !_owningObject.TryGetTarget(out _)) - { - // When _pooledCount is -1 and the owning object no longer exists, - // it indicates a closed (or leaked), non-pooled connection so - // it is safe to dispose. - - TerminateStasis(false); - - Deactivate(); // call it one more time just in case - - // it's a non-pooled connection, we need to dispose of it - // once and for all, or the server will have fits about us - // leaving connections open until the client-side GC kicks - // in. - SqlClientEventSource.Log.ExitNonPooledConnection(); - Dispose(); - } - // When _pooledCount is 0, the connection is a pooled connection - // that is either open (if the owning object is alive) or leaked (if - // the owning object is not alive) In either case, we can't muck - // with the connection here. - } - - public virtual void Dispose() - { - _connectionPool = null; - _connectionIsDoomed = true; - _enlistedTransactionOriginal = null; // should not be disposed - - // Dispose of the _enlistedTransaction since it is a clone - // of the original reference. - // VSDD 780271 - _enlistedTransaction can be changed by another thread (TX end event) - Transaction enlistedTransaction = Interlocked.Exchange(ref _enlistedTransaction, null); - if (enlistedTransaction != null) - { - enlistedTransaction.Dispose(); - } - } - - abstract public void EnlistTransaction(Transaction transaction); - - // Cleanup connection's transaction-specific structures (currently used by Delegated transaction). - // This is a separate method because cleanup can be triggered in multiple ways for a delegated - // transaction. - virtual protected void CleanupTransactionOnCompletion(Transaction transaction) - { - } - - internal void DetachCurrentTransactionIfEnded() - { - Transaction enlistedTransaction = EnlistedTransaction; - if (enlistedTransaction != null) - { - bool transactionIsDead; - try - { - transactionIsDead = (TransactionStatus.Active != enlistedTransaction.TransactionInformation.Status); - } - catch (TransactionException) - { - // If the transaction is being processed (i.e. is part way through a rollback\commit\etc then TransactionInformation.Status will throw an exception) - transactionIsDead = true; - } - if (transactionIsDead) - { - DetachTransaction(enlistedTransaction, true); - } - } - } - - // Detach transaction from connection. - internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleasing) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount={1})", ObjectID, _pooledCount); - - // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new - // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should - // be the exception, not the rule. - // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. - lock (transaction) - { - // Detach if detach-on-end behavior, or if outer connection was closed - DbConnection owner = Owner; - if (isExplicitlyReleasing || UnbindOnTransactionCompletion || owner is null) - { - Transaction currentEnlistedTransaction = _enlistedTransaction; - if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) - { - // We need to remove the transaction completed event handler to cease listening for the transaction to end. - currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; - - EnlistedTransaction = null; - - if (IsTxRootWaitingForTxEnd) - { - DelegatedTransactionEnded(); - } - } - } - } - } - - // Handle transaction detach, pool cleanup and other post-transaction cleanup tasks associated with - internal void CleanupConnectionOnTransactionCompletion(Transaction transaction) - { - DetachTransaction(transaction, false); - - DbConnectionPool pool = Pool; - if (pool != null) - { - pool.TransactionEnded(transaction, this); - } - } - - void TransactionCompletedEvent(object sender, TransactionEventArgs e) - { - Transaction transaction = e.Transaction; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount = {1})", ObjectID, _pooledCount); - CleanupTransactionOnCompletion(transaction); - CleanupConnectionOnTransactionCompletion(transaction); - } - - private void TransactionOutcomeEnlist(Transaction transaction) - { - _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); - transaction.TransactionCompleted += _transactionCompletedEventHandler; - } - - internal void SetInStasis() - { - _isInStasis = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); - SqlClientEventSource.Log.EnterStasisConnection(); - } - - private void TerminateStasis(bool returningToPool) - { - if (returningToPool) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed. Returning to general pool.", ObjectID); - } - else - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed/leaked. Disposing.", ObjectID); - } - SqlClientEventSource.Log.ExitStasisConnection(); - _isInStasis = false; - } - } -} From 45965cb3b4fff128a68ea5943e811c80315292c6 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 16:26:10 -0500 Subject: [PATCH 09/19] Change the project files to point to the merged file (this completes the basic merge process) --- .../netcore/src/Microsoft.Data.SqlClient.csproj | 8 ++++++-- .../netfx/src/Microsoft.Data.SqlClient.csproj | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index f956aa8594..b2f4b12c20 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -77,6 +77,9 @@ Microsoft\Data\ProviderBase\DbConnectionPool.cs + + Microsoft\Data\ProviderBase\DbConnectionInternal.cs + Microsoft\Data\ProviderBase\DbConnectionPoolAuthenticationContext.cs @@ -614,7 +617,7 @@ Microsoft\Data\SqlDbTypeExtensions.cs - + @@ -630,7 +633,8 @@ - + + diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj index 03eb18b686..b6130f3c28 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -139,6 +139,9 @@ Microsoft\Data\ProviderBase\DbConnectionFactory.cs + + Microsoft\Data\ProviderBase\DbConnectionInternal.cs + Microsoft\Data\ProviderBase\DbConnectionPool.cs @@ -148,6 +151,9 @@ Microsoft\Data\ProviderBase\DbConnectionPoolAuthenticationContextKey.cs + + Microsoft\Data\ProviderBase\DbConnectionPoolCounters.netfx.cs + Microsoft\Data\ProviderBase\DbConnectionPoolGroup.cs @@ -699,8 +705,6 @@ - - From 7013f90b94dc1dfac037b608f8cb50e64377c913 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 10 Oct 2024 17:15:51 -0500 Subject: [PATCH 10/19] Patching up after rebase --- .../src/Microsoft.Data.SqlClient.csproj | 3 -- .../ProviderBase/DbConnectionPool.stub.cs | 31 ------------------- 2 files changed, 34 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index b2f4b12c20..80d3596754 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -618,7 +618,6 @@ Microsoft\Data\SqlDbTypeExtensions.cs - @@ -633,8 +632,6 @@ - - diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs deleted file mode 100644 index a548d3d4d8..0000000000 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.stub.cs +++ /dev/null @@ -1,31 +0,0 @@ -// 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.Transactions; - -namespace Microsoft.Data.ProviderBase -{ - // DO NOT USE THIS FILE IN ANY PROJECT! - // This is a temporary stub to enable migrating DbConnectionInternal to the common project. - internal class DbConnectionPool - { - internal TimeSpan LoadBalanceTimeout => throw new NotImplementedException("STUB"); - - #if NETFRAMEWORK - internal DbConnectionPoolCounters PerformanceCounters => throw new NotImplementedException("STUB"); - #endif - - internal bool UseLoadBalancing => throw new NotImplementedException("STUB"); - - internal void PutObject(DbConnectionInternal obj, object owningObject) => - throw new NotImplementedException("STUB"); - - internal void PutObjectFromTransactedPool(DbConnectionInternal obj) => - throw new NotImplementedException("STUB"); - - internal void TransactionEnded(Transaction transaction, DbConnectionInternal transactedObject) => - throw new NotImplementedException("STUB"); - } -} From d538728960f710dfa915867e42cb6398a05f0849 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Oct 2024 18:26:03 -0500 Subject: [PATCH 11/19] Move and group members to be in order as per style --- .../Data/ProviderBase/DbConnectionInternal.cs | 724 +++++++++--------- 1 file changed, 369 insertions(+), 355 deletions(-) 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 13d191555c..72731e15f2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -21,36 +21,33 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionInternal { - private static int _objectTypeCount; - internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); - private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; + #region Fields internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); + private static int _objectTypeCount; + private readonly bool _allowSetConnectionString; private readonly bool _hidePassword; - private readonly ConnectionState _state; - + private readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) + private readonly ConnectionState _state; - private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only) - private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated - private int _pooledCount; // [usage must be thread safe] the number of times this object has been pushed into the pool less the number of times it's been popped (0 != inPool) - - private bool _connectionIsDoomed; // true when the connection should no longer be used. private bool _cannotBePooled; // true when the connection should no longer be pooled. - private bool _isInStasis; - + private bool _connectionIsDoomed; // true when the connection should no longer be used. + private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only) private DateTime _createTime; // when the connection was created. - private Transaction _enlistedTransaction; // [usage must be thread-safe] the transaction that we're enlisted in, either manually or automatically - // _enlistedTransaction is a clone, so that transaction information can be queried even if the original transaction object is disposed. // However, there are times when we need to know if the original transaction object was disposed, so we keep a reference to it here. // This field should only be assigned a value at the same time _enlistedTransaction is updated. // Also, this reference should not be disposed, since we aren't taking ownership of it. private Transaction _enlistedTransactionOriginal; + private bool _isInStasis; + private int _pooledCount; // [usage must be thread safe] the number of times this object has been pushed into the pool less the number of times it's been popped (0 != inPool) + private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated + private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; #if NETFRAMEWORK private DbConnectionPoolCounters _performanceCounters; // the performance counters we're supposed to update @@ -60,6 +57,8 @@ internal abstract class DbConnectionInternal private int _activateCount; // debug only counter to verify activate/deactivates are in sync. #endif + #endregion + protected DbConnectionInternal() : this(ConnectionState.Open, true, false) { } @@ -72,6 +71,8 @@ internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool all _state = state; } + #region Properties + internal bool AllowSetConnectionString { get @@ -88,6 +89,115 @@ internal bool CanBePooled } } + internal virtual bool IsAccessTokenExpired => false; + + internal bool IsEmancipated + { + get + { + // NOTE: There are race conditions between PrePush, PostPop and this + // property getter -- only use this while this object is locked; + // (DbConnectionPool.Clear and ReclaimEmancipatedObjects + // do this for us) + + // The functionality is as follows: + // + // _pooledCount is incremented when the connection is pushed into the pool + // _pooledCount is decremented when the connection is popped from the pool + // _pooledCount is set to -1 when the connection is not pooled (just in case...) + // + // That means that: + // + // _pooledCount > 1 connection is in the pool multiple times (This should not happen) + // _pooledCount == 1 connection is in the pool + // _pooledCount == 0 connection is out of the pool + // _pooledCount == -1 connection is not a pooled connection; we shouldn't be here for non-pooled connections. + // _pooledCount < -1 connection out of the pool multiple times + // + // Now, our job is to return TRUE when the connection is out + // of the pool and it's owning object is no longer around to + // return it. + + #if NETFRAMEWORK + return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + #else + return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); + #endif + } + } + + internal bool IsInPool + { + get + { + Debug.Assert(_pooledCount <= 1 && _pooledCount >= -1, "Pooled count for object is invalid"); + return (_pooledCount == 1); + } + } + + virtual internal bool IsTransactionRoot + { + get + { + return false; // if you want to have delegated transactions, you better override this... + } + } + + // Is this connection in stasis, waiting for transaction to end before returning to pool? + internal bool IsTxRootWaitingForTxEnd + { + get + { + return _isInStasis; + } + } + + internal int ObjectID + { + get + { + return _objectID; + } + } + + internal DbConnectionPool Pool + { + get + { + return _connectionPool; + } + } + + abstract public string ServerVersion + { + get; + } + + // this should be abstract but until it is added to all the providers virtual will have to do RickFe + virtual public string ServerVersionNormalized + { + get + { + throw ADP.NotSupported(); + } + } + + public bool ShouldHidePassword + { + get + { + return _hidePassword; + } + } + + public ConnectionState State + { + get + { + return _state; + } + } + protected internal Transaction EnlistedTransaction { get @@ -215,27 +325,11 @@ protected bool EnlistedTransactionDisposed } } - // Is this connection in stasis, waiting for transaction to end before returning to pool? - internal bool IsTxRootWaitingForTxEnd - { - get - { - return _isInStasis; - } - } - - /// - /// Get boolean that specifies whether an enlisted transaction can be unbound from - /// the connection when that transaction completes. - /// - /// - /// True if the enlisted transaction can be unbound on transaction completion; otherwise false. - /// - virtual protected bool UnbindOnTransactionCompletion + protected internal bool IsConnectionDoomed { get { - return true; + return _connectionIsDoomed; } } @@ -248,74 +342,6 @@ virtual protected internal bool IsNonPoolableTransactionRoot } } - virtual internal bool IsTransactionRoot - { - get - { - return false; // if you want to have delegated transactions, you better override this... - } - } - - protected internal bool IsConnectionDoomed - { - get - { - return _connectionIsDoomed; - } - } - - internal bool IsEmancipated - { - get - { - // NOTE: There are race conditions between PrePush, PostPop and this - // property getter -- only use this while this object is locked; - // (DbConnectionPool.Clear and ReclaimEmancipatedObjects - // do this for us) - - // The functionality is as follows: - // - // _pooledCount is incremented when the connection is pushed into the pool - // _pooledCount is decremented when the connection is popped from the pool - // _pooledCount is set to -1 when the connection is not pooled (just in case...) - // - // That means that: - // - // _pooledCount > 1 connection is in the pool multiple times (This should not happen) - // _pooledCount == 1 connection is in the pool - // _pooledCount == 0 connection is out of the pool - // _pooledCount == -1 connection is not a pooled connection; we shouldn't be here for non-pooled connections. - // _pooledCount < -1 connection out of the pool multiple times - // - // Now, our job is to return TRUE when the connection is out - // of the pool and it's owning object is no longer around to - // return it. - - #if NETFRAMEWORK - return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - #else - return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - #endif - } - } - - internal bool IsInPool - { - get - { - Debug.Assert(_pooledCount <= 1 && _pooledCount >= -1, "Pooled count for object is invalid"); - return (_pooledCount == 1); - } - } - - internal int ObjectID - { - get - { - return _objectID; - } - } - protected internal DbConnection Owner { // We use a weak reference to the owning object so we can identify when @@ -330,14 +356,6 @@ protected internal DbConnection Owner } } - internal DbConnectionPool Pool - { - get - { - return _connectionPool; - } - } - #if NETFRAMEWORK protected DbConnectionPoolCounters PerformanceCounters { @@ -364,41 +382,28 @@ protected internal DbReferenceCollection ReferenceCollection } } - abstract public string ServerVersion - { - get; - } - - // this should be abstract but until it is added to all the providers virtual will have to do RickFe - virtual public string ServerVersionNormalized + /// + /// Get boolean that specifies whether an enlisted transaction can be unbound from + /// the connection when that transaction completes. + /// + /// + /// True if the enlisted transaction can be unbound on transaction completion; otherwise false. + /// + virtual protected bool UnbindOnTransactionCompletion { get { - throw ADP.NotSupported(); + return true; } } - public bool ShouldHidePassword - { - get - { - return _hidePassword; - } - } + #endregion - public ConnectionState State - { - get - { - return _state; - } - } - - internal virtual bool IsAccessTokenExpired => false; - - abstract protected void Activate(Transaction transaction); - - internal void ActivateConnection(Transaction transaction) + #region Public/Internal Methods + + abstract protected void Activate(Transaction transaction); + + internal void ActivateConnection(Transaction transaction) { // Internal method called from the connection pooler so we don't expose // the Activate method publicly. @@ -437,6 +442,18 @@ virtual public void ChangeDatabase(string value) throw ADP.MethodNotImplemented(); } + // Handle transaction detach, pool cleanup and other post-transaction cleanup tasks associated with + internal void CleanupConnectionOnTransactionCompletion(Transaction transaction) + { + DetachTransaction(transaction, false); + + DbConnectionPool pool = Pool; + if (pool != null) + { + pool.TransactionEnded(transaction, this); + } + } + internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) { // The implementation here is the implementation required for the @@ -559,33 +576,6 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac } } - virtual internal void PrepareForReplaceConnection() - { - // By default, there is no preparation required - } - - virtual protected void PrepareForCloseConnection() - { - // By default, there is no preparation required - } - - virtual protected bool ObtainAdditionalLocksForClose() - { - return false; // no additional locks in default implementation - } - - virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) - { - // no additional locks in default implementation - } - - virtual protected DbReferenceCollection CreateReferenceCollection() - { - throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject); - } - - abstract protected void Deactivate(); - internal void DeactivateConnection() { // Internal method called from the connection pooler so we don't expose @@ -600,7 +590,7 @@ internal void DeactivateConnection() #if NETFRAMEWORK if (PerformanceCounters != null) - { // Pool.Clear will DestroyObject that will clean performanceCounters before going here + { // Pool.Clear will DestroyObject that will clean performanceCounters before going here PerformanceCounters.NumberOfActiveConnections.Decrement(); } #else @@ -653,7 +643,7 @@ virtual internal void DelegatedTransactionEnded() else if (-1 == _pooledCount && !_owningObject.TryGetTarget(out _)) { // When _pooledCount is -1 and the owning object no longer exists, - // it indicates a closed (or leaked), non-pooled connection so + // it indicates a closed (or leaked), non-pooled connection so // it is safe to dispose. TerminateStasis(false); @@ -662,7 +652,7 @@ virtual internal void DelegatedTransactionEnded() // it's a non-pooled connection, we need to dispose of it // once and for all, or the server will have fits about us - // leaving connections open until the client-side GC kicks + // leaving connections open until the client-side GC kicks // in. #if NETFRAMEWORK PerformanceCounters.NumberOfNonPooledConnections.Decrement(); @@ -674,10 +664,64 @@ virtual internal void DelegatedTransactionEnded() } // When _pooledCount is 0, the connection is a pooled connection // that is either open (if the owning object is alive) or leaked (if - // the owning object is not alive) In either case, we can't muck + // the owning object is not alive) In either case, we can't muck // with the connection here. } + internal void DetachCurrentTransactionIfEnded() + { + Transaction enlistedTransaction = EnlistedTransaction; + if (enlistedTransaction != null) + { + bool transactionIsDead; + try + { + transactionIsDead = (TransactionStatus.Active != enlistedTransaction.TransactionInformation.Status); + } + catch (TransactionException) + { + // If the transaction is being processed (i.e. is part way through a rollback\commit\etc then TransactionInformation.Status will throw an exception) + transactionIsDead = true; + } + if (transactionIsDead) + { + DetachTransaction(enlistedTransaction, true); + } + } + } + + // Detach transaction from connection. + internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleasing) + { + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount={1})", ObjectID, _pooledCount); + + // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new + // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should + // be the exception, not the rule. + // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. + lock (transaction) + { + // Detach if detach-on-end behavior, or if outer connection was closed + DbConnection owner = Owner; + if (isExplicitlyReleasing || UnbindOnTransactionCompletion || owner is null) + { + Transaction currentEnlistedTransaction = _enlistedTransaction; + if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) + { + // We need to remove the transaction completed event handler to cease listening for the transaction to end. + currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; + + EnlistedTransaction = null; + + if (IsTxRootWaitingForTxEnd) + { + DelegatedTransactionEnded(); + } + } + } + } + } + public virtual void Dispose() { _connectionPool = null; @@ -698,38 +742,17 @@ public virtual void Dispose() } } - protected internal void DoNotPoolThisConnection() - { - _cannotBePooled = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Marking pooled object as non-poolable so it will be disposed", ObjectID); - } - - /// Ensure that this connection cannot be put back into the pool. - #if NETFRAMEWORK - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] - #endif - protected internal void DoomThisConnection() - { - _connectionIsDoomed = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Dooming", ObjectID); - } - - // Reset connection doomed status so it can be re-connected and pooled. - protected internal void UnDoomThisConnection() - { - _connectionIsDoomed = false; - } - abstract public void EnlistTransaction(Transaction transaction); - protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + /// + /// When overridden in a derived class, will check if the underlying connection is still actually alive + /// + /// If true an exception will be thrown if the connection is dead instead of returning true\false + /// (this allows the caller to have the real reason that the connection is not alive (e.g. network error, etc)) + /// True if the connection is still alive, otherwise false (If not overridden, then always true) + internal virtual bool IsConnectionAlive(bool throwOnException = false) { - Debug.Assert(outerConnection != null, "outerConnection may not be null."); - - DbMetaDataFactory metaDataFactory = factory.GetMetaDataFactory(poolGroup, this); - Debug.Assert(metaDataFactory != null, "metaDataFactory may not be null."); - - return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); + return true; } #if NETFRAMEWORK @@ -780,50 +803,49 @@ internal virtual void OpenConnection(DbConnection outerConnection, DbConnectionF } } - /// The default implementation is for the open connection objects, and - /// it simply throws. Our private closed-state connection objects - /// override this and do the correct thing. - // User code should either override DbConnectionInternal.Activate when it comes out of the pool - // or override DbConnectionFactory.CreateConnection when the connection is created for non-pooled connections - internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + internal void PostPop(DbConnection newOwner) { - throw ADP.ConnectionAlreadyOpen(State); - } + // Called by DbConnectionPool right after it pulls this from it's pool, we + // take this opportunity to ensure ownership and pool counts are legit. - internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - throw ADP.MethodNotImplemented(); - } + Debug.Assert(!IsEmancipated, "pooled object not in pool"); - protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - // ?->Connecting: prevent set_ConnectionString during Open - if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) + // SQLBUDT #356871 -- When another thread is clearing this pool, it + // will doom all connections in this pool without prejudice which + // causes the following assert to fire, which really mucks up stress + // against checked bits. The assert is benign, so we're commenting + // it out. + //Debug.Assert(CanBePooled, "pooled object is not poolable"); + + // IMPORTANT NOTE: You must have taken a lock on the object before + // you call this method to prevent race conditions with Clear and + // ReclaimEmancipatedObjects. + if (_owningObject.TryGetTarget(out _)) { - DbConnectionInternal openConnection = null; - try - { - connectionFactory.PermissionDemand(outerConnection); - if (!connectionFactory.TryGetConnection(outerConnection, retry, userOptions, this, out openConnection)) - { - return false; - } - } - catch - { - // This should occur for all exceptions, even ADP.UnCatchableExceptions. - connectionFactory.SetInnerConnectionTo(outerConnection, this); - throw; - } - if (openConnection == null) + throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner! + } + _owningObject.SetTarget(newOwner); + _pooledCount--; + + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); + + //3 // The following tests are retail assertions of things we can't allow to happen. + if (Pool != null) + { + if (0 != _pooledCount) { - connectionFactory.SetInnerConnectionTo(outerConnection, this); - throw ADP.InternalConnectionError(ADP.ConnectionError.GetConnectionReturnsNull); + throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectInPoolMoreThanOnce); // popping object off stack with multiple pooledCount } - connectionFactory.SetInnerConnectionEvent(outerConnection, openConnection); } + else if (-1 != _pooledCount) + { + throw ADP.InternalError(ADP.InternalErrorCode.NonPooledObjectUsedMoreThanOnce); // popping object off stack with multiple pooledCount + } + } - return true; + virtual internal void PrepareForReplaceConnection() + { + // By default, there is no preparation required } internal void PrePush(object expectedOwner) @@ -859,55 +881,46 @@ internal void PrePush(object expectedOwner) _owningObject.SetTarget(null); // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2% } - internal void PostPop(DbConnection newOwner) + internal void RemoveWeakReference(object value) { - // Called by DbConnectionPool right after it pulls this from it's pool, we - // take this opportunity to ensure ownership and pool counts are legit. - - Debug.Assert(!IsEmancipated, "pooled object not in pool"); - - // SQLBUDT #356871 -- When another thread is clearing this pool, it - // will doom all connections in this pool without prejudice which - // causes the following assert to fire, which really mucks up stress - // against checked bits. The assert is benign, so we're commenting - // it out. - //Debug.Assert(CanBePooled, "pooled object is not poolable"); - - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. - if (_owningObject.TryGetTarget(out _)) + DbReferenceCollection referenceCollection = ReferenceCollection; + if (referenceCollection != null) { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner! + referenceCollection.Remove(value); } - _owningObject.SetTarget(newOwner); - _pooledCount--; + } - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); + internal void SetInStasis() + { + _isInStasis = true; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); - //3 // The following tests are retail assertions of things we can't allow to happen. - if (Pool != null) - { - if (0 != _pooledCount) - { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectInPoolMoreThanOnce); // popping object off stack with multiple pooledCount - } - } - else if (-1 != _pooledCount) - { - throw ADP.InternalError(ADP.InternalErrorCode.NonPooledObjectUsedMoreThanOnce); // popping object off stack with multiple pooledCount - } + #if NETFRAMEWORK + PerformanceCounters.NumberOfStasisConnections.Increment(); + #else + SqlClientEventSource.Log.EnterStasisConnection(); + #endif } - internal void RemoveWeakReference(object value) + /// The default implementation is for the open connection objects, and + /// it simply throws. Our private closed-state connection objects + /// override this and do the correct thing. + // User code should either override DbConnectionInternal.Activate when it comes out of the pool + // or override DbConnectionFactory.CreateConnection when the connection is created for non-pooled connections + internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { - DbReferenceCollection referenceCollection = ReferenceCollection; - if (referenceCollection != null) - { - referenceCollection.Remove(value); - } + throw ADP.ConnectionAlreadyOpen(State); } + internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + { + throw ADP.MethodNotImplemented(); + } + + #endregion + + #region Protected Methods + // Cleanup connection's transaction-specific structures (currently used by Delegated transaction). // This is a separate method because cleanup can be triggered in multiple ways for a delegated // transaction. @@ -915,104 +928,95 @@ virtual protected void CleanupTransactionOnCompletion(Transaction transaction) { } - internal void DetachCurrentTransactionIfEnded() + virtual protected DbReferenceCollection CreateReferenceCollection() { - Transaction enlistedTransaction = EnlistedTransaction; - if (enlistedTransaction != null) - { - bool transactionIsDead; - try - { - transactionIsDead = (TransactionStatus.Active != enlistedTransaction.TransactionInformation.Status); - } - catch (TransactionException) - { - // If the transaction is being processed (i.e. is part way through a rollback\commit\etc then TransactionInformation.Status will throw an exception) - transactionIsDead = true; - } - if (transactionIsDead) - { - DetachTransaction(enlistedTransaction, true); - } - } + throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject); } - // Detach transaction from connection. - internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleasing) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount={1})", ObjectID, _pooledCount); - - // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new - // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should - // be the exception, not the rule. - // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. - lock (transaction) - { - // Detach if detach-on-end behavior, or if outer connection was closed - DbConnection owner = Owner; - if (isExplicitlyReleasing || UnbindOnTransactionCompletion || owner is null) - { - Transaction currentEnlistedTransaction = _enlistedTransaction; - if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) - { - // We need to remove the transaction completed event handler to cease listening for the transaction to end. - currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; + abstract protected void Deactivate(); - EnlistedTransaction = null; + protected internal void DoNotPoolThisConnection() + { + _cannotBePooled = true; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Marking pooled object as non-poolable so it will be disposed", ObjectID); + } - if (IsTxRootWaitingForTxEnd) - { - DelegatedTransactionEnded(); - } - } - } - } + /// Ensure that this connection cannot be put back into the pool. + #if NETFRAMEWORK + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + #endif + protected internal void DoomThisConnection() + { + _connectionIsDoomed = true; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Dooming", ObjectID); } - // Handle transaction detach, pool cleanup and other post-transaction cleanup tasks associated with - internal void CleanupConnectionOnTransactionCompletion(Transaction transaction) + protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) { - DetachTransaction(transaction, false); + Debug.Assert(outerConnection != null, "outerConnection may not be null."); - DbConnectionPool pool = Pool; - if (pool != null) - { - pool.TransactionEnded(transaction, this); - } + DbMetaDataFactory metaDataFactory = factory.GetMetaDataFactory(poolGroup, this); + Debug.Assert(metaDataFactory != null, "metaDataFactory may not be null."); + + return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); } - void TransactionCompletedEvent(object sender, TransactionEventArgs e) + virtual protected bool ObtainAdditionalLocksForClose() { - Transaction transaction = e.Transaction; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount = {1})", ObjectID, _pooledCount); - - CleanupTransactionOnCompletion(transaction); - CleanupConnectionOnTransactionCompletion(transaction); + return false; // no additional locks in default implementation } + virtual protected void PrepareForCloseConnection() + { + // By default, there is no preparation required + } - #if NETFRAMEWORK - // TODO: Review whether we need the unmanaged code permission when we have the new object model available. - [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] - #endif - private void TransactionOutcomeEnlist(Transaction transaction) + virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) { - _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); - transaction.TransactionCompleted += _transactionCompletedEventHandler; + // no additional locks in default implementation } - internal void SetInStasis() + protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { - _isInStasis = true; - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); + // ?->Connecting: prevent set_ConnectionString during Open + if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) + { + DbConnectionInternal openConnection = null; + try + { + connectionFactory.PermissionDemand(outerConnection); + if (!connectionFactory.TryGetConnection(outerConnection, retry, userOptions, this, out openConnection)) + { + return false; + } + } + catch + { + // This should occur for all exceptions, even ADP.UnCatchableExceptions. + connectionFactory.SetInnerConnectionTo(outerConnection, this); + throw; + } + if (openConnection == null) + { + connectionFactory.SetInnerConnectionTo(outerConnection, this); + throw ADP.InternalConnectionError(ADP.ConnectionError.GetConnectionReturnsNull); + } + connectionFactory.SetInnerConnectionEvent(outerConnection, openConnection); + } - #if NETFRAMEWORK - PerformanceCounters.NumberOfStasisConnections.Increment(); - #else - SqlClientEventSource.Log.EnterStasisConnection(); - #endif + return true; + } + + // Reset connection doomed status so it can be re-connected and pooled. + protected internal void UnDoomThisConnection() + { + _connectionIsDoomed = false; } + #endregion + + #region Private Methods + private void TerminateStasis(bool returningToPool) { if (returningToPool) @@ -1033,15 +1037,25 @@ private void TerminateStasis(bool returningToPool) _isInStasis = false; } - /// - /// When overridden in a derived class, will check if the underlying connection is still actually alive - /// - /// If true an exception will be thrown if the connection is dead instead of returning true\false - /// (this allows the caller to have the real reason that the connection is not alive (e.g. network error, etc)) - /// True if the connection is still alive, otherwise false (If not overridden, then always true) - internal virtual bool IsConnectionAlive(bool throwOnException = false) + void TransactionCompletedEvent(object sender, TransactionEventArgs e) { - return true; + Transaction transaction = e.Transaction; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount = {1})", ObjectID, _pooledCount); + + CleanupTransactionOnCompletion(transaction); + CleanupConnectionOnTransactionCompletion(transaction); } + + #if NETFRAMEWORK + // TODO: Review whether we need the unmanaged code permission when we have the new object model available. + [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] + #endif + private void TransactionOutcomeEnlist(Transaction transaction) + { + _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); + transaction.TransactionCompleted += _transactionCompletedEventHandler; + } + + #endregion } } From 4c1d7161b8b8c6de590222e1d797107f5fcfbd3c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 9 Oct 2024 17:43:38 -0500 Subject: [PATCH 12/19] Cleanup member variables --- .../Data/ProviderBase/DbConnectionInternal.cs | 73 +++++++++++++++---- 1 file changed, 57 insertions(+), 16 deletions(-) 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 72731e15f2..ff6c7f481f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -23,30 +23,71 @@ internal abstract class DbConnectionInternal { #region Fields - internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); - internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); + internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs( + ConnectionState.Open, + ConnectionState.Closed); + internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs( + ConnectionState.Closed, + ConnectionState.Open); private static int _objectTypeCount; private readonly bool _allowSetConnectionString; private readonly bool _hidePassword; - private readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); - private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) + private readonly int _objectId = Interlocked.Increment(ref _objectTypeCount); + + /// + /// [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) + /// + private readonly WeakReference _owningObject = new WeakReference(null, false); private readonly ConnectionState _state; - private bool _cannotBePooled; // true when the connection should no longer be pooled. - private bool _connectionIsDoomed; // true when the connection should no longer be used. - private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only) - private DateTime _createTime; // when the connection was created. - private Transaction _enlistedTransaction; // [usage must be thread-safe] the transaction that we're enlisted in, either manually or automatically - // _enlistedTransaction is a clone, so that transaction information can be queried even if the original transaction object is disposed. - // However, there are times when we need to know if the original transaction object was disposed, so we keep a reference to it here. - // This field should only be assigned a value at the same time _enlistedTransaction is updated. - // Also, this reference should not be disposed, since we aren't taking ownership of it. + /// + /// True when the connection should no longer be pooled. + /// + private bool _cannotBePooled; + + /// + /// True when the connection should no longer be used. + /// + private bool _connectionIsDoomed; + + /// + /// The pooler that the connection came from (Pooled connections only) + /// + private DbConnectionPool _connectionPool; + + /// + /// When the connection was created. + /// + private DateTime _createTime; + + /// + /// [usage must be thread-safe] the transaction that we're enlisted in, either manually or automatically. + /// + private Transaction _enlistedTransaction; + + /// + /// is a clone, so that transaction information can be + /// queried even if the original transaction object is disposed. However, there are times + /// when we need to know if the original transaction object was disposed, so we keep a + /// reference to it here. This field should only be assigned a value at the same time + /// is updated. + /// Also, this reference should not be disposed, since we aren't taking ownership of it. + /// private Transaction _enlistedTransactionOriginal; private bool _isInStasis; - private int _pooledCount; // [usage must be thread safe] the number of times this object has been pushed into the pool less the number of times it's been popped (0 != inPool) - private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated + + /// + /// usage must be thread safe] the number of times this object has been pushed into the + /// pool less the number of times it's been popped (0 != inPool) + /// + private int _pooledCount; + + /// + /// Collection of objects that we need to notify in some way when we're being deactivated + /// + private DbReferenceCollection _referenceCollection; private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; #if NETFRAMEWORK @@ -156,7 +197,7 @@ internal int ObjectID { get { - return _objectID; + return _objectId; } } From e8f778416cdb017d119b302317ded3f091caca9a Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 9 Oct 2024 17:59:10 -0500 Subject: [PATCH 13/19] Cleaning up the properties --- .../Data/ProviderBase/DbConnectionInternal.cs | 245 ++++++------------ 1 file changed, 80 insertions(+), 165 deletions(-) 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 ff6c7f481f..a1d6121375 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -32,31 +32,18 @@ internal abstract class DbConnectionInternal private static int _objectTypeCount; - private readonly bool _allowSetConnectionString; - private readonly bool _hidePassword; private readonly int _objectId = Interlocked.Increment(ref _objectTypeCount); /// /// [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections) /// private readonly WeakReference _owningObject = new WeakReference(null, false); - private readonly ConnectionState _state; /// /// True when the connection should no longer be pooled. /// private bool _cannotBePooled; - /// - /// True when the connection should no longer be used. - /// - private bool _connectionIsDoomed; - - /// - /// The pooler that the connection came from (Pooled connections only) - /// - private DbConnectionPool _connectionPool; - /// /// When the connection was created. /// @@ -76,7 +63,6 @@ internal abstract class DbConnectionInternal /// Also, this reference should not be disposed, since we aren't taking ownership of it. /// private Transaction _enlistedTransactionOriginal; - private bool _isInStasis; /// /// usage must be thread safe] the number of times this object has been pushed into the @@ -84,18 +70,13 @@ internal abstract class DbConnectionInternal /// private int _pooledCount; - /// - /// Collection of objects that we need to notify in some way when we're being deactivated - /// - private DbReferenceCollection _referenceCollection; private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; - #if NETFRAMEWORK - private DbConnectionPoolCounters _performanceCounters; // the performance counters we're supposed to update - #endif - #if DEBUG - private int _activateCount; // debug only counter to verify activate/deactivates are in sync. + /// + /// Debug only counter to verify activate/deactivates are in sync. + /// + private int _activateCount; #endif #endregion @@ -107,28 +88,16 @@ protected DbConnectionInternal() : this(ConnectionState.Open, true, false) // Constructor for internal connections internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool allowSetConnectionString) { - _allowSetConnectionString = allowSetConnectionString; - _hidePassword = hidePassword; - _state = state; + AllowSetConnectionString = allowSetConnectionString; + ShouldHidePassword = hidePassword; + State = state; } #region Properties - internal bool AllowSetConnectionString - { - get - { - return _allowSetConnectionString; - } - } + internal bool AllowSetConnectionString { get; } - internal bool CanBePooled - { - get - { - return (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out _)); - } - } + internal bool CanBePooled => !IsConnectionDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out _); internal virtual bool IsAccessTokenExpired => false; @@ -172,72 +141,38 @@ internal bool IsInPool get { Debug.Assert(_pooledCount <= 1 && _pooledCount >= -1, "Pooled count for object is invalid"); - return (_pooledCount == 1); + return _pooledCount == 1; } } - virtual internal bool IsTransactionRoot - { - get - { - return false; // if you want to have delegated transactions, you better override this... - } - } + /// + /// If you want to have delegated transactions, you had better override this... + /// + internal virtual bool IsTransactionRoot => false; - // Is this connection in stasis, waiting for transaction to end before returning to pool? - internal bool IsTxRootWaitingForTxEnd - { - get - { - return _isInStasis; - } - } + /// + /// Is this connection in stasis, waiting for transaction to end before returning to pool? + /// + internal bool IsTxRootWaitingForTxEnd { get; private set; } - internal int ObjectID - { - get - { - return _objectId; - } - } + internal int ObjectID => _objectId; - internal DbConnectionPool Pool - { - get - { - return _connectionPool; - } - } + /// + /// The pooler that the connection came from (Pooled connections only) + /// + internal DbConnectionPool Pool { get; private set; } - abstract public string ServerVersion - { - get; - } + public abstract string ServerVersion { get; } // this should be abstract but until it is added to all the providers virtual will have to do RickFe - virtual public string ServerVersionNormalized + public virtual string ServerVersionNormalized { - get - { - throw ADP.NotSupported(); - } + get => throw ADP.NotSupported(); } - public bool ShouldHidePassword - { - get - { - return _hidePassword; - } - } + public bool ShouldHidePassword { get; } - public ConnectionState State - { - get - { - return _state; - } - } + public ConnectionState State { get; } protected internal Transaction EnlistedTransaction { @@ -248,10 +183,9 @@ protected internal Transaction EnlistedTransaction set { Transaction currentEnlistedTransaction = _enlistedTransaction; - if ((currentEnlistedTransaction == null && value != null) - || (currentEnlistedTransaction != null && !currentEnlistedTransaction.Equals(value))) - { // WebData 20000024 - + if ((currentEnlistedTransaction == null && value != null) || + (currentEnlistedTransaction != null && !currentEnlistedTransaction.Equals(value))) + { // Pay attention to the order here: // 1) defect from any notifications // 2) replace the transaction @@ -299,12 +233,11 @@ protected internal Transaction EnlistedTransaction // we really need to dispose our clones; they may have // native resources and GC may not happen soon enough. // VSDevDiv 479564: don't dispose if still holding reference in _enlistedTransaction - if (previousTransactionClone != null && - !Object.ReferenceEquals(previousTransactionClone, _enlistedTransaction)) + if (previousTransactionClone != null && !ReferenceEquals(previousTransactionClone, _enlistedTransaction)) { previousTransactionClone.Dispose(); } - if (valueClone != null && !Object.ReferenceEquals(valueClone, _enlistedTransaction)) + if (valueClone != null && !ReferenceEquals(valueClone, _enlistedTransaction)) { valueClone.Dispose(); } @@ -328,8 +261,9 @@ protected internal Transaction EnlistedTransaction /// Get boolean value that indicates whether the enlisted transaction has been disposed. /// /// - /// True if there is an enlisted transaction, and it has been disposed. - /// False if there is an enlisted transaction that has not been disposed, or if the transaction reference is null. + /// if there is an enlisted transaction, and it has been disposed. + /// if there is an enlisted transaction that has not been disposed, + /// or if the transaction reference is null. /// /// /// This method must be called while holding a lock on the DbConnectionInternal instance. @@ -366,62 +300,46 @@ protected bool EnlistedTransactionDisposed } } - protected internal bool IsConnectionDoomed - { - get - { - return _connectionIsDoomed; - } - } + /// + /// when the connection should no longer be used. + /// + protected internal bool IsConnectionDoomed { get; private set; } - // Is this a connection that must be put in stasis (or is already in stasis) pending the end of it's transaction? - virtual protected internal bool IsNonPoolableTransactionRoot + /// + /// Is this a connection that must be put in stasis (or is already in stasis) pending the + /// end of its transaction? + /// + /// + /// If you want to have delegated transactions that are non-poolable, you had better + /// override this... + /// + protected internal virtual bool IsNonPoolableTransactionRoot { - get - { - return false; // if you want to have delegated transactions that are non-poolable, you better override this... - } + get => false; } + /// + /// We use a weak reference to the owning object so we can identify when it has been + /// garbage collected without throwing exceptions. + /// protected internal DbConnection Owner { - // We use a weak reference to the owning object so we can identify when - // it has been garbage collected without throwing exceptions. - get - { - if (_owningObject.TryGetTarget(out DbConnection connection)) - { - return connection; - } - return null; - } + get => _owningObject.TryGetTarget(out DbConnection connection) ? connection : null; } #if NETFRAMEWORK - protected DbConnectionPoolCounters PerformanceCounters - { - get - { - return _performanceCounters; - } - } + protected DbConnectionPoolCounters PerformanceCounters { get; private set; } #endif - virtual protected bool ReadyToPrepareTransaction + protected virtual bool ReadyToPrepareTransaction { - get - { - return true; - } + get => true; } - protected internal DbReferenceCollection ReferenceCollection - { - get - { - return _referenceCollection; - } - } + /// + /// Collection of objects that we need to notify in some way when we're being deactivated + /// + protected internal DbReferenceCollection ReferenceCollection { get; private set; } /// /// Get boolean that specifies whether an enlisted transaction can be unbound from @@ -430,12 +348,9 @@ protected internal DbReferenceCollection ReferenceCollection /// /// True if the enlisted transaction can be unbound on transaction completion; otherwise false. /// - virtual protected bool UnbindOnTransactionCompletion + protected virtual bool UnbindOnTransactionCompletion { - get - { - return true; - } + get => true; } #endregion @@ -465,15 +380,15 @@ internal void ActivateConnection(Transaction transaction) internal void AddWeakReference(object value, int tag) { - if (_referenceCollection == null) + if (ReferenceCollection == null) { - _referenceCollection = CreateReferenceCollection(); - if (_referenceCollection == null) + ReferenceCollection = CreateReferenceCollection(); + if (ReferenceCollection == null) { throw ADP.InternalError(ADP.InternalErrorCode.CreateReferenceCollectionReturnedNull); } } - _referenceCollection.Add(value, tag); + ReferenceCollection.Add(value, tag); } abstract public DbTransaction BeginTransaction(System.Data.IsolationLevel il); @@ -638,7 +553,7 @@ internal void DeactivateConnection() SqlClientEventSource.Log.ExitActiveConnection(); #endif - if (!_connectionIsDoomed && Pool.UseLoadBalancing) + if (!IsConnectionDoomed && Pool.UseLoadBalancing) { // If we're not already doomed, check the connection's lifetime and // doom it if it's lifetime has elapsed. @@ -765,12 +680,12 @@ internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleas public virtual void Dispose() { - _connectionPool = null; - _connectionIsDoomed = true; + Pool = null; + IsConnectionDoomed = true; _enlistedTransactionOriginal = null; // should not be disposed #if NETFRAMEWORK - _performanceCounters = null; + PerformanceCounters = null; #endif // Dispose of the _enlistedTransaction since it is a clone @@ -806,10 +721,10 @@ internal void MakeNonPooledObject(DbConnection owningObject) // a connection pool. #if NETFRAMEWORK - _performanceCounters = performanceCounters; + PerformanceCounters = performanceCounters; #endif - _connectionPool = null; + Pool = null; _owningObject.SetTarget(owningObject); _pooledCount = -1; } @@ -820,10 +735,10 @@ internal void MakePooledConnection(DbConnectionPool connectionPool) // a connection pool. _createTime = DateTime.UtcNow; - _connectionPool = connectionPool; + Pool = connectionPool; #if NETFRAMEWORK - _performanceCounters = connectionPool.PerformanceCounters; + PerformanceCounters = connectionPool.PerformanceCounters; #endif } @@ -933,7 +848,7 @@ internal void RemoveWeakReference(object value) internal void SetInStasis() { - _isInStasis = true; + IsTxRootWaitingForTxEnd = true; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Non-Pooled Connection has Delegated Transaction, waiting to Dispose.", ObjectID); #if NETFRAMEWORK @@ -988,7 +903,7 @@ protected internal void DoNotPoolThisConnection() #endif protected internal void DoomThisConnection() { - _connectionIsDoomed = true; + IsConnectionDoomed = true; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Dooming", ObjectID); } @@ -1051,7 +966,7 @@ protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnect // Reset connection doomed status so it can be re-connected and pooled. protected internal void UnDoomThisConnection() { - _connectionIsDoomed = false; + IsConnectionDoomed = false; } #endregion @@ -1075,7 +990,7 @@ private void TerminateStasis(bool returningToPool) SqlClientEventSource.Log.ExitStasisConnection(); #endif - _isInStasis = false; + IsTxRootWaitingForTxEnd = false; } void TransactionCompletedEvent(object sender, TransactionEventArgs e) From ddbbb98141a32ff2902c0a5711e9b28122a0f911 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 10 Oct 2024 15:10:59 -0500 Subject: [PATCH 14/19] Cleaning up public/internal methods --- .../Data/ProviderBase/DbConnectionInternal.cs | 275 +++++++++--------- .../SqlInternalConnectionSmi.stub.cs | 8 +- 2 files changed, 147 insertions(+), 136 deletions(-) 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 a1d6121375..a6d37ba1df 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -346,7 +346,8 @@ protected virtual bool ReadyToPrepareTransaction /// the connection when that transaction completes. /// /// - /// True if the enlisted transaction can be unbound on transaction completion; otherwise false. + /// if the enlisted transaction can be unbound on transaction + /// completion; otherwise . /// protected virtual bool UnbindOnTransactionCompletion { @@ -357,17 +358,12 @@ protected virtual bool UnbindOnTransactionCompletion #region Public/Internal Methods - abstract protected void Activate(Transaction transaction); - internal void ActivateConnection(Transaction transaction) { // Internal method called from the connection pooler so we don't expose // the Activate method publicly. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Activating", ObjectID); -#if DEBUG - int activateCount = Interlocked.Increment(ref _activateCount); - Debug.Assert(1 == activateCount, "activated multiple times?"); -#endif // DEBUG + Debug.Assert(Interlocked.Increment(ref _activateCount) == 1, "activated multiple times?"); Activate(transaction); @@ -380,20 +376,21 @@ internal void ActivateConnection(Transaction transaction) internal void AddWeakReference(object value, int tag) { - if (ReferenceCollection == null) + if (ReferenceCollection is null) { ReferenceCollection = CreateReferenceCollection(); - if (ReferenceCollection == null) + if (ReferenceCollection is null) { throw ADP.InternalError(ADP.InternalErrorCode.CreateReferenceCollectionReturnedNull); } } + ReferenceCollection.Add(value, tag); } - abstract public DbTransaction BeginTransaction(System.Data.IsolationLevel il); + public abstract DbTransaction BeginTransaction(System.Data.IsolationLevel il); - virtual public void ChangeDatabase(string value) + public virtual void ChangeDatabase(string value) { throw ADP.MethodNotImplemented(); } @@ -404,10 +401,7 @@ internal void CleanupConnectionOnTransactionCompletion(Transaction transaction) DetachTransaction(transaction, false); DbConnectionPool pool = Pool; - if (pool != null) - { - pool.TransactionEnded(transaction, this); - } + pool?.TransactionEnded(transaction, this); } internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) @@ -448,8 +442,8 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac //////////////////////////////////////////////////////////////// // DON'T MESS WITH THIS CODE UNLESS YOU KNOW WHAT YOU'RE DOING! //////////////////////////////////////////////////////////////// - Debug.Assert(owningObject != null, "null owningObject"); - Debug.Assert(connectionFactory != null, "null connectionFactory"); + Debug.Assert(owningObject is not null, "null owningObject"); + Debug.Assert(connectionFactory is not null, "null connectionFactory"); SqlClientEventSource.Log.TryPoolerTraceEvent(" {0} Closing.", ObjectID); // if an exception occurs after the state change but before the try block @@ -459,7 +453,6 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac // Open->Closed: guarantee internal connection is returned to correct pool if (connectionFactory.SetInnerConnectionFrom(owningObject, DbConnectionOpenBusy.SingletonInstance, this)) { - // Lock to prevent race condition with cancellation lock (this) { @@ -476,16 +469,20 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac // The singleton closed classes won't have owners and // connection pools, and we won't want to put them back // into the pool. - if (connectionPool != null) + if (connectionPool is not null) { - connectionPool.PutObject(this, owningObject); // PutObject calls Deactivate for us... - // NOTE: Before we leave the PutObject call, another - // thread may have already popped the connection from - // the pool, so don't expect to be able to verify it. + // PutObject calls Deactivate for us... + connectionPool.PutObject(this, owningObject); + + // NOTE: Before we leave the PutObject call, another thread may have + // already popped the connection from the pool, so don't expect to be + // able to verify it. } else { - Deactivate(); // ensure we de-activate non-pooled connections, or the data readers and transactions may not get cleaned up... + // Ensure we de-activate non-pooled connections, or the data readers + // and transactions may not get cleaned up... + Deactivate(); #if NETFRAMEWORK PerformanceCounters.HardDisconnectsPerSecond.Increment(); @@ -493,13 +490,11 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac SqlClientEventSource.Log.HardDisconnectRequest(); #endif - // To prevent an endless recursion, we need to clear - // the owning object before we call dispose so that - // we can't get here a second time... Ordinarily, I - // would call setting the owner to null a hack, but - // this is safe since we're about to dispose the - // object and it won't have an owner after that for - // certain. + // To prevent an endless recursion, we need to clear the owning object + // before we call dispose so that we can't get here a second time... + // Ordinarily, I would call setting the owner to null a hack, but this + // is safe since we're about to dispose the object, and it won't have + // an owner after that for certain. _owningObject.SetTarget(null); if (IsTransactionRoot) @@ -510,7 +505,7 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac { #if NETFRAMEWORK PerformanceCounters.NumberOfNonPooledConnections.Decrement(); - if (this.GetType() != typeof(Microsoft.Data.SqlClient.SqlInternalConnectionSmi)) + if (this is not SqlInternalConnectionSmi) { Dispose(); } @@ -524,9 +519,13 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac finally { ReleaseAdditionalLocksForClose(lockToken); - // if a ThreadAbort puts us here then its possible the outer connection will not reference - // this and this will be orphaned, not reclaimed by object pool until outer connection goes out of scope. - connectionFactory.SetInnerConnectionEvent(owningObject, DbConnectionClosedPreviouslyOpened.SingletonInstance); + + // If a ThreadAbort puts us here then its possible the outer connection + // will not reference this and this will be orphaned, not reclaimed by + // object pool until outer connection goes out of scope. + connectionFactory.SetInnerConnectionEvent( + owningObject, + DbConnectionClosedPreviouslyOpened.SingletonInstance); } } } @@ -537,16 +536,12 @@ internal void DeactivateConnection() // Internal method called from the connection pooler so we don't expose // the Deactivate method publicly. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Deactivating", ObjectID); -#if DEBUG - int activateCount = Interlocked.Decrement(ref _activateCount); - #if NETFRAMEWORK - Debug.Assert(0 == activateCount, "activated multiple times?"); - #endif -#endif // DEBUG + Debug.Assert(Interlocked.Decrement(ref _activateCount) == 0, "activated multiple times?"); #if NETFRAMEWORK - if (PerformanceCounters != null) - { // Pool.Clear will DestroyObject that will clean performanceCounters before going here + if (PerformanceCounters is not null) + { + // Pool.Clear will DestroyObject that will clean performanceCounters before going here PerformanceCounters.NumberOfActiveConnections.Decrement(); } #else @@ -557,9 +552,8 @@ internal void DeactivateConnection() { // If we're not already doomed, check the connection's lifetime and // doom it if it's lifetime has elapsed. - DateTime now = DateTime.UtcNow; - if ((now.Ticks - _createTime.Ticks) > Pool.LoadBalanceTimeout.Ticks) + if (now.Ticks - _createTime.Ticks > Pool.LoadBalanceTimeout.Ticks) { DoNotPoolThisConnection(); } @@ -567,22 +561,21 @@ internal void DeactivateConnection() Deactivate(); } - virtual internal void DelegatedTransactionEnded() + internal virtual void DelegatedTransactionEnded() { - // Called by System.Transactions when the delegated transaction has - // completed. We need to make closed connections that are in stasis - // available again, or disposed closed/leaked non-pooled connections. + // Called by System.Transactions when the delegated transaction has completed. We need + // to make closed connections that are in stasis available again, or disposed + // closed/leaked non-pooled connections. // IMPORTANT NOTE: You must have taken a lock on the object before // you call this method to prevent race conditions with Clear and // ReclaimEmancipatedObjects. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction Completed.", ObjectID); - if (1 == _pooledCount) + if (_pooledCount == 1) { - // When _pooledCount is 1, it indicates a closed, pooled, - // connection so it is ready to put back into the pool for - // general use. + // When _pooledCount is 1, it indicates a closed, pooled, connection so it is ready + // to put back into the pool for general use. TerminateStasis(true); @@ -592,11 +585,13 @@ virtual internal void DelegatedTransactionEnded() if (pool == null) { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectWithoutPool); // pooled connection does not have a pool + // pooled connection does not have a pool + throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectWithoutPool); } + pool.PutObjectFromTransactedPool(this); } - else if (-1 == _pooledCount && !_owningObject.TryGetTarget(out _)) + else if (_pooledCount == -1 && !_owningObject.TryGetTarget(out _)) { // When _pooledCount is -1 and the owning object no longer exists, // it indicates a closed (or leaked), non-pooled connection so @@ -604,7 +599,8 @@ virtual internal void DelegatedTransactionEnded() TerminateStasis(false); - Deactivate(); // call it one more time just in case + // Call it one more time just in case + Deactivate(); // it's a non-pooled connection, we need to dispose of it // once and for all, or the server will have fits about us @@ -618,6 +614,7 @@ virtual internal void DelegatedTransactionEnded() Dispose(); } + // When _pooledCount is 0, the connection is a pooled connection // that is either open (if the owning object is alive) or leaked (if // the owning object is not alive) In either case, we can't muck @@ -632,11 +629,12 @@ internal void DetachCurrentTransactionIfEnded() bool transactionIsDead; try { - transactionIsDead = (TransactionStatus.Active != enlistedTransaction.TransactionInformation.Status); + transactionIsDead = enlistedTransaction.TransactionInformation.Status != TransactionStatus.Active; } catch (TransactionException) { - // If the transaction is being processed (i.e. is part way through a rollback\commit\etc then TransactionInformation.Status will throw an exception) + // If the transaction is being processed (i.e. is partially through a rollback\ + // commit\etc then TransactionInformation.Status will throw an exception) transactionIsDead = true; } if (transactionIsDead) @@ -651,10 +649,12 @@ internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleas { SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount={1})", ObjectID, _pooledCount); - // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new - // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should - // be the exception, not the rule. - // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. + // Potentially a multithreaded event, so lock the connection to make sure we don't + // enlist in a new transaction between compare and assignment. No need to short + // circuit outside of lock, since failed comparisons should be the exception, not the + // rule. + // Locking on anything other than the transaction object would lead to a thread + // deadlock with System.Transaction.TransactionCompleted event. lock (transaction) { // Detach if detach-on-end behavior, or if outer connection was closed @@ -664,7 +664,8 @@ internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleas Transaction currentEnlistedTransaction = _enlistedTransaction; if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) { - // We need to remove the transaction completed event handler to cease listening for the transaction to end. + // We need to remove the transaction completed event handler to cease + // listening for the transaction to end. currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; EnlistedTransaction = null; @@ -688,8 +689,7 @@ public virtual void Dispose() PerformanceCounters = null; #endif - // Dispose of the _enlistedTransaction since it is a clone - // of the original reference. + // Dispose of the _enlistedTransaction since it is a clone of the original reference. // VSDD 780271 - _enlistedTransaction can be changed by another thread (TX end event) Transaction enlistedTransaction = Interlocked.Exchange(ref _enlistedTransaction, null); if (enlistedTransaction != null) @@ -698,28 +698,32 @@ public virtual void Dispose() } } - abstract public void EnlistTransaction(Transaction transaction); + public abstract void EnlistTransaction(Transaction transaction); /// - /// When overridden in a derived class, will check if the underlying connection is still actually alive + /// When overridden in a derived class, will check if the underlying connection is still + /// actually alive. /// - /// If true an exception will be thrown if the connection is dead instead of returning true\false - /// (this allows the caller to have the real reason that the connection is not alive (e.g. network error, etc)) - /// True if the connection is still alive, otherwise false (If not overridden, then always true) - internal virtual bool IsConnectionAlive(bool throwOnException = false) - { - return true; - } + /// + /// If true an exception will be thrown if the connection is dead instead of returning + /// true\false (this allows the caller to have the real reason that the connection is not + /// alive (e.g. network error, etc.)). + /// + /// + /// if the connection is still alive, otherwise . + /// (If not overridden, then always true) + /// + internal virtual bool IsConnectionAlive(bool throwOnException = false) => true; + /// + /// Used by DbConnectionFactory to indicate that this object IS NOT part of a connection pool. + /// #if NETFRAMEWORK internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters) #else internal void MakeNonPooledObject(DbConnection owningObject) #endif { - // Used by DbConnectionFactory to indicate that this object IS NOT part of - // a connection pool. - #if NETFRAMEWORK PerformanceCounters = performanceCounters; #endif @@ -729,12 +733,13 @@ internal void MakeNonPooledObject(DbConnection owningObject) _pooledCount = -1; } + /// + /// Used by DbConnectionFactory to indicate that this object IS part of a connection pool. + /// + /// internal void MakePooledConnection(DbConnectionPool connectionPool) { - // Used by DbConnectionFactory to indicate that this object IS part of - // a connection pool. _createTime = DateTime.UtcNow; - Pool = connectionPool; #if NETFRAMEWORK @@ -742,14 +747,8 @@ internal void MakePooledConnection(DbConnectionPool connectionPool) #endif } - internal void NotifyWeakReference(int message) - { - DbReferenceCollection referenceCollection = ReferenceCollection; - if (referenceCollection != null) - { - referenceCollection.Notify(message); - } - } + internal void NotifyWeakReference(int message) => + ReferenceCollection?.Notify(message); internal virtual void OpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory) { @@ -761,91 +760,85 @@ internal virtual void OpenConnection(DbConnection outerConnection, DbConnectionF internal void PostPop(DbConnection newOwner) { - // Called by DbConnectionPool right after it pulls this from it's pool, we - // take this opportunity to ensure ownership and pool counts are legit. - + // Called by DbConnectionPool right after it pulls this from its pool, we take this + // opportunity to ensure ownership and pool counts are legit. Debug.Assert(!IsEmancipated, "pooled object not in pool"); - // SQLBUDT #356871 -- When another thread is clearing this pool, it - // will doom all connections in this pool without prejudice which - // causes the following assert to fire, which really mucks up stress - // against checked bits. The assert is benign, so we're commenting - // it out. - //Debug.Assert(CanBePooled, "pooled object is not poolable"); - - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. + // IMPORTANT NOTE: You must have taken a lock on the object before you call this method + // to prevent race conditions with Clear and ReclaimEmancipatedObjects. if (_owningObject.TryGetTarget(out _)) { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner! + // Pooled connection already has an owner! + throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); } + _owningObject.SetTarget(newOwner); _pooledCount--; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); //3 // The following tests are retail assertions of things we can't allow to happen. - if (Pool != null) + if (Pool is not null) { - if (0 != _pooledCount) + if (_pooledCount != 0) { - throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectInPoolMoreThanOnce); // popping object off stack with multiple pooledCount + // Popping object off stack with multiple pooledCount + throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectInPoolMoreThanOnce); } } - else if (-1 != _pooledCount) + else if (_pooledCount != -1) { - throw ADP.InternalError(ADP.InternalErrorCode.NonPooledObjectUsedMoreThanOnce); // popping object off stack with multiple pooledCount + // Popping object off stack with multiple pooledCount + throw ADP.InternalError(ADP.InternalErrorCode.NonPooledObjectUsedMoreThanOnce); } } - virtual internal void PrepareForReplaceConnection() + internal virtual void PrepareForReplaceConnection() { // By default, there is no preparation required } internal void PrePush(object expectedOwner) { - // Called by DbConnectionPool when we're about to be put into it's pool, we - // take this opportunity to ensure ownership and pool counts are legit. + // Called by DbConnectionPool when we're about to be put into it's pool, we take this + // opportunity to ensure ownership and pool counts are legit. - // IMPORTANT NOTE: You must have taken a lock on the object before - // you call this method to prevent race conditions with Clear and - // ReclaimEmancipatedObjects. + // IMPORTANT NOTE: You must have taken a lock on the object before you call this method + // to prevent race conditions with Clear and ReclaimEmancipatedObjects. - //3 // The following tests are retail assertions of things we can't allow to happen. + // The following tests are retail assertions of things we can't allow to happen. bool isAlive = _owningObject.TryGetTarget(out DbConnection connection); - if (expectedOwner == null) + if (expectedOwner is null) { if (isAlive) { - throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasOwner); // new unpooled object has an owner + // New unpooled object has an owner + throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasOwner); } } else if (isAlive && connection != expectedOwner) { - throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasWrongOwner); // unpooled object has incorrect owner + // Unpooled object has incorrect owner + throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasWrongOwner); } - if (0 != _pooledCount) + if (_pooledCount != 0) { - throw ADP.InternalError(ADP.InternalErrorCode.PushingObjectSecondTime); // pushing object onto stack a second time + // Pushing object onto stack a second time + throw ADP.InternalError(ADP.InternalErrorCode.PushingObjectSecondTime); } SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Preparing to push into pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount); _pooledCount++; - _owningObject.SetTarget(null); // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2% - } - internal void RemoveWeakReference(object value) - { - DbReferenceCollection referenceCollection = ReferenceCollection; - if (referenceCollection != null) - { - referenceCollection.Remove(value); - } + // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the + // close by 2% + _owningObject.SetTarget(null); } + internal void RemoveWeakReference(object value) => + ReferenceCollection?.Remove(value); + internal void SetInStasis() { IsTxRootWaitingForTxEnd = true; @@ -858,17 +851,27 @@ internal void SetInStasis() #endif } - /// The default implementation is for the open connection objects, and - /// it simply throws. Our private closed-state connection objects - /// override this and do the correct thing. - // User code should either override DbConnectionInternal.Activate when it comes out of the pool - // or override DbConnectionFactory.CreateConnection when the connection is created for non-pooled connections - internal virtual bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + /// + /// The default implementation is for the open connection objects, and it simply throws. + /// Our private closed-state connection objects override this and do the correct thing. + /// User code should either override DbConnectionInternal.Activate when it comes out of the + /// pool or override DbConnectionFactory.CreateConnection when the connection is created + /// for non-pooled connections. + /// + internal virtual bool TryOpenConnection( + DbConnection outerConnection, + DbConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) { throw ADP.ConnectionAlreadyOpen(State); } - internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + internal virtual bool TryReplaceConnection( + DbConnection outerConnection, + DbConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) { throw ADP.MethodNotImplemented(); } @@ -877,6 +880,8 @@ internal virtual bool TryReplaceConnection(DbConnection outerConnection, DbConne #region Protected Methods + protected abstract void Activate(Transaction transaction); + // Cleanup connection's transaction-specific structures (currently used by Delegated transaction). // This is a separate method because cleanup can be triggered in multiple ways for a delegated // transaction. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs index aaafc74b5a..333cc3bf5f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalConnectionSmi.stub.cs @@ -2,14 +2,20 @@ // 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; + #if NETFRAMEWORK namespace Microsoft.Data.SqlClient { // DO NOT USE THIS FILE IN ANY PROJECT! // This is a temporary stub to enable migrating DbConnectionInternal to the common project. - internal class SqlInternalConnectionSmi + internal abstract class SqlInternalConnectionSmi : SqlInternalConnection { + protected SqlInternalConnectionSmi(SqlConnectionString connectionOptions) : base(connectionOptions) + { + throw new NotImplementedException(); + } } } From 31ce64b46c8818ca9b1f3f9456e72a7578c16508 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 10 Oct 2024 15:17:15 -0500 Subject: [PATCH 15/19] Cleaning up protected methods - adding trace event for undooming a connection. --- .../Data/ProviderBase/DbConnectionInternal.cs | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) 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 a6d37ba1df..8eed7cecde 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -882,19 +882,21 @@ internal virtual bool TryReplaceConnection( protected abstract void Activate(Transaction transaction); - // Cleanup connection's transaction-specific structures (currently used by Delegated transaction). - // This is a separate method because cleanup can be triggered in multiple ways for a delegated - // transaction. - virtual protected void CleanupTransactionOnCompletion(Transaction transaction) + /// + /// Cleanup connection's transaction-specific structures (currently used by Delegated transaction). + /// This is a separate method because cleanup can be triggered in multiple ways for a delegated + /// transaction. + /// + protected virtual void CleanupTransactionOnCompletion(Transaction transaction) { } - virtual protected DbReferenceCollection CreateReferenceCollection() + protected virtual DbReferenceCollection CreateReferenceCollection() { throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject); } - abstract protected void Deactivate(); + protected abstract void Deactivate(); protected internal void DoNotPoolThisConnection() { @@ -902,7 +904,9 @@ protected internal void DoNotPoolThisConnection() SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Marking pooled object as non-poolable so it will be disposed", ObjectID); } - /// Ensure that this connection cannot be put back into the pool. + /// + /// Ensure that this connection cannot be put back into the pool. + /// #if NETFRAMEWORK [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] #endif @@ -912,29 +916,35 @@ protected internal void DoomThisConnection() SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Dooming", ObjectID); } - protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + protected internal virtual DataTable GetSchema( + DbConnectionFactory factory, + DbConnectionPoolGroup poolGroup, + DbConnection outerConnection, + string collectionName, + string[] restrictions) { - Debug.Assert(outerConnection != null, "outerConnection may not be null."); + Debug.Assert(outerConnection is not null, "outerConnection may not be null."); DbMetaDataFactory metaDataFactory = factory.GetMetaDataFactory(poolGroup, this); - Debug.Assert(metaDataFactory != null, "metaDataFactory may not be null."); + Debug.Assert(metaDataFactory is not null, "metaDataFactory may not be null."); return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions); } - virtual protected bool ObtainAdditionalLocksForClose() + protected virtual bool ObtainAdditionalLocksForClose() { - return false; // no additional locks in default implementation + // No additional locks in default implementation + return false; } - virtual protected void PrepareForCloseConnection() + protected virtual void PrepareForCloseConnection() { // By default, there is no preparation required } - virtual protected void ReleaseAdditionalLocksForClose(bool lockToken) + protected virtual void ReleaseAdditionalLocksForClose(bool lockToken) { - // no additional locks in default implementation + // No additional locks in default implementation } protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) @@ -942,7 +952,7 @@ protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnect // ?->Connecting: prevent set_ConnectionString during Open if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) { - DbConnectionInternal openConnection = null; + DbConnectionInternal openConnection; try { connectionFactory.PermissionDemand(outerConnection); @@ -957,21 +967,26 @@ protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnect connectionFactory.SetInnerConnectionTo(outerConnection, this); throw; } + if (openConnection == null) { connectionFactory.SetInnerConnectionTo(outerConnection, this); throw ADP.InternalConnectionError(ADP.ConnectionError.GetConnectionReturnsNull); } + connectionFactory.SetInnerConnectionEvent(outerConnection, openConnection); } return true; } - // Reset connection doomed status so it can be re-connected and pooled. - protected internal void UnDoomThisConnection() + /// + /// Reset connection doomed status so it can be re-connected and pooled. + /// + protected void UnDoomThisConnection() { IsConnectionDoomed = false; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, UnDooming", ObjectID); } #endregion From fe952fe4db359f7a4bfb3b7c839338b5f3469f53 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 10 Oct 2024 15:20:35 -0500 Subject: [PATCH 16/19] Cleaning up private methods --- .../Data/ProviderBase/DbConnectionInternal.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) 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 8eed7cecde..fe68768c5d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -995,14 +995,10 @@ protected void UnDoomThisConnection() private void TerminateStasis(bool returningToPool) { - if (returningToPool) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed. Returning to general pool.", ObjectID); - } - else - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Delegated Transaction has ended, connection is closed/leaked. Disposing.", ObjectID); - } + string message = returningToPool + ? "Delegated Transaction has ended, connection is closed. Returning to general pool." + : "Delegated Transaction has ended, connection is closed/leaked. Disposing."; + SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, {1}", ObjectID, message); #if NETFRAMEWORK PerformanceCounters.NumberOfStasisConnections.Decrement(); @@ -1013,7 +1009,7 @@ private void TerminateStasis(bool returningToPool) IsTxRootWaitingForTxEnd = false; } - void TransactionCompletedEvent(object sender, TransactionEventArgs e) + private void TransactionCompletedEvent(object sender, TransactionEventArgs e) { Transaction transaction = e.Transaction; SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction Completed. (pooledCount = {1})", ObjectID, _pooledCount); @@ -1028,7 +1024,7 @@ void TransactionCompletedEvent(object sender, TransactionEventArgs e) #endif private void TransactionOutcomeEnlist(Transaction transaction) { - _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); + _transactionCompletedEventHandler ??= TransactionCompletedEvent; transaction.TransactionCompleted += _transactionCompletedEventHandler; } From 4d6f4e2521f105ff89073b4b5fc30d78cdf32f6c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 10 Oct 2024 18:09:47 -0500 Subject: [PATCH 17/19] Fix issues from cleanup --- .../src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs | 2 +- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs index 0de99051f3..4f5a70bf7f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs @@ -465,7 +465,7 @@ internal void TransactionEnded(Transaction transaction) if (connection != null) { - SqlClientEventSource.Log.TryTraceEvent("SqlDelegatedTransaction.TransactionEnded | RES | CPOOL | Object Id {0}, Connection Id {1}, transaction completed externally.", ObjectID, connection?._objectID); + SqlClientEventSource.Log.TryTraceEvent("SqlDelegatedTransaction.TransactionEnded | RES | CPOOL | Object Id {0}, Connection Id {1}, transaction completed externally.", ObjectID, connection.ObjectID); lock (connection) { if (_atomicTransaction.Equals(transaction)) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 6119676b6e..02668842d2 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -375,7 +375,7 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj) { if (stateObj._attentionSent) { - SqlClientEventSource.Log.TryTraceEvent("TdsParser.ProcessPendingAck | INFO | Connection Object Id {0}, State Obj Id {1}, Processing Attention.", _connHandler._objectID, stateObj.ObjectID); + SqlClientEventSource.Log.TryTraceEvent("TdsParser.ProcessPendingAck | INFO | Connection Object Id {0}, State Obj Id {1}, Processing Attention.", _connHandler.ObjectID, stateObj.ObjectID); ProcessAttention(stateObj); } } @@ -419,7 +419,7 @@ internal void Connect( else { _sniSpnBuffer = null; - SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler._objectID, + SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); } From d87240dcf3bccb6d20e381590efb927c7747fc43 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 11 Oct 2024 11:57:26 -0500 Subject: [PATCH 18/19] Aw man, I thought I was being fancy with using Debug stuff. --- .../Data/ProviderBase/DbConnectionInternal.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 fe68768c5d..6c06db4ea2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -363,7 +363,11 @@ internal void ActivateConnection(Transaction transaction) // Internal method called from the connection pooler so we don't expose // the Activate method publicly. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Activating", ObjectID); - Debug.Assert(Interlocked.Increment(ref _activateCount) == 1, "activated multiple times?"); + + #if DEBUG + int activateCount = Interlocked.Increment(ref _activateCount); + Debug.Assert(activateCount == 1, "activated multiple times?"); + #endif Activate(transaction); @@ -536,7 +540,11 @@ internal void DeactivateConnection() // Internal method called from the connection pooler so we don't expose // the Deactivate method publicly. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Deactivating", ObjectID); - Debug.Assert(Interlocked.Decrement(ref _activateCount) == 0, "activated multiple times?"); + + #if DEBUG + int activateCount = Interlocked.Decrement(ref _activateCount); + Debug.Assert(activateCount == 0, "activated multiple times?"); + #endif #if NETFRAMEWORK if (PerformanceCounters is not null) From a2580810fcf62557fefa559f6a7be76ee98f91ea Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Mon, 14 Oct 2024 12:31:38 -0500 Subject: [PATCH 19/19] Adopt netfx implementation for IsEmancipated --- .../src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 6c06db4ea2..33c7f23335 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -127,12 +127,8 @@ internal bool IsEmancipated // Now, our job is to return TRUE when the connection is out // of the pool and it's owning object is no longer around to // return it. - - #if NETFRAMEWORK + return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - #else - return (_pooledCount < 1) && !_owningObject.TryGetTarget(out _); - #endif } }