From 276966730c1942b94c54c9bb1611c1d708b104ba Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 13:02:01 -0500 Subject: [PATCH 01/20] Sort modifiers --- .../Data/SqlClient/SqlConnectionFactory.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index ae4c7bc98d..a4c5fee9af 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -18,9 +18,9 @@ namespace Microsoft.Data.SqlClient { - sealed internal class SqlConnectionFactory : DbConnectionFactory + internal sealed class SqlConnectionFactory : DbConnectionFactory { - public static readonly SqlConnectionFactory SingletonInstance = new SqlConnectionFactory(); + internal static readonly SqlConnectionFactory SingletonInstance = new SqlConnectionFactory(); private SqlConnectionFactory() : base() { @@ -29,7 +29,7 @@ private SqlConnectionFactory() : base() #endif } - override public DbProviderFactory ProviderFactory + public override DbProviderFactory ProviderFactory { get { @@ -245,7 +245,7 @@ internal static SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKe } // @TODO: All these methods seem redundant ... shouldn't we always have a SqlConnection? - override internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection) + internal override DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -255,7 +255,7 @@ override internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection conn return null; } - override internal DbConnectionInternal GetInnerConnection(DbConnection connection) + internal override DbConnectionInternal GetInnerConnection(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -265,7 +265,7 @@ override internal DbConnectionInternal GetInnerConnection(DbConnection connectio return null; } - override protected int GetObjectId(DbConnection connection) + protected override int GetObjectId(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -275,7 +275,7 @@ override protected int GetObjectId(DbConnection connection) return 0; } - override internal void PermissionDemand(DbConnection outerConnection) + internal override void PermissionDemand(DbConnection outerConnection) { SqlConnection c = (outerConnection as SqlConnection); if (c != null) @@ -284,7 +284,7 @@ override internal void PermissionDemand(DbConnection outerConnection) } } - override internal void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup) + internal override void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup) { SqlConnection c = (outerConnection as SqlConnection); if (c != null) @@ -293,7 +293,7 @@ override internal void SetConnectionPoolGroup(DbConnection outerConnection, DbCo } } - override internal void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to) + internal override void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to) { SqlConnection c = (owningObject as SqlConnection); if (c != null) @@ -302,7 +302,7 @@ override internal void SetInnerConnectionEvent(DbConnection owningObject, DbConn } } - override internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from) + internal override bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from) { SqlConnection c = (owningObject as SqlConnection); if (c != null) @@ -312,7 +312,7 @@ override internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConne return false; } - override internal void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to) + internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to) { SqlConnection c = (owningObject as SqlConnection); if (c != null) From b75c7c0c0333c7f6e7b3f5240bbac5d84b15a10c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 13:20:45 -0500 Subject: [PATCH 02/20] Replace all usages of DbConnectionFactory with SqlConnectionFactory let's see how this goes... --- .../Data/SqlClient/SqlConnectionHelper.cs | 4 +- .../SqlClient/SqlInternalConnectionTds.cs | 10 ++-- .../Data/SqlClient/SqlConnectionHelper.cs | 4 +- .../SqlClient/SqlInternalConnectionTds.cs | 10 ++-- .../Data/ProviderBase/DbConnectionClosed.cs | 50 ++++++++++++++----- .../Data/ProviderBase/DbConnectionFactory.cs | 4 +- .../Data/ProviderBase/DbConnectionInternal.cs | 16 +++--- .../ConnectionPool/ChannelDbConnectionPool.cs | 2 +- .../ConnectionPool/DbConnectionPoolGroup.cs | 6 +-- .../ConnectionPool/IDbConnectionPool.cs | 2 +- .../WaitHandleDbConnectionPool.cs | 12 ++--- 11 files changed, 77 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index 133854c1f0..14e6269e01 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -17,7 +17,7 @@ namespace Microsoft.Data.SqlClient { public sealed partial class SqlConnection : DbConnection { - private static readonly DbConnectionFactory s_connectionFactory = SqlConnectionFactory.SingletonInstance; + private static readonly SqlConnectionFactory s_connectionFactory = SqlConnectionFactory.SingletonInstance; private DbConnectionOptions _userConnectionOptions; private DbConnectionPoolGroup _poolGroup; @@ -42,7 +42,7 @@ internal int CloseCount } } - internal DbConnectionFactory ConnectionFactory + internal SqlConnectionFactory ConnectionFactory { get { diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index 0d1da114cc..4c7cd8bf7d 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -3049,10 +3049,12 @@ internal bool ThreadHasParserLockForClose } } - internal override bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - return base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); - } + internal override bool TryReplaceConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => + TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); } internal sealed class ServerInfo diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index eb5cb511be..a6efe1cd37 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -18,7 +18,7 @@ namespace Microsoft.Data.SqlClient { public sealed partial class SqlConnection : DbConnection { - private static readonly DbConnectionFactory _connectionFactory = SqlConnectionFactory.SingletonInstance; + private static readonly SqlConnectionFactory _connectionFactory = SqlConnectionFactory.SingletonInstance; internal static readonly System.Security.CodeAccessPermission ExecutePermission = SqlConnection.CreateExecutePermission(); private DbConnectionOptions _userConnectionOptions; @@ -69,7 +69,7 @@ internal int CloseCount } } - internal DbConnectionFactory ConnectionFactory + internal SqlConnectionFactory ConnectionFactory { get { 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 d460c61619..9d55be1dd5 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 @@ -3081,10 +3081,12 @@ internal bool ThreadHasParserLockForClose } } - internal override bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - { - return base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); - } + internal override bool TryReplaceConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => + base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); } internal sealed class ServerInfo diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs index d4ea183312..0c2cf99ce7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.Data.Common; using Microsoft.Data.Common.ConnectionString; +using Microsoft.Data.SqlClient; using Microsoft.Data.SqlClient.ConnectionPool; namespace Microsoft.Data.ProviderBase @@ -27,7 +28,7 @@ protected DbConnectionClosed(ConnectionState state, bool hidePassword, bool allo public override void ChangeDatabase(string database) => throw ADP.ClosedConnectionError(); - internal override void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) + internal override void CloseConnection(DbConnection owningObject, SqlConnectionFactory connectionFactory) { // not much to do here... } @@ -36,13 +37,22 @@ internal override void CloseConnection(DbConnection owningObject, DbConnectionFa public override void EnlistTransaction(System.Transactions.Transaction transaction) => throw ADP.ClosedConnectionError(); - protected internal override DataTable GetSchema(DbConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, string[] restrictions) + protected internal override DataTable GetSchema( + SqlConnectionFactory factory, + DbConnectionPoolGroup poolGroup, + DbConnection outerConnection, + string collectionName, + string[] restrictions) => throw ADP.ClosedConnectionError(); protected override DbReferenceCollection CreateReferenceCollection() => throw ADP.ClosedConnectionError(); - internal override bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - => base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + internal override bool TryOpenConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => + TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); } internal abstract class DbConnectionBusy : DbConnectionClosed @@ -51,7 +61,11 @@ protected DbConnectionBusy(ConnectionState state) : base(state, true, false) { } - internal override bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + internal override bool TryOpenConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => throw ADP.ConnectionAlreadyOpen(State); } @@ -84,15 +98,23 @@ private DbConnectionClosedConnecting() : base(ConnectionState.Connecting) { } - internal override void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) + internal override void CloseConnection(DbConnection owningObject, SqlConnectionFactory connectionFactory) { connectionFactory.SetInnerConnectionTo(owningObject, DbConnectionClosedPreviouslyOpened.SingletonInstance); } - internal override bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - => TryOpenConnection(outerConnection, connectionFactory, retry, userOptions); - - internal override bool TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + internal override bool TryReplaceConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => + TryOpenConnection(outerConnection, connectionFactory, retry, userOptions); + + internal override bool TryOpenConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) { if (retry == null || !retry.Task.IsCompleted) { @@ -137,7 +159,11 @@ private DbConnectionClosedPreviouslyOpened() : base(ConnectionState.Closed, true { } - internal override bool TryReplaceConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) - => TryOpenConnection(outerConnection, connectionFactory, retry, userOptions); + internal override bool TryReplaceConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) => + TryOpenConnection(outerConnection, connectionFactory, retry, userOptions); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 399b98435b..bb24b5f1c7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -50,7 +50,7 @@ public abstract DbProviderFactory ProviderFactory public void ClearAllPools() { - using (TryEventScope.Create(nameof(DbConnectionFactory))) + using (TryEventScope.Create(nameof(SqlConnectionFactory))) { Dictionary connectionPoolGroups = _connectionPoolGroups; foreach (KeyValuePair entry in connectionPoolGroups) @@ -399,7 +399,7 @@ private IDbConnectionPool GetConnectionPool(DbConnection owningObject, DbConnect Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup?"); SetConnectionPoolGroup(owningObject, connectionPoolGroup); } - IDbConnectionPool connectionPool = connectionPoolGroup.GetConnectionPool(this); + IDbConnectionPool connectionPool = connectionPoolGroup.GetConnectionPool((SqlConnectionFactory)this); return connectionPool; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index b4dfb4f214..31f207e5ce 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -398,7 +398,7 @@ internal void CleanupConnectionOnTransactionCompletion(Transaction transaction) pool?.TransactionEnded(transaction, this); } - internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFactory connectionFactory) + internal virtual void CloseConnection(DbConnection owningObject, SqlConnectionFactory connectionFactory) { // The implementation here is the implementation required for the // "open" internal connections, since our own private "closed" @@ -708,7 +708,7 @@ internal void MakePooledConnection(IDbConnectionPool connectionPool) internal void NotifyWeakReference(int message) => ReferenceCollection?.Notify(message); - internal virtual void OpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory) + internal virtual void OpenConnection(DbConnection outerConnection, SqlConnectionFactory connectionFactory) { if (!TryOpenConnection(outerConnection, connectionFactory, null, null)) { @@ -814,7 +814,7 @@ internal void SetInStasis() /// internal virtual bool TryOpenConnection( DbConnection outerConnection, - DbConnectionFactory connectionFactory, + SqlConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { @@ -823,7 +823,7 @@ internal virtual bool TryOpenConnection( internal virtual bool TryReplaceConnection( DbConnection outerConnection, - DbConnectionFactory connectionFactory, + SqlConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) { @@ -871,7 +871,7 @@ protected internal void DoomThisConnection() } protected internal virtual DataTable GetSchema( - DbConnectionFactory factory, + SqlConnectionFactory factory, DbConnectionPoolGroup poolGroup, DbConnection outerConnection, string collectionName, @@ -901,7 +901,11 @@ protected virtual void ReleaseAdditionalLocksForClose(bool lockToken) // No additional locks in default implementation } - protected bool TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource retry, DbConnectionOptions userOptions) + protected bool TryOpenConnectionInternal( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + DbConnectionOptions userOptions) { // ?->Connecting: prevent set_ConnectionString during Open if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index c4772bb736..8b0efe8763 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -25,7 +25,7 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool public ConcurrentDictionary AuthenticationContexts => throw new NotImplementedException(); /// - public DbConnectionFactory ConnectionFactory => throw new NotImplementedException(); + public SqlConnectionFactory ConnectionFactory => throw new NotImplementedException(); /// public int Count => throw new NotImplementedException(); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs index 805beed2b5..226a5749b4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs @@ -132,7 +132,7 @@ internal int Clear() IDbConnectionPool pool = entry.Value; if (pool != null) { - DbConnectionFactory connectionFactory = pool.ConnectionFactory; + SqlConnectionFactory connectionFactory = pool.ConnectionFactory; connectionFactory.QueuePoolForRelease(pool, true); } @@ -143,7 +143,7 @@ internal int Clear() return _poolCollection.Count; } - internal IDbConnectionPool GetConnectionPool(DbConnectionFactory connectionFactory) + internal IDbConnectionPool GetConnectionPool(SqlConnectionFactory connectionFactory) { // When this method returns null it indicates that the connection // factory should not use pooling. @@ -282,7 +282,7 @@ internal bool Prune() // to use it while we're processing and finally we put the // pool into a list of pools to be released when they // are completely empty. - DbConnectionFactory connectionFactory = pool.ConnectionFactory; + SqlConnectionFactory connectionFactory = pool.ConnectionFactory; connectionFactory.QueuePoolForRelease(pool, false); } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs index d6647f832e..066318fce2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs @@ -28,7 +28,7 @@ internal interface IDbConnectionPool /// /// Gets the factory used to create database connections. /// - DbConnectionFactory ConnectionFactory { get; } + SqlConnectionFactory ConnectionFactory { get; } /// /// The number of connections currently managed by the pool. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 77d2b3bae1..6f359d981a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -403,7 +403,7 @@ internal WaitHandle[] GetHandles(bool withCreate) private readonly int _cleanupWait; private readonly DbConnectionPoolIdentity _identity; - private readonly DbConnectionFactory _connectionFactory; + private readonly SqlConnectionFactory _connectionFactory; private readonly DbConnectionPoolGroup _connectionPoolGroup; private readonly DbConnectionPoolGroupOptions _connectionPoolGroupOptions; private DbConnectionPoolProviderInfo _connectionPoolProviderInfo; @@ -439,10 +439,10 @@ internal WaitHandle[] GetHandles(bool withCreate) // only created by DbConnectionPoolGroup.GetConnectionPool internal WaitHandleDbConnectionPool( - DbConnectionFactory connectionFactory, - DbConnectionPoolGroup connectionPoolGroup, - DbConnectionPoolIdentity identity, - DbConnectionPoolProviderInfo connectionPoolProviderInfo) + SqlConnectionFactory connectionFactory, + DbConnectionPoolGroup connectionPoolGroup, + DbConnectionPoolIdentity identity, + DbConnectionPoolProviderInfo connectionPoolProviderInfo) { Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup"); @@ -492,7 +492,7 @@ private int CreationTimeout public int Count => _totalObjects; - public DbConnectionFactory ConnectionFactory => _connectionFactory; + public SqlConnectionFactory ConnectionFactory => _connectionFactory; public bool ErrorOccurred => _errorOccurred; From 9a4af4c816e36efbd6dd5fbdd8786184d02e6a01 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 13:37:08 -0500 Subject: [PATCH 03/20] Move member variables and constructor --- .../src/Microsoft/Data/Common/AdapterUtil.cs | 6 ++- .../Data/ProviderBase/DbConnectionFactory.cs | 35 -------------- .../Data/SqlClient/SqlConnectionFactory.cs | 48 +++++++++++++++---- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index 46e6091ad4..cebd223a1c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -130,7 +130,10 @@ internal static Exception ExceptionWithStackTrace(Exception e) } } - internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) + internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) => + UnsafeCreateTimer(callback, state, TimeSpan.FromSeconds(dueTime), TimeSpan.FromSeconds(period)); + + internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period) { // Don't capture the current ExecutionContext and its AsyncLocals onto // a global timer causing them to live forever @@ -154,6 +157,7 @@ internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, in } } } + #region COM+ exceptions internal static ArgumentException Argument(string error) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index bb24b5f1c7..ca5c2cb270 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,36 +18,8 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - private Dictionary _connectionPoolGroups; - private readonly List _poolsToRelease; - private readonly List _poolGroupsToRelease; - private readonly Timer _pruningTimer; - - private const int PruningDueTime = 4 * 60 * 1000; // 4 minutes - private const int PruningPeriod = 30 * 1000; // thirty seconds - - private static int _objectTypeCount; // EventSource counter internal int ObjectID { get; } = Interlocked.Increment(ref _objectTypeCount); - // s_pendingOpenNonPooled is an array of tasks used to throttle creation of non-pooled connections to - // a maximum of Environment.ProcessorCount at a time. - private static uint s_pendingOpenNonPooledNext = 0; - private static Task[] s_pendingOpenNonPooled = new Task[Environment.ProcessorCount]; - private static Task s_completedTask; - - protected DbConnectionFactory() - { - _connectionPoolGroups = new Dictionary(); - _poolsToRelease = new List(); - _poolGroupsToRelease = new List(); - _pruningTimer = CreatePruningTimer(); - } - - public abstract DbProviderFactory ProviderFactory - { - get; - } - public void ClearAllPools() { using (TryEventScope.Create(nameof(SqlConnectionFactory))) @@ -140,13 +112,6 @@ internal DbConnectionInternal CreatePooledConnection(IDbConnectionPool pool, DbC internal abstract DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( DbConnectionOptions connectionOptions); - private Timer CreatePruningTimer() => - ADP.UnsafeCreateTimer( - new TimerCallback(PruneConnectionPoolGroups), - null, - PruningDueTime, - PruningPeriod); - protected DbConnectionOptions FindConnectionOptions(DbConnectionPoolKey key) { Debug.Assert(key != null, "key cannot be null"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index a4c5fee9af..a3b717e7c5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -3,10 +3,13 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Data.Common; using System.Diagnostics; using System.IO; using System.Reflection; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Data.Common; using Microsoft.Data.Common.ConnectionString; using Microsoft.Data.ProviderBase; @@ -20,22 +23,49 @@ namespace Microsoft.Data.SqlClient { internal sealed class SqlConnectionFactory : DbConnectionFactory { - internal static readonly SqlConnectionFactory SingletonInstance = new SqlConnectionFactory(); + #region Member Variables - private SqlConnectionFactory() : base() + private static readonly TimeSpan PruningDueTime = TimeSpan.FromMinutes(4); + private static readonly TimeSpan PruningPeriod = TimeSpan.FromSeconds(30); + + // s_pendingOpenNonPooled is an array of tasks used to throttle creation of non-pooled + // connections to a maximum of Environment.ProcessorCount at a time. + private static Task s_completedTask; + private static int s_objectTypeCount; + private static Task[] s_pendingOpenNonPooled = + new Task[Environment.ProcessorCount]; + private static uint s_pendingOpenNonPooledNext = 0; + + private readonly List _poolGroupsToRelease; + private readonly List _poolsToRelease; + private readonly Timer _pruningTimer; + private Dictionary _connectionPoolGroups; + + #endregion + + #region Constructors + + private SqlConnectionFactory() { + _connectionPoolGroups = new Dictionary(); + _poolsToRelease = new List(); + _poolGroupsToRelease = new List(); + _pruningTimer = ADP.UnsafeCreateTimer( + new TimerCallback(PruneConnectionPoolGroups), + state: null, + PruningDueTime, + PruningPeriod); + #if NET SubscribeToAssemblyLoadContextUnload(); #endif } + + #endregion - public override DbProviderFactory ProviderFactory - { - get - { - return SqlClientFactory.Instance; - } - } + public static readonly SqlConnectionFactory SingletonInstance = new SqlConnectionFactory(); + + public DbProviderFactory ProviderFactory => SqlClientFactory.Instance; protected override DbConnectionInternal CreateConnection( DbConnectionOptions options, From 5a0b1e8733e76cfd9675b86f151b9e91a73a9362 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 13:42:49 -0500 Subject: [PATCH 04/20] Move properties over --- .../Microsoft/Data/SqlClient/SqlConnection.cs | 10 +++++----- .../Data/SqlClient/SqlConnectionHelper.cs | 4 ++-- .../Microsoft/Data/SqlClient/SqlConnection.cs | 6 +++--- .../Data/SqlClient/SqlConnectionHelper.cs | 6 +++--- .../Data/ProviderBase/DbConnectionFactory.cs | 4 +--- .../Data/SqlClient/SqlConnectionFactory.cs | 18 ++++++++++++------ 6 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs index 45df50badb..dae580e0bb 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -1272,7 +1272,7 @@ public override void ChangeDatabase(string database) /// public static void ClearAllPools() { - SqlConnectionFactory.SingletonInstance.ClearAllPools(); + SqlConnectionFactory.Instance.ClearAllPools(); } /// @@ -1283,7 +1283,7 @@ public static void ClearPool(SqlConnection connection) DbConnectionOptions connectionOptions = connection.UserConnectionOptions; if (connectionOptions != null) { - SqlConnectionFactory.SingletonInstance.ClearPool(connection); + SqlConnectionFactory.Instance.ClearPool(connection); } } @@ -2265,7 +2265,7 @@ public static void ChangePassword(string connectionString, string newPassword) SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential: null, accessToken: null, accessTokenCallback: null); - SqlConnectionString connectionOptions = SqlConnectionFactory.FindSqlConnectionOptions(key); + SqlConnectionString connectionOptions = SqlConnectionFactory.Instance.FindSqlConnectionOptions(key); if (connectionOptions.IntegratedSecurity) { throw SQL.ChangePasswordConflictsWithSSPI(); @@ -2314,7 +2314,7 @@ public static void ChangePassword(string connectionString, SqlCredential credent SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential, accessToken: null, accessTokenCallback: null); - SqlConnectionString connectionOptions = SqlConnectionFactory.FindSqlConnectionOptions(key); + SqlConnectionString connectionOptions = SqlConnectionFactory.Instance.FindSqlConnectionOptions(key); // Check for connection string values incompatible with SqlCredential if (!string.IsNullOrEmpty(connectionOptions.UserID) || !string.IsNullOrEmpty(connectionOptions.Password)) @@ -2352,7 +2352,7 @@ private static void ChangePassword(string connectionString, SqlConnectionString } SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential, accessToken: null, accessTokenCallback: null); - SqlConnectionFactory.SingletonInstance.ClearPool(key); + SqlConnectionFactory.Instance.ClearPool(key); } internal Task RegisterForConnectionCloseNotification(Task outerTask, object value, int tag) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index 14e6269e01..0888b400d0 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -17,7 +17,7 @@ namespace Microsoft.Data.SqlClient { public sealed partial class SqlConnection : DbConnection { - private static readonly SqlConnectionFactory s_connectionFactory = SqlConnectionFactory.SingletonInstance; + private static readonly SqlConnectionFactory s_connectionFactory = SqlConnectionFactory.Instance; private DbConnectionOptions _userConnectionOptions; private DbConnectionPoolGroup _poolGroup; @@ -157,7 +157,7 @@ override protected DbCommand CreateDbCommand() using (TryEventScope.Create(" {0}", ObjectID)) { DbCommand command = null; - DbProviderFactory providerFactory = ConnectionFactory.ProviderFactory; + DbProviderFactory providerFactory = SqlConnectionFactory.ProviderFactory; command = providerFactory.CreateCommand(); command.Connection = this; return command; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs index 9681a6a3ae..76bad9d82c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -1266,7 +1266,7 @@ public override void ChangeDatabase(string database) public static void ClearAllPools() { (new SqlClientPermission(PermissionState.Unrestricted)).Demand(); - SqlConnectionFactory.SingletonInstance.ClearAllPools(); + SqlConnectionFactory.Instance.ClearAllPools(); } /// @@ -1278,7 +1278,7 @@ public static void ClearPool(SqlConnection connection) if (connectionOptions != null) { connectionOptions.DemandPermission(); - SqlConnectionFactory.SingletonInstance.ClearPool(connection); + SqlConnectionFactory.Instance.ClearPool(connection); } } @@ -2290,7 +2290,7 @@ private static void ChangePassword(string connectionString, SqlConnectionString } SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential, accessToken: null, accessTokenCallback: null); - SqlConnectionFactory.SingletonInstance.ClearPool(key); + SqlConnectionFactory.Instance.ClearPool(key); } internal Task RegisterForConnectionCloseNotification(Task outerTask, object value, int tag) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index a6efe1cd37..3e2204b683 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -18,7 +18,7 @@ namespace Microsoft.Data.SqlClient { public sealed partial class SqlConnection : DbConnection { - private static readonly SqlConnectionFactory _connectionFactory = SqlConnectionFactory.SingletonInstance; + private static readonly SqlConnectionFactory _connectionFactory = SqlConnectionFactory.Instance; internal static readonly System.Security.CodeAccessPermission ExecutePermission = SqlConnection.CreateExecutePermission(); private DbConnectionOptions _userConnectionOptions; @@ -203,7 +203,7 @@ override protected DbCommand CreateDbCommand() { using (TryEventScope.Create(" {0}", ObjectID)) { - DbProviderFactory providerFactory = ConnectionFactory.ProviderFactory; + DbProviderFactory providerFactory = SqlConnectionFactory.ProviderFactory; DbCommand command = providerFactory.CreateCommand(); command.Connection = this; return command; @@ -212,7 +212,7 @@ override protected DbCommand CreateDbCommand() private static System.Security.CodeAccessPermission CreateExecutePermission() { - DBDataPermission p = (DBDataPermission)SqlConnectionFactory.SingletonInstance.ProviderFactory.CreatePermission(System.Security.Permissions.PermissionState.None); + DBDataPermission p = (DBDataPermission)SqlConnectionFactory.ProviderFactory.CreatePermission(System.Security.Permissions.PermissionState.None); p.Add(String.Empty, String.Empty, KeyRestrictionBehavior.AllowOnly); return p; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index ca5c2cb270..1b5a8cec48 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,8 +18,6 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - internal int ObjectID { get; } = Interlocked.Increment(ref _objectTypeCount); - public void ClearAllPools() { using (TryEventScope.Create(nameof(SqlConnectionFactory))) @@ -479,7 +477,7 @@ internal DbMetaDataFactory GetMetaDataFactory(DbConnectionPoolGroup connectionPo return metaDataFactory; } - private void PruneConnectionPoolGroups(object state) + protected void PruneConnectionPoolGroups(object state) { // when debugging this method, expect multiple threads at the same time SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}", ObjectID); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index a3b717e7c5..0c5dd010c8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -51,7 +51,7 @@ private SqlConnectionFactory() _poolsToRelease = new List(); _poolGroupsToRelease = new List(); _pruningTimer = ADP.UnsafeCreateTimer( - new TimerCallback(PruneConnectionPoolGroups), + PruneConnectionPoolGroups, state: null, PruningDueTime, PruningPeriod); @@ -62,11 +62,17 @@ private SqlConnectionFactory() } #endregion + + #region Properties - public static readonly SqlConnectionFactory SingletonInstance = new SqlConnectionFactory(); - - public DbProviderFactory ProviderFactory => SqlClientFactory.Instance; + internal static DbProviderFactory ProviderFactory => SqlClientFactory.Instance; + + internal static SqlConnectionFactory Instance { get; } = new SqlConnectionFactory(); + internal int ObjectId { get; } = Interlocked.Increment(ref s_objectTypeCount); + + #endregion + protected override DbConnectionInternal CreateConnection( DbConnectionOptions options, DbConnectionPoolKey poolKey, @@ -260,9 +266,9 @@ internal override DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupPro return new SqlConnectionPoolGroupProviderInfo((SqlConnectionString)connectionOptions); } - internal static SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKey key) + internal SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKey key) { - SqlConnectionString connectionOptions = (SqlConnectionString)SingletonInstance.FindConnectionOptions(key); + SqlConnectionString connectionOptions = (SqlConnectionString)FindConnectionOptions(key); if (connectionOptions == null) { connectionOptions = new SqlConnectionString(key.ConnectionString); From d49d488d0add6ffd76d3978ec6e1e8c681090ca5 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 13:55:34 -0500 Subject: [PATCH 05/20] Move pool clearing methods --- .../Data/ProviderBase/DbConnectionFactory.cs | 43 ++----------------- .../Data/SqlClient/SqlConnectionFactory.cs | 33 ++++++++++++++ 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 1b5a8cec48..901c4dba22 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,48 +18,11 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - public void ClearAllPools() - { - using (TryEventScope.Create(nameof(SqlConnectionFactory))) - { - Dictionary connectionPoolGroups = _connectionPoolGroups; - foreach (KeyValuePair entry in connectionPoolGroups) - { - DbConnectionPoolGroup poolGroup = entry.Value; - if (poolGroup != null) - { - poolGroup.Clear(); - } - } - } - } + - public void ClearPool(DbConnection connection) - { - ADP.CheckArgumentNull(connection, nameof(connection)); - using (TryEventScope.Create(" {0}", GetObjectId(connection))) - { - DbConnectionPoolGroup poolGroup = GetConnectionPoolGroup(connection); - if (poolGroup != null) - { - poolGroup.Clear(); - } - } - } + - public void ClearPool(DbConnectionPoolKey key) - { - Debug.Assert(key != null, "key cannot be null"); - ADP.CheckArgumentNull(key.ConnectionString, $"{nameof(key)}.{nameof(key.ConnectionString)}"); - using (TryEventScope.Create(" connectionString")) - { - Dictionary connectionPoolGroups = _connectionPoolGroups; - if (connectionPoolGroups.TryGetValue(key, out DbConnectionPoolGroup poolGroup)) - { - poolGroup.Clear(); - } - } - } + internal abstract DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo( DbConnectionOptions connectionOptions); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 0c5dd010c8..ceeeac09b2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -73,6 +73,39 @@ private SqlConnectionFactory() #endregion + #region Public Methods + + internal void ClearAllPools() + { + using TryEventScope scope = TryEventScope.Create(nameof(SqlConnectionFactory)); + foreach ((DbConnectionPoolKey _, DbConnectionPoolGroup group) in _connectionPoolGroups) + { + group?.Clear(); + } + } + + internal void ClearPool(DbConnection connection) + { + ADP.CheckArgumentNull(connection, nameof(connection)); + + using TryEventScope scope = TryEventScope.Create(" {0}", GetObjectId(connection)); + DbConnectionPoolGroup poolGroup = GetConnectionPoolGroup(connection); + poolGroup?.Clear(); + } + + internal void ClearPool(DbConnectionPoolKey key) + { + ADP.CheckArgumentNull(key.ConnectionString, $"{nameof(key)}.{nameof(key.ConnectionString)}"); + + using TryEventScope scope = TryEventScope.Create(" connectionString"); + if (_connectionPoolGroups.TryGetValue(key, out DbConnectionPoolGroup poolGroup)) + { + poolGroup?.Clear(); + } + } + + #endregion + protected override DbConnectionInternal CreateConnection( DbConnectionOptions options, DbConnectionPoolKey poolKey, From ca3e33d3ef1543b03e3751e1096f013e0eb6752b Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 14:16:02 -0500 Subject: [PATCH 06/20] Move Create*Connection methods over --- .../Data/ProviderBase/DbConnectionFactory.cs | 55 +--- .../WaitHandleDbConnectionPool.cs | 7 +- .../Data/SqlClient/SqlConnectionFactory.cs | 286 +++++++++++------- 3 files changed, 180 insertions(+), 168 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 901c4dba22..f882370a92 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,11 +18,8 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - - - - - + internal abstract DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( + DbConnectionOptions connectionOptions); internal abstract DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo( DbConnectionOptions connectionOptions); @@ -34,45 +31,7 @@ protected virtual DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal i cacheMetaDataFactory = false; throw ADP.NotSupported(); } - - internal DbConnectionInternal CreateNonPooledConnection(DbConnection owningConnection, DbConnectionPoolGroup poolGroup, DbConnectionOptions userOptions) - { - Debug.Assert(owningConnection != null, "null owningConnection?"); - Debug.Assert(poolGroup != null, "null poolGroup?"); - - DbConnectionOptions connectionOptions = poolGroup.ConnectionOptions; - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo = poolGroup.ProviderInfo; - DbConnectionPoolKey poolKey = poolGroup.PoolKey; - - DbConnectionInternal newConnection = CreateConnection(connectionOptions, poolKey, poolGroupProviderInfo, null, owningConnection, userOptions); - if (newConnection != null) - { - SqlClientEventSource.Metrics.HardConnectRequest(); - newConnection.MakeNonPooledObject(owningConnection); - } - SqlClientEventSource.Log.TryTraceEvent(" {0}, Non-pooled database connection created.", ObjectID); - return newConnection; - } - - internal DbConnectionInternal CreatePooledConnection(IDbConnectionPool pool, DbConnection owningObject, DbConnectionOptions options, DbConnectionPoolKey poolKey, DbConnectionOptions userOptions) - { - Debug.Assert(pool != null, "null pool?"); - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo = pool.PoolGroup.ProviderInfo; - DbConnectionInternal newConnection = CreateConnection(options, poolKey, poolGroupProviderInfo, pool, owningObject, userOptions); - - if (newConnection != null) - { - SqlClientEventSource.Metrics.HardConnectRequest(); - - newConnection.MakePooledConnection(pool); - } - SqlClientEventSource.Log.TryTraceEvent(" {0}, Pooled database connection created.", ObjectID); - return newConnection; - } - - internal abstract DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( - DbConnectionOptions connectionOptions); - + protected DbConnectionOptions FindConnectionOptions(DbConnectionPoolKey key) { Debug.Assert(key != null, "key cannot be null"); @@ -568,14 +527,6 @@ internal void QueuePoolGroupForRelease(DbConnectionPoolGroup poolGroup) SqlClientEventSource.Metrics.ExitActiveConnectionPoolGroup(); } - protected abstract DbConnectionInternal CreateConnection( - DbConnectionOptions options, - DbConnectionPoolKey poolKey, - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, - IDbConnectionPool pool, - DbConnection owningConnection, - DbConnectionOptions userOptions); - abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous); abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 6f359d981a..bd6ebf708e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -746,7 +746,12 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio try { - newObj = _connectionFactory.CreatePooledConnection(this, owningObject, _connectionPoolGroup.ConnectionOptions, _connectionPoolGroup.PoolKey, userOptions); + newObj = _connectionFactory.CreatePooledConnection( + owningObject, + this, + _connectionPoolGroup.PoolKey, + _connectionPoolGroup.ConnectionOptions, + userOptions); if (newObj == null) { throw ADP.InternalError(ADP.InternalErrorCode.CreateObjectReturnedNull); // CreateObject succeeded, but null object diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index ceeeac09b2..f5daf2950d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -103,131 +103,59 @@ internal void ClearPool(DbConnectionPoolKey key) poolGroup?.Clear(); } } - - #endregion - - protected override DbConnectionInternal CreateConnection( - DbConnectionOptions options, - DbConnectionPoolKey poolKey, - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, - IDbConnectionPool pool, + + internal SqlInternalConnectionTds CreateNonPooledConnection( DbConnection owningConnection, + DbConnectionPoolGroup poolGroup, DbConnectionOptions userOptions) { - SqlConnectionString opt = (SqlConnectionString)options; - SqlConnectionPoolKey key = (SqlConnectionPoolKey)poolKey; - SessionData recoverySessionData = null; - - SqlConnection sqlOwningConnection = owningConnection as SqlConnection; - bool applyTransientFaultHandling = sqlOwningConnection?._applyTransientFaultHandling ?? false; - - SqlConnectionString userOpt = null; - if (userOptions != null) - { - userOpt = (SqlConnectionString)userOptions; - } - else if (sqlOwningConnection != null) - { - userOpt = (SqlConnectionString)(sqlOwningConnection.UserConnectionOptions); - } - - if (sqlOwningConnection != null) + Debug.Assert(owningConnection is not null, "null owningConnection?"); + Debug.Assert(poolGroup is not null, "null poolGroup?"); + + SqlInternalConnectionTds newConnection = CreateConnection( + poolGroup.ConnectionOptions, + poolGroup.PoolKey, + poolGroup.ProviderInfo, + pool: null, + owningConnection, + userOptions); + if (newConnection is not null) { - recoverySessionData = sqlOwningConnection._recoverySessionData; + SqlClientEventSource.Metrics.HardConnectRequest(); + newConnection.MakeNonPooledObject(owningConnection); } + + SqlClientEventSource.Log.TryTraceEvent(" {0}, Non-pooled database connection created.", ObjectId); + return newConnection; + } - bool redirectedUserInstance = false; - DbConnectionPoolIdentity identity = null; + internal SqlInternalConnectionTds CreatePooledConnection( + DbConnection owningConnection, + IDbConnectionPool pool, + DbConnectionPoolKey poolKey, + DbConnectionOptions options, + DbConnectionOptions userOptions) + { + Debug.Assert(pool != null, "null pool?"); - // Pass DbConnectionPoolIdentity to SqlInternalConnectionTds if using integrated security - // or active directory integrated security. - // Used by notifications. - if (opt.IntegratedSecurity || opt.Authentication is SqlAuthenticationMethod.ActiveDirectoryIntegrated) + SqlInternalConnectionTds newConnection = CreateConnection( + options, + poolKey, // @TODO: is pool.PoolGroup.Key the same thing? + pool.PoolGroup.ProviderInfo, + pool, + owningConnection, + userOptions); + if (newConnection is not null) { - if (pool != null) - { - identity = pool.Identity; - } - else - { - identity = DbConnectionPoolIdentity.GetCurrent(); - } + SqlClientEventSource.Metrics.HardConnectRequest(); + newConnection.MakePooledConnection(pool); } - - // FOLLOWING IF BLOCK IS ENTIRELY FOR SSE USER INSTANCES - // If "user instance=true" is in the connection string, we're using SSE user instances - if (opt.UserInstance) - { - // opt.DataSource is used to create the SSE connection - redirectedUserInstance = true; - string instanceName; - - if (pool == null || (pool != null && pool.Count <= 0)) - { // Non-pooled or pooled and no connections in the pool. - SqlInternalConnectionTds sseConnection = null; - try - { - // We throw an exception in case of a failure - // NOTE: Cloning connection option opt to set 'UserInstance=True' and 'Enlist=False' - // This first connection is established to SqlExpress to get the instance name - // of the UserInstance. - SqlConnectionString sseopt = new SqlConnectionString(opt, opt.DataSource, userInstance: true, setEnlistValue: false); - sseConnection = new SqlInternalConnectionTds(identity, sseopt, key.Credential, null, "", null, false, applyTransientFaultHandling: applyTransientFaultHandling); - // NOTE: Retrieve here. This user instance name will be used below to connect to the Sql Express User Instance. - instanceName = sseConnection.InstanceName; - - // Set future transient fault handling based on connection options - sqlOwningConnection._applyTransientFaultHandling = opt != null && opt.ConnectRetryCount > 0; - - if (!instanceName.StartsWith("\\\\.\\", StringComparison.Ordinal)) - { - throw SQL.NonLocalSSEInstance(); - } - - if (pool != null) - { // Pooled connection - cache result - SqlConnectionPoolProviderInfo providerInfo = (SqlConnectionPoolProviderInfo)pool.ProviderInfo; - // No lock since we are already in creation mutex - providerInfo.InstanceName = instanceName; - } - } - finally - { - if (sseConnection != null) - { - sseConnection.Dispose(); - } - } - } - else - { // Cached info from pool. - SqlConnectionPoolProviderInfo providerInfo = (SqlConnectionPoolProviderInfo)pool.ProviderInfo; - // No lock since we are already in creation mutex - instanceName = providerInfo.InstanceName; - } - - // NOTE: Here connection option opt is cloned to set 'instanceName=' that was - // retrieved from the previous SSE connection. For this UserInstance connection 'Enlist=True'. - // options immutable - stored in global hash - don't modify - opt = new SqlConnectionString(opt, instanceName, userInstance: false, setEnlistValue: null); - poolGroupProviderInfo = null; // null so we do not pass to constructor below... - } - - return new SqlInternalConnectionTds( - identity, - opt, - key.Credential, - poolGroupProviderInfo, - newPassword: string.Empty, - newSecurePassword: null, - redirectedUserInstance, - userOpt, - recoverySessionData, - applyTransientFaultHandling, - key.AccessToken, - pool, - key.AccessTokenCallback); + + SqlClientEventSource.Log.TryTraceEvent(" {0}, Pooled database connection created.", ObjectId); + return newConnection; } + + #endregion protected override DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous) { @@ -404,6 +332,132 @@ protected override DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal internalConnection.ServerVersion); } + #region Private Methods + + // @TODO: I think this could be broken down into methods more specific to use cases above + private static SqlInternalConnectionTds CreateConnection( + DbConnectionOptions options, + DbConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, + IDbConnectionPool pool, + DbConnection owningConnection, + DbConnectionOptions userOptions) + { + SqlConnectionString opt = (SqlConnectionString)options; + SqlConnectionPoolKey key = (SqlConnectionPoolKey)poolKey; + SessionData recoverySessionData = null; + + SqlConnection sqlOwningConnection = owningConnection as SqlConnection; + bool applyTransientFaultHandling = sqlOwningConnection?._applyTransientFaultHandling ?? false; + + SqlConnectionString userOpt = null; + if (userOptions != null) + { + userOpt = (SqlConnectionString)userOptions; + } + else if (sqlOwningConnection != null) + { + userOpt = (SqlConnectionString)(sqlOwningConnection.UserConnectionOptions); + } + + if (sqlOwningConnection != null) + { + recoverySessionData = sqlOwningConnection._recoverySessionData; + } + + bool redirectedUserInstance = false; + DbConnectionPoolIdentity identity = null; + + // Pass DbConnectionPoolIdentity to SqlInternalConnectionTds if using integrated security + // or active directory integrated security. + // Used by notifications. + if (opt.IntegratedSecurity || opt.Authentication == SqlAuthenticationMethod.ActiveDirectoryIntegrated) + { + if (pool != null) + { + identity = pool.Identity; + } + else + { + identity = DbConnectionPoolIdentity.GetCurrent(); + } + } + + // FOLLOWING IF BLOCK IS ENTIRELY FOR SSE USER INSTANCES + // If "user instance=true" is in the connection string, we're using SSE user instances + if (opt.UserInstance) + { + // opt.DataSource is used to create the SSE connection + redirectedUserInstance = true; + string instanceName; + + if (pool == null || (pool != null && pool.Count <= 0)) + { // Non-pooled or pooled and no connections in the pool. + SqlInternalConnectionTds sseConnection = null; + try + { + // We throw an exception in case of a failure + // NOTE: Cloning connection option opt to set 'UserInstance=True' and 'Enlist=False' + // This first connection is established to SqlExpress to get the instance name + // of the UserInstance. + SqlConnectionString sseopt = new SqlConnectionString(opt, opt.DataSource, userInstance: true, setEnlistValue: false); + sseConnection = new SqlInternalConnectionTds(identity, sseopt, key.Credential, null, "", null, false, applyTransientFaultHandling: applyTransientFaultHandling); + // NOTE: Retrieve here. This user instance name will be used below to connect to the Sql Express User Instance. + instanceName = sseConnection.InstanceName; + + // Set future transient fault handling based on connection options + sqlOwningConnection._applyTransientFaultHandling = opt != null && opt.ConnectRetryCount > 0; + + if (!instanceName.StartsWith("\\\\.\\", StringComparison.Ordinal)) + { + throw SQL.NonLocalSSEInstance(); + } + + if (pool != null) + { // Pooled connection - cache result + SqlConnectionPoolProviderInfo providerInfo = (SqlConnectionPoolProviderInfo)pool.ProviderInfo; + // No lock since we are already in creation mutex + providerInfo.InstanceName = instanceName; + } + } + finally + { + if (sseConnection != null) + { + sseConnection.Dispose(); + } + } + } + else + { // Cached info from pool. + SqlConnectionPoolProviderInfo providerInfo = (SqlConnectionPoolProviderInfo)pool.ProviderInfo; + // No lock since we are already in creation mutex + instanceName = providerInfo.InstanceName; + } + + // NOTE: Here connection option opt is cloned to set 'instanceName=' that was + // retrieved from the previous SSE connection. For this UserInstance connection 'Enlist=True'. + // options immutable - stored in global hash - don't modify + opt = new SqlConnectionString(opt, instanceName, userInstance: false, setEnlistValue: null); + poolGroupProviderInfo = null; // null so we do not pass to constructor below... + } + + return new SqlInternalConnectionTds( + identity, + opt, + key.Credential, + poolGroupProviderInfo, + newPassword: string.Empty, + newSecurePassword: null, + redirectedUserInstance, + userOpt, + recoverySessionData, + applyTransientFaultHandling, + key.AccessToken, + pool, + key.AccessTokenCallback); + } + #if NET private void Unload(object sender, EventArgs e) { @@ -428,6 +482,8 @@ private void SubscribeToAssemblyLoadContextUnload() SqlConnectionFactoryAssemblyLoadContext_Unloading; } #endif + + #endregion } } From c43f371c8814b636aef430b551dd4dec6e758986 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:21:56 -0500 Subject: [PATCH 07/20] Move CreateConnectionPoolGroupProviderInfo --- .../Data/ProviderBase/DbConnectionFactory.cs | 8 +----- .../Data/SqlClient/SqlConnectionFactory.cs | 25 ++++++------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index f882370a92..16f948c041 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,12 +18,6 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - internal abstract DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( - DbConnectionOptions connectionOptions); - - internal abstract DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo( - DbConnectionOptions connectionOptions); - protected virtual DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal internalConnection, out bool cacheMetaDataFactory) { // providers that support GetSchema must override this with a method that creates a meta data @@ -46,7 +40,7 @@ protected DbConnectionOptions FindConnectionOptions(DbConnectionPoolKey key) } return null; } - + private static Task GetCompletedTask() { Debug.Assert(Monitor.IsEntered(s_pendingOpenNonPooled), $"Expected {nameof(s_pendingOpenNonPooled)} lock to be held."); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index f5daf2950d..adb72f83f7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -104,6 +104,11 @@ internal void ClearPool(DbConnectionPoolKey key) } } + internal DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo(DbConnectionOptions connectionOptions) => + ((SqlConnectionString)connectionOptions).UserInstance + ? new SqlConnectionPoolProviderInfo() + : null; + internal SqlInternalConnectionTds CreateNonPooledConnection( DbConnection owningConnection, DbConnectionPoolGroup poolGroup, @@ -164,18 +169,6 @@ protected override DbConnectionOptions CreateConnectionOptions(string connection return result; } - internal override DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo(DbConnectionOptions connectionOptions) - { - DbConnectionPoolProviderInfo providerInfo = null; - - if (((SqlConnectionString)connectionOptions).UserInstance) - { - providerInfo = new SqlConnectionPoolProviderInfo(); - } - - return providerInfo; - } - protected override DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions connectionOptions) { SqlConnectionString opt = (SqlConnectionString)connectionOptions; @@ -221,11 +214,9 @@ protected override DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions return poolingOptions; } - internal override DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( - DbConnectionOptions connectionOptions) - { - return new SqlConnectionPoolGroupProviderInfo((SqlConnectionString)connectionOptions); - } + internal DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( + DbConnectionOptions connectionOptions) => + new SqlConnectionPoolGroupProviderInfo((SqlConnectionString)connectionOptions); internal SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKey key) { From 5aa2d1389f53181397c9ea4d20cdeef66ca626c7 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:30:58 -0500 Subject: [PATCH 08/20] Move QueuePool*ForRelease --- .../Data/ProviderBase/DbConnectionFactory.cs | 38 ------------------- .../Data/SqlClient/SqlConnectionFactory.cs | 38 +++++++++++++++++++ 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 16f948c041..8f5fa25c9a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -483,44 +483,6 @@ protected void PruneConnectionPoolGroups(object state) } } - internal void QueuePoolForRelease(IDbConnectionPool pool, bool clearing) - { - // Queue the pool up for release -- we'll clear it out and dispose - // of it as the last part of the pruning timer callback so we don't - // do it with the pool entry or the pool collection locked. - Debug.Assert(pool != null, "null pool?"); - - // set the pool to the shutdown state to force all active - // connections to be automatically disposed when they - // are returned to the pool - pool.Shutdown(); - - lock (_poolsToRelease) - { - if (clearing) - { - pool.Clear(); - } - _poolsToRelease.Add(pool); - } - SqlClientEventSource.Metrics.EnterInactiveConnectionPool(); - SqlClientEventSource.Metrics.ExitActiveConnectionPool(); - } - - internal void QueuePoolGroupForRelease(DbConnectionPoolGroup poolGroup) - { - Debug.Assert(poolGroup != null, "null poolGroup?"); - SqlClientEventSource.Log.TryTraceEvent(" {0}, poolGroup={1}", ObjectID, poolGroup.ObjectID); - - lock (_poolGroupsToRelease) - { - _poolGroupsToRelease.Add(poolGroup); - } - - SqlClientEventSource.Metrics.EnterInactiveConnectionPoolGroup(); - SqlClientEventSource.Metrics.ExitActiveConnectionPoolGroup(); - } - abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous); abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index adb72f83f7..d3ed8f84f0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -159,6 +159,44 @@ internal SqlInternalConnectionTds CreatePooledConnection( SqlClientEventSource.Log.TryTraceEvent(" {0}, Pooled database connection created.", ObjectId); return newConnection; } + + internal void QueuePoolForRelease(IDbConnectionPool pool, bool clearing) + { + // Queue the pool up for release -- we'll clear it out and dispose of it as the last + // part of the pruning timer callback so we don't do it with the pool entry or the pool + // collection locked. + Debug.Assert(pool != null, "null pool?"); + + // Set the pool to the shutdown state to force all active connections to be + // automatically disposed when they are returned to the pool + pool.Shutdown(); + + lock (_poolsToRelease) + { + if (clearing) + { + pool.Clear(); + } + _poolsToRelease.Add(pool); + } + + SqlClientEventSource.Metrics.EnterInactiveConnectionPool(); + SqlClientEventSource.Metrics.ExitActiveConnectionPool(); + } + + internal void QueuePoolGroupForRelease(DbConnectionPoolGroup poolGroup) + { + Debug.Assert(poolGroup != null, "null poolGroup?"); + SqlClientEventSource.Log.TryTraceEvent(" {0}, poolGroup={1}", ObjectId, poolGroup.ObjectID); + + lock (_poolGroupsToRelease) + { + _poolGroupsToRelease.Add(poolGroup); + } + + SqlClientEventSource.Metrics.EnterInactiveConnectionPoolGroup(); + SqlClientEventSource.Metrics.ExitActiveConnectionPoolGroup(); + } #endregion From efc0c2826a8b5de3bf3084e9deb1e304870482d1 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:40:21 -0500 Subject: [PATCH 09/20] Move TryGetConnection --- .../Data/ProviderBase/DbConnectionFactory.cs | 147 +----------------- .../Data/SqlClient/SqlConnectionFactory.cs | 143 +++++++++++++++++ 2 files changed, 147 insertions(+), 143 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 8f5fa25c9a..5af7241c9b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -41,152 +41,13 @@ protected DbConnectionOptions FindConnectionOptions(DbConnectionPoolKey key) return null; } - private static Task GetCompletedTask() + protected static Task GetCompletedTask() { Debug.Assert(Monitor.IsEntered(s_pendingOpenNonPooled), $"Expected {nameof(s_pendingOpenNonPooled)} lock to be held."); return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - internal bool TryGetConnection(DbConnection owningConnection, TaskCompletionSource retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, out DbConnectionInternal connection) - { - Debug.Assert(owningConnection != null, "null owningConnection?"); - - DbConnectionPoolGroup poolGroup; - IDbConnectionPool connectionPool; - connection = null; - - // Work around race condition with clearing the pool between GetConnectionPool obtaining pool - // and GetConnection on the pool checking the pool state. Clearing the pool in this window - // will switch the pool into the ShuttingDown state, and GetConnection will return null. - // There is probably a better solution involving locking the pool/group, but that entails a major - // re-design of the connection pooling synchronization, so is postponed for now. - - // Use retriesLeft to prevent CPU spikes with incremental sleep - // start with one msec, double the time every retry - // max time is: 1 + 2 + 4 + ... + 2^(retries-1) == 2^retries -1 == 1023ms (for 10 retries) - int retriesLeft = 10; - int timeBetweenRetriesMilliseconds = 1; - - do - { - poolGroup = GetConnectionPoolGroup(owningConnection); - // Doing this on the callers thread is important because it looks up the WindowsIdentity from the thread. - connectionPool = GetConnectionPool(owningConnection, poolGroup); - if (connectionPool == null) - { - // If GetConnectionPool returns null, we can be certain that - // this connection should not be pooled via DbConnectionPool - // or have a disabled pool entry. - poolGroup = GetConnectionPoolGroup(owningConnection); // previous entry have been disabled - - if (retry != null) - { - Task newTask; - CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); - lock (s_pendingOpenNonPooled) - { - // look for an available task slot (completed or empty) - int idx; - for (idx = 0; idx < s_pendingOpenNonPooled.Length; idx++) - { - Task task = s_pendingOpenNonPooled[idx]; - if (task == null) - { - s_pendingOpenNonPooled[idx] = GetCompletedTask(); - break; - } - else if (task.IsCompleted) - { - break; - } - } - - // if didn't find one, pick the next one in round-robin fashion - if (idx == s_pendingOpenNonPooled.Length) - { - idx = (int)(s_pendingOpenNonPooledNext % s_pendingOpenNonPooled.Length); - unchecked - { - s_pendingOpenNonPooledNext++; - } - } - - // now that we have an antecedent task, schedule our work when it is completed. - // If it is a new slot or a completed task, this continuation will start right away. - newTask = CreateReplaceConnectionContinuation(s_pendingOpenNonPooled[idx], owningConnection, retry, userOptions, oldConnection, poolGroup, cancellationTokenSource); - - // Place this new task in the slot so any future work will be queued behind it - s_pendingOpenNonPooled[idx] = newTask; - } - - // Set up the timeout (if needed) - if (owningConnection.ConnectionTimeout > 0) - { - int connectionTimeoutMilliseconds = owningConnection.ConnectionTimeout * 1000; - cancellationTokenSource.CancelAfter(connectionTimeoutMilliseconds); - } - - // once the task is done, propagate the final results to the original caller - newTask.ContinueWith( - continuationAction: TryGetConnectionCompletedContinuation, - state: Tuple.Create(cancellationTokenSource, retry), - scheduler: TaskScheduler.Default - ); - - return false; - } - - connection = CreateNonPooledConnection(owningConnection, poolGroup, userOptions); - - SqlClientEventSource.Metrics.EnterNonPooledConnection(); - } - else - { - if (((SqlClient.SqlConnection)owningConnection).ForceNewConnection) - { - Debug.Assert(!(oldConnection is DbConnectionClosed), "Force new connection, but there is no old connection"); - connection = connectionPool.ReplaceConnection(owningConnection, userOptions, oldConnection); - } - else - { - if (!connectionPool.TryGetConnection(owningConnection, retry, userOptions, out connection)) - { - return false; - } - } - - if (connection == null) - { - // connection creation failed on semaphore waiting or if max pool reached - if (connectionPool.IsRunning) - { - SqlClientEventSource.Log.TryTraceEvent(" {0}, GetConnection failed because a pool timeout occurred.", ObjectID); - // If GetConnection failed while the pool is running, the pool timeout occurred. - throw ADP.PooledOpenTimeout(); - } - else - { - // We've hit the race condition, where the pool was shut down after we got it from the group. - // Yield time slice to allow shut down activities to complete and a new, running pool to be instantiated - // before retrying. - System.Threading.Thread.Sleep(timeBetweenRetriesMilliseconds); - timeBetweenRetriesMilliseconds *= 2; // double the wait time for next iteration - } - } - } - } while (connection == null && retriesLeft-- > 0); - - if (connection == null) - { - SqlClientEventSource.Log.TryTraceEvent(" {0}, GetConnection failed because a pool timeout occurred and all retries were exhausted.", ObjectID); - // exhausted all retries or timed out - give up - throw ADP.PooledOpenTimeout(); - } - - return true; - } - - private Task CreateReplaceConnectionContinuation(Task task, DbConnection owningConnection, TaskCompletionSource retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionPoolGroup poolGroup, CancellationTokenSource cancellationTokenSource) + protected Task CreateReplaceConnectionContinuation(Task task, DbConnection owningConnection, TaskCompletionSource retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionPoolGroup poolGroup, CancellationTokenSource cancellationTokenSource) { return task.ContinueWith( (_) => @@ -214,7 +75,7 @@ private Task CreateReplaceConnectionContinuation(Task task, object state) + protected void TryGetConnectionCompletedContinuation(Task task, object state) { Tuple> parameters = (Tuple>)state; CancellationTokenSource source = parameters.Item1; @@ -247,7 +108,7 @@ private void TryGetConnectionCompletedContinuation(Task ta } } - private IDbConnectionPool GetConnectionPool(DbConnection owningObject, DbConnectionPoolGroup connectionPoolGroup) + protected IDbConnectionPool GetConnectionPool(DbConnection owningObject, DbConnectionPoolGroup connectionPoolGroup) { // if poolgroup is disabled, it will be replaced with a new entry diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index d3ed8f84f0..f9f1295289 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -197,6 +197,149 @@ internal void QueuePoolGroupForRelease(DbConnectionPoolGroup poolGroup) SqlClientEventSource.Metrics.EnterInactiveConnectionPoolGroup(); SqlClientEventSource.Metrics.ExitActiveConnectionPoolGroup(); } + + internal bool TryGetConnection( + DbConnection owningConnection, + TaskCompletionSource retry, + DbConnectionOptions userOptions, + DbConnectionInternal oldConnection, + out DbConnectionInternal connection) + { + Debug.Assert(owningConnection is not null, "null owningConnection?"); + + connection = null; + + // Work around race condition with clearing the pool between GetConnectionPool obtaining pool + // and GetConnection on the pool checking the pool state. Clearing the pool in this window + // will switch the pool into the ShuttingDown state, and GetConnection will return null. + // There is probably a better solution involving locking the pool/group, but that entails a major + // re-design of the connection pooling synchronization, so is postponed for now. + + // Use retriesLeft to prevent CPU spikes with incremental sleep + // start with one msec, double the time every retry + // max time is: 1 + 2 + 4 + ... + 2^(retries-1) == 2^retries -1 == 1023ms (for 10 retries) + int retriesLeft = 10; + int timeBetweenRetriesMilliseconds = 1; + + do + { + DbConnectionPoolGroup poolGroup = GetConnectionPoolGroup(owningConnection); + + // Doing this on the callers thread is important because it looks up the WindowsIdentity from the thread. + IDbConnectionPool connectionPool = GetConnectionPool(owningConnection, poolGroup); + if (connectionPool == null) + { + // If GetConnectionPool returns null, we can be certain that this connection + // should not be pooled via DbConnectionPool or have a disabled pool entry. + poolGroup = GetConnectionPoolGroup(owningConnection); // previous entry have been disabled + + if (retry is not null) + { + Task newTask; + CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); + lock (s_pendingOpenNonPooled) + { + // look for an available task slot (completed or empty) + int idx; + for (idx = 0; idx < s_pendingOpenNonPooled.Length; idx++) + { + Task task = s_pendingOpenNonPooled[idx]; + if (task is null) + { + s_pendingOpenNonPooled[idx] = GetCompletedTask(); + break; + } + + if (task.IsCompleted) + { + break; + } + } + + // if didn't find one, pick the next one in round-robin fashion + if (idx == s_pendingOpenNonPooled.Length) + { + idx = (int)(s_pendingOpenNonPooledNext % s_pendingOpenNonPooled.Length); + unchecked + { + s_pendingOpenNonPooledNext++; + } + } + + // now that we have an antecedent task, schedule our work when it is completed. + // If it is a new slot or a completed task, this continuation will start right away. + newTask = CreateReplaceConnectionContinuation(s_pendingOpenNonPooled[idx], owningConnection, retry, userOptions, oldConnection, poolGroup, cancellationTokenSource); + + // Place this new task in the slot so any future work will be queued behind it + s_pendingOpenNonPooled[idx] = newTask; + } + + // Set up the timeout (if needed) + if (owningConnection.ConnectionTimeout > 0) + { + int connectionTimeoutMilliseconds = owningConnection.ConnectionTimeout * 1000; + cancellationTokenSource.CancelAfter(connectionTimeoutMilliseconds); + } + + // once the task is done, propagate the final results to the original caller + newTask.ContinueWith( + continuationAction: TryGetConnectionCompletedContinuation, + state: Tuple.Create(cancellationTokenSource, retry), + scheduler: TaskScheduler.Default + ); + + return false; + } + + connection = CreateNonPooledConnection(owningConnection, poolGroup, userOptions); + + SqlClientEventSource.Metrics.EnterNonPooledConnection(); + } + else + { + if (((SqlConnection)owningConnection).ForceNewConnection) + { + Debug.Assert(oldConnection is not DbConnectionClosed, "Force new connection, but there is no old connection"); + + connection = connectionPool.ReplaceConnection(owningConnection, userOptions, oldConnection); + } + else + { + if (!connectionPool.TryGetConnection(owningConnection, retry, userOptions, out connection)) + { + return false; + } + } + + if (connection is null) + { + // connection creation failed on semaphore waiting or if max pool reached + if (connectionPool.IsRunning) + { + SqlClientEventSource.Log.TryTraceEvent(" {0}, GetConnection failed because a pool timeout occurred.", ObjectId); + // If GetConnection failed while the pool is running, the pool timeout occurred. + throw ADP.PooledOpenTimeout(); + } + + // We've hit the race condition, where the pool was shut down after we + // got it from the group. Yield time slice to allow shutdown activities + // to complete and a new, running pool to be instantiated before + // retrying. + Thread.Sleep(timeBetweenRetriesMilliseconds); + timeBetweenRetriesMilliseconds *= 2; // double the wait time for next iteration + } + } + } while (connection == null && retriesLeft-- > 0); + + if (connection == null) + { + SqlClientEventSource.Log.TryTraceEvent(" {0}, GetConnection failed because a pool timeout occurred and all retries were exhausted.", ObjectId); + // exhausted all retries or timed out - give up + throw ADP.PooledOpenTimeout(); + } + + return true; + } #endregion From 396bab51b1880fae74132768f3476a9c4a7f1d11 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:45:22 -0500 Subject: [PATCH 10/20] Move CreateReplaceConnectionContinuation --- .../Data/ProviderBase/DbConnectionFactory.cs | 28 ------------- .../Data/SqlClient/SqlConnectionFactory.cs | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 5af7241c9b..d15c0d95d2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -47,34 +47,6 @@ protected static Task GetCompletedTask() return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - protected Task CreateReplaceConnectionContinuation(Task task, DbConnection owningConnection, TaskCompletionSource retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionPoolGroup poolGroup, CancellationTokenSource cancellationTokenSource) - { - return task.ContinueWith( - (_) => - { - System.Transactions.Transaction originalTransaction = ADP.GetCurrentTransaction(); - try - { - ADP.SetCurrentTransaction(retry.Task.AsyncState as System.Transactions.Transaction); - var newConnection = CreateNonPooledConnection(owningConnection, poolGroup, userOptions); - if ((oldConnection != null) && (oldConnection.State == ConnectionState.Open)) - { - oldConnection.PrepareForReplaceConnection(); - oldConnection.Dispose(); - } - return newConnection; - } - finally - { - ADP.SetCurrentTransaction(originalTransaction); - } - }, - cancellationTokenSource.Token, - TaskContinuationOptions.LongRunning, - TaskScheduler.Default - ); - } - protected void TryGetConnectionCompletedContinuation(Task task, object state) { Tuple> parameters = (Tuple>)state; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index f9f1295289..1ed54e8022 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Diagnostics; using System.IO; @@ -629,6 +630,44 @@ private static SqlInternalConnectionTds CreateConnection( pool, key.AccessTokenCallback); } + + private Task CreateReplaceConnectionContinuation( + Task task, + DbConnection owningConnection, + TaskCompletionSource retry, + DbConnectionOptions userOptions, + DbConnectionInternal oldConnection, + DbConnectionPoolGroup poolGroup, + CancellationTokenSource cancellationTokenSource) + { + return task.ContinueWith( + _ => + { + System.Transactions.Transaction originalTransaction = ADP.GetCurrentTransaction(); + try + { + ADP.SetCurrentTransaction(retry.Task.AsyncState as System.Transactions.Transaction); + + DbConnectionInternal newConnection = CreateNonPooledConnection(owningConnection, poolGroup, userOptions); + + if (oldConnection?.State == ConnectionState.Open) + { + oldConnection.PrepareForReplaceConnection(); + oldConnection.Dispose(); + } + + return newConnection; + } + finally + { + ADP.SetCurrentTransaction(originalTransaction); + } + }, + cancellationTokenSource.Token, + TaskContinuationOptions.LongRunning, + TaskScheduler.Default + ); + } #if NET private void Unload(object sender, EventArgs e) From e5d42d9908bf01a573f88e425772fa45131c9f7b Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:51:35 -0500 Subject: [PATCH 11/20] Move TryGetConnectionCompletedContinuation --- .../Data/ProviderBase/DbConnectionFactory.cs | 33 ------------------- .../Data/SqlClient/SqlConnectionFactory.cs | 33 +++++++++++++++++++ 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index d15c0d95d2..f9cecdc2c7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -47,39 +47,6 @@ protected static Task GetCompletedTask() return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - protected void TryGetConnectionCompletedContinuation(Task task, object state) - { - Tuple> parameters = (Tuple>)state; - CancellationTokenSource source = parameters.Item1; - source.Dispose(); - - TaskCompletionSource retryCompletionSource = parameters.Item2; - - if (task.IsCanceled) - { - retryCompletionSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.NonPooledOpenTimeout())); - } - else if (task.IsFaulted) - { - retryCompletionSource.TrySetException(task.Exception.InnerException); - } - else - { - if (!retryCompletionSource.TrySetResult(task.Result)) - { - // The outer TaskCompletionSource was already completed - // Which means that we don't know if someone has messed with the outer connection in the middle of creation - // So the best thing to do now is to destroy the newly created connection - task.Result.DoomThisConnection(); - task.Result.Dispose(); - } - else - { - SqlClientEventSource.Metrics.EnterNonPooledConnection(); - } - } - } - protected IDbConnectionPool GetConnectionPool(DbConnection owningObject, DbConnectionPoolGroup connectionPoolGroup) { // if poolgroup is disabled, it will be replaced with a new entry diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 1ed54e8022..205686d75c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -668,6 +668,39 @@ private Task CreateReplaceConnectionContinuation( TaskScheduler.Default ); } + + private void TryGetConnectionCompletedContinuation(Task task, object state) + { + // Decompose the state into the parameters we want + (CancellationTokenSource cts, TaskCompletionSource tcs) = + (Tuple>)state; + + cts.Dispose(); + + if (task.IsCanceled) + { + tcs.TrySetException(ADP.ExceptionWithStackTrace(ADP.NonPooledOpenTimeout())); + } + else if (task.IsFaulted) + { + tcs.TrySetException(task.Exception.InnerException); + } + else + { + if (!tcs.TrySetResult(task.Result)) + { + // The outer TaskCompletionSource was already completed + // Which means that we don't know if someone has messed with the outer connection in the middle of creation + // So the best thing to do now is to destroy the newly created connection + task.Result.DoomThisConnection(); + task.Result.Dispose(); + } + else + { + SqlClientEventSource.Metrics.EnterNonPooledConnection(); + } + } + } #if NET private void Unload(object sender, EventArgs e) From 25d580dd0d556015638f39adac702863fcfcfd5c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 16:55:54 -0500 Subject: [PATCH 12/20] Move GetConnectionPool --- .../Data/ProviderBase/DbConnectionFactory.cs | 35 ------------------ .../Data/SqlClient/SqlConnectionFactory.cs | 36 +++++++++++++++++++ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index f9cecdc2c7..eda97de219 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -47,41 +47,6 @@ protected static Task GetCompletedTask() return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - protected IDbConnectionPool GetConnectionPool(DbConnection owningObject, DbConnectionPoolGroup connectionPoolGroup) - { - // if poolgroup is disabled, it will be replaced with a new entry - - Debug.Assert(owningObject != null, "null owningObject?"); - Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup?"); - - // It is possible that while the outer connection object has - // been sitting around in a closed and unused state in some long - // running app, the pruner may have come along and remove this - // the pool entry from the master list. If we were to use a - // pool entry in this state, we would create "unmanaged" pools, - // which would be bad. To avoid this problem, we automagically - // re-create the pool entry whenever it's disabled. - - // however, don't rebuild connectionOptions if no pooling is involved - let new connections do that work - if (connectionPoolGroup.IsDisabled && connectionPoolGroup.PoolGroupOptions != null) - { - SqlClientEventSource.Log.TryTraceEvent(" {0}, DisabledPoolGroup={1}", ObjectID, connectionPoolGroup?.ObjectID); - - // reusing existing pool option in case user originally used SetConnectionPoolOptions - DbConnectionPoolGroupOptions poolOptions = connectionPoolGroup.PoolGroupOptions; - - // get the string to hash on again - DbConnectionOptions connectionOptions = connectionPoolGroup.ConnectionOptions; - Debug.Assert(connectionOptions != null, "prevent expansion of connectionString"); - - connectionPoolGroup = GetConnectionPoolGroup(connectionPoolGroup.PoolKey, poolOptions, ref connectionOptions); - Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup?"); - SetConnectionPoolGroup(owningObject, connectionPoolGroup); - } - IDbConnectionPool connectionPool = connectionPoolGroup.GetConnectionPool((SqlConnectionFactory)this); - return connectionPool; - } - internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnectionPoolKey key, DbConnectionPoolGroupOptions poolOptions, ref DbConnectionOptions userConnectionOptions) { if (string.IsNullOrEmpty(key.ConnectionString)) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 205686d75c..61c7d12786 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -669,6 +669,42 @@ private Task CreateReplaceConnectionContinuation( ); } + private IDbConnectionPool GetConnectionPool( + DbConnection owningObject, + DbConnectionPoolGroup connectionPoolGroup) + { + // If poolgroup is disabled, it will be replaced with a new entry + + Debug.Assert(owningObject != null, "null owningObject?"); + Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup?"); + + // It is possible that while the outer connection object has been sitting around in a + // closed and unused state in some long-running app, the pruner may have come along and + // remove this the pool entry from the master list. If we were to use a pool entry in + // this state, we would create "unmanaged" pools, which would be bad. To avoid this + // problem, we automagically re-create the pool entry whenever it's disabled. + + // however, don't rebuild connectionOptions if no pooling is involved - let new connections do that work + if (connectionPoolGroup.IsDisabled && connectionPoolGroup.PoolGroupOptions != null) + { + SqlClientEventSource.Log.TryTraceEvent(" {0}, DisabledPoolGroup={1}", ObjectId, connectionPoolGroup.ObjectID); + + // reusing existing pool option in case user originally used SetConnectionPoolOptions + DbConnectionPoolGroupOptions poolOptions = connectionPoolGroup.PoolGroupOptions; + + // get the string to hash on again + DbConnectionOptions connectionOptions = connectionPoolGroup.ConnectionOptions; + Debug.Assert(connectionOptions != null, "prevent expansion of connectionString"); + + connectionPoolGroup = GetConnectionPoolGroup(connectionPoolGroup.PoolKey, poolOptions, ref connectionOptions); + Debug.Assert(connectionPoolGroup != null, "null connectionPoolGroup?"); + SetConnectionPoolGroup(owningObject, connectionPoolGroup); + } + + IDbConnectionPool connectionPool = connectionPoolGroup.GetConnectionPool(this); + return connectionPool; + } + private void TryGetConnectionCompletedContinuation(Task task, object state) { // Decompose the state into the parameters we want From 111e42ba44e6ad836de4fa9c4652f7cfc78aee7b Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 17:02:38 -0500 Subject: [PATCH 13/20] Move GetConnectionPoolGroup --- .../Data/ProviderBase/DbConnectionFactory.cs | 89 ----------------- .../Data/SqlClient/SqlConnectionFactory.cs | 99 +++++++++++++++++++ 2 files changed, 99 insertions(+), 89 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index eda97de219..3df1e168b6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -47,95 +47,6 @@ protected static Task GetCompletedTask() return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnectionPoolKey key, DbConnectionPoolGroupOptions poolOptions, ref DbConnectionOptions userConnectionOptions) - { - if (string.IsNullOrEmpty(key.ConnectionString)) - { - return (DbConnectionPoolGroup)null; - } - - DbConnectionPoolGroup connectionPoolGroup; - Dictionary connectionPoolGroups = _connectionPoolGroups; - if (!connectionPoolGroups.TryGetValue(key, out connectionPoolGroup) || (connectionPoolGroup.IsDisabled && connectionPoolGroup.PoolGroupOptions != null)) - { - // If we can't find an entry for the connection string in - // our collection of pool entries, then we need to create a - // new pool entry and add it to our collection. - - DbConnectionOptions connectionOptions = CreateConnectionOptions(key.ConnectionString, userConnectionOptions); - if (connectionOptions == null) - { - throw ADP.InternalConnectionError(ADP.ConnectionError.ConnectionOptionsMissing); - } - - if (userConnectionOptions == null) - { - // we only allow one expansion on the connection string - - userConnectionOptions = connectionOptions; - string expandedConnectionString = connectionOptions.Expand(); - - // if the expanded string is same instance (default implementation), then use the already created options - if ((object)expandedConnectionString != (object)key.ConnectionString) - { - // CONSIDER: caching the original string to reduce future parsing - DbConnectionPoolKey newKey = (DbConnectionPoolKey)((ICloneable)key).Clone(); - newKey.ConnectionString = expandedConnectionString; - return GetConnectionPoolGroup(newKey, null, ref userConnectionOptions); - } - } - - if (poolOptions == null) - { - if (connectionPoolGroup != null) - { - // reusing existing pool option in case user originally used SetConnectionPoolOptions - poolOptions = connectionPoolGroup.PoolGroupOptions; - } - else - { - // Note: may return null for non-pooled connections - poolOptions = CreateConnectionPoolGroupOptions(connectionOptions); - } - } - - lock (this) - { - connectionPoolGroups = _connectionPoolGroups; - if (!connectionPoolGroups.TryGetValue(key, out connectionPoolGroup)) - { - DbConnectionPoolGroup newConnectionPoolGroup = new DbConnectionPoolGroup(connectionOptions, key, poolOptions); - newConnectionPoolGroup.ProviderInfo = CreateConnectionPoolGroupProviderInfo(connectionOptions); - - // build new dictionary with space for new connection string - Dictionary newConnectionPoolGroups = new Dictionary(1 + connectionPoolGroups.Count); - foreach (KeyValuePair entry in connectionPoolGroups) - { - newConnectionPoolGroups.Add(entry.Key, entry.Value); - } - - // lock prevents race condition with PruneConnectionPoolGroups - newConnectionPoolGroups.Add(key, newConnectionPoolGroup); - - SqlClientEventSource.Metrics.EnterActiveConnectionPoolGroup(); - connectionPoolGroup = newConnectionPoolGroup; - _connectionPoolGroups = newConnectionPoolGroups; - } - else - { - Debug.Assert(!connectionPoolGroup.IsDisabled, "Disabled pool entry discovered"); - } - } - Debug.Assert(connectionPoolGroup != null, "how did we not create a pool entry?"); - Debug.Assert(userConnectionOptions != null, "how did we not have user connection options?"); - } - else if (userConnectionOptions == null) - { - userConnectionOptions = connectionPoolGroup.ConnectionOptions; - } - return connectionPoolGroup; - } - internal DbMetaDataFactory GetMetaDataFactory(DbConnectionPoolGroup connectionPoolGroup, DbConnectionInternal internalConnection) { Debug.Assert(connectionPoolGroup != null, "connectionPoolGroup may not be null."); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 61c7d12786..229e96bc76 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -161,6 +161,103 @@ internal SqlInternalConnectionTds CreatePooledConnection( return newConnection; } + internal DbConnectionPoolGroup GetConnectionPoolGroup( + DbConnectionPoolKey key, + DbConnectionPoolGroupOptions poolOptions, + ref DbConnectionOptions userConnectionOptions) + { + if (string.IsNullOrEmpty(key.ConnectionString)) + { + return null; + } + + if (!_connectionPoolGroups.TryGetValue(key, out DbConnectionPoolGroup connectionPoolGroup) || + (connectionPoolGroup.IsDisabled && connectionPoolGroup.PoolGroupOptions != null)) + { + // If we can't find an entry for the connection string in + // our collection of pool entries, then we need to create a + // new pool entry and add it to our collection. + + DbConnectionOptions connectionOptions = CreateConnectionOptions( + key.ConnectionString, + userConnectionOptions); + if (connectionOptions is null) + { + throw ADP.InternalConnectionError(ADP.ConnectionError.ConnectionOptionsMissing); + } + + if (userConnectionOptions is null) + { + // We only allow one expansion on the connection string + userConnectionOptions = connectionOptions; + string expandedConnectionString = connectionOptions.Expand(); + + // if the expanded string is same instance (default implementation), then use the already created options + if ((object)expandedConnectionString != (object)key.ConnectionString) + { + // CONSIDER: caching the original string to reduce future parsing + DbConnectionPoolKey newKey = (DbConnectionPoolKey)key.Clone(); + newKey.ConnectionString = expandedConnectionString; + return GetConnectionPoolGroup(newKey, null, ref userConnectionOptions); + } + } + + if (poolOptions is null) + { + if (connectionPoolGroup is not null) + { + // reusing existing pool option in case user originally used SetConnectionPoolOptions + poolOptions = connectionPoolGroup.PoolGroupOptions; + } + else + { + // Note: may return null for non-pooled connections + poolOptions = CreateConnectionPoolGroupOptions(connectionOptions); + } + } + + lock (this) + { + if (!_connectionPoolGroups.TryGetValue(key, out connectionPoolGroup)) + { + DbConnectionPoolGroup newConnectionPoolGroup = + new DbConnectionPoolGroup(connectionOptions, key, poolOptions) + { + ProviderInfo = CreateConnectionPoolGroupProviderInfo(connectionOptions) + }; + + // build new dictionary with space for new connection string + Dictionary newConnectionPoolGroups = + new Dictionary(1 + _connectionPoolGroups.Count); + foreach (KeyValuePair entry in _connectionPoolGroups) + { + newConnectionPoolGroups.Add(entry.Key, entry.Value); + } + + // lock prevents race condition with PruneConnectionPoolGroups + newConnectionPoolGroups.Add(key, newConnectionPoolGroup); + + SqlClientEventSource.Metrics.EnterActiveConnectionPoolGroup(); + connectionPoolGroup = newConnectionPoolGroup; + _connectionPoolGroups = newConnectionPoolGroups; + } + else + { + Debug.Assert(!connectionPoolGroup.IsDisabled, "Disabled pool entry discovered"); + } + } + + Debug.Assert(connectionPoolGroup != null, "how did we not create a pool entry?"); + Debug.Assert(userConnectionOptions != null, "how did we not have user connection options?"); + } + else if (userConnectionOptions is null) + { + userConnectionOptions = connectionPoolGroup.ConnectionOptions; + } + + return connectionPoolGroup; + } + internal void QueuePoolForRelease(IDbConnectionPool pool, bool clearing) { // Queue the pool up for release -- we'll clear it out and dispose of it as the last @@ -705,6 +802,8 @@ private IDbConnectionPool GetConnectionPool( return connectionPool; } + + private void TryGetConnectionCompletedContinuation(Task task, object state) { // Decompose the state into the parameters we want From 3d12896691b273f00f58f2c09c8099f70db4389b Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 13 Jun 2025 17:10:31 -0500 Subject: [PATCH 14/20] Move GetMetaDataFactory and PruneConnectionPoolGroups # Conflicts: # src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs --- .../Microsoft/Data/SqlClient/SqlConnection.cs | 4 +- .../Data/ProviderBase/DbConnectionFactory.cs | 112 ----------------- .../Data/SqlClient/SqlConnectionFactory.cs | 114 +++++++++++++++++- 3 files changed, 114 insertions(+), 116 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs index 76bad9d82c..ff5efa0ef9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2197,7 +2197,7 @@ public static void ChangePassword(string connectionString, string newPassword) SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential: null, accessToken: null, accessTokenCallback: null); - SqlConnectionString connectionOptions = SqlConnectionFactory.FindSqlConnectionOptions(key); + SqlConnectionString connectionOptions = SqlConnectionFactory.Instance.FindSqlConnectionOptions(key); if (connectionOptions.IntegratedSecurity || connectionOptions.Authentication == SqlAuthenticationMethod.ActiveDirectoryIntegrated) { throw SQL.ChangePasswordConflictsWithSSPI(); @@ -2249,7 +2249,7 @@ public static void ChangePassword(string connectionString, SqlCredential credent SqlConnectionPoolKey key = new SqlConnectionPoolKey(connectionString, credential, accessToken: null, accessTokenCallback: null); - SqlConnectionString connectionOptions = SqlConnectionFactory.FindSqlConnectionOptions(key); + SqlConnectionString connectionOptions = SqlConnectionFactory.Instance.FindSqlConnectionOptions(key); // Check for connection string values incompatible with SqlCredential if (!string.IsNullOrEmpty(connectionOptions.UserID) || !string.IsNullOrEmpty(connectionOptions.Password)) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 3df1e168b6..889f3da750 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -47,118 +47,6 @@ protected static Task GetCompletedTask() return s_completedTask ?? (s_completedTask = Task.FromResult(null)); } - internal DbMetaDataFactory GetMetaDataFactory(DbConnectionPoolGroup connectionPoolGroup, DbConnectionInternal internalConnection) - { - Debug.Assert(connectionPoolGroup != null, "connectionPoolGroup may not be null."); - - // get the matadatafactory from the pool entry. If it does not already have one - // create one and save it on the pool entry - DbMetaDataFactory metaDataFactory = connectionPoolGroup.MetaDataFactory; - - // consider serializing this so we don't construct multiple metadata factories - // if two threads happen to hit this at the same time. One will be GC'd - if (metaDataFactory == null) - { - bool allowCache = false; - metaDataFactory = CreateMetaDataFactory(internalConnection, out allowCache); - if (allowCache) - { - connectionPoolGroup.MetaDataFactory = metaDataFactory; - } - } - return metaDataFactory; - } - - protected void PruneConnectionPoolGroups(object state) - { - // when debugging this method, expect multiple threads at the same time - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}", ObjectID); - - // First, walk the pool release list and attempt to clear each - // pool, when the pool is finally empty, we dispose of it. If the - // pool isn't empty, it's because there are active connections or - // distributed transactions that need it. - lock (_poolsToRelease) - { - if (0 != _poolsToRelease.Count) - { - IDbConnectionPool[] poolsToRelease = _poolsToRelease.ToArray(); - foreach (IDbConnectionPool pool in poolsToRelease) - { - if (pool != null) - { - pool.Clear(); - - if (0 == pool.Count) - { - _poolsToRelease.Remove(pool); - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, ReleasePool={1}", ObjectID, pool.Id); - - SqlClientEventSource.Metrics.ExitInactiveConnectionPool(); - } - } - } - } - } - - // Next, walk the pool entry release list and dispose of each - // pool entry when it is finally empty. If the pool entry isn't - // empty, it's because there are active pools that need it. - lock (_poolGroupsToRelease) - { - if (0 != _poolGroupsToRelease.Count) - { - DbConnectionPoolGroup[] poolGroupsToRelease = _poolGroupsToRelease.ToArray(); - foreach (DbConnectionPoolGroup poolGroup in poolGroupsToRelease) - { - if (poolGroup != null) - { - int poolsLeft = poolGroup.Clear(); // may add entries to _poolsToRelease - - if (0 == poolsLeft) - { - _poolGroupsToRelease.Remove(poolGroup); - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, ReleasePoolGroup={1}", ObjectID, poolGroup.ObjectID); - - SqlClientEventSource.Metrics.ExitInactiveConnectionPoolGroup(); - } - } - } - } - } - - // Finally, we walk through the collection of connection pool entries - // and prune each one. This will cause any empty pools to be put - // into the release list. - lock (this) - { - Dictionary connectionPoolGroups = _connectionPoolGroups; - Dictionary newConnectionPoolGroups = new Dictionary(connectionPoolGroups.Count); - - foreach (KeyValuePair entry in connectionPoolGroups) - { - if (entry.Value != null) - { - Debug.Assert(!entry.Value.IsDisabled, "Disabled pool entry discovered"); - - // entries start active and go idle during prune if all pools are gone - // move idle entries from last prune pass to a queue for pending release - // otherwise process entry which may move it from active to idle - if (entry.Value.Prune()) - { - // may add entries to _poolsToRelease - QueuePoolGroupForRelease(entry.Value); - } - else - { - newConnectionPoolGroups.Add(entry.Key, entry.Value); - } - } - } - _connectionPoolGroups = newConnectionPoolGroups; - } - } - abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous); abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 229e96bc76..4e1425cec6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -257,6 +257,30 @@ internal DbConnectionPoolGroup GetConnectionPoolGroup( return connectionPoolGroup; } + + internal DbMetaDataFactory GetMetaDataFactory( + DbConnectionPoolGroup poolGroup, + DbConnectionInternal internalConnection) + { + Debug.Assert(poolGroup is not null, "connectionPoolGroup may not be null."); + + // Get the matadatafactory from the pool entry. If it does not already have one + // create one and save it on the pool entry + DbMetaDataFactory metaDataFactory = poolGroup.MetaDataFactory; + + // CONSIDER: serializing this so we don't construct multiple metadata factories + // if two threads happen to hit this at the same time. One will be GC'd + if (metaDataFactory is null) + { + metaDataFactory = CreateMetaDataFactory(internalConnection, out bool allowCache); + if (allowCache) + { + poolGroup.MetaDataFactory = metaDataFactory; + } + } + + return metaDataFactory; + } internal void QueuePoolForRelease(IDbConnectionPool pool, bool clearing) { @@ -801,8 +825,94 @@ private IDbConnectionPool GetConnectionPool( IDbConnectionPool connectionPool = connectionPoolGroup.GetConnectionPool(this); return connectionPool; } - - + + private void PruneConnectionPoolGroups(object state) + { + // When debugging this method, expect multiple threads at the same time + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}", ObjectId); + + // First, walk the pool release list and attempt to clear each pool, when the pool is + // finally empty, we dispose of it. If the pool isn't empty, it's because there are + // active connections or distributed transactions that need it. + lock (_poolsToRelease) + { + if (_poolsToRelease.Count != 0) + { + IDbConnectionPool[] poolsToRelease = _poolsToRelease.ToArray(); + foreach (IDbConnectionPool pool in poolsToRelease) + { + if (pool is not null) + { + pool.Clear(); + + if (pool.Count == 0) + { + _poolsToRelease.Remove(pool); + + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, ReleasePool={1}", ObjectId, pool.Id); + SqlClientEventSource.Metrics.ExitInactiveConnectionPool(); + } + } + } + } + } + + // Next, walk the pool entry release list and dispose of each pool entry when it is + // finally empty. If the pool entry isn't empty, it's because there are active pools + // that need it. + lock (_poolGroupsToRelease) + { + if (_poolGroupsToRelease.Count != 0) + { + DbConnectionPoolGroup[] poolGroupsToRelease = _poolGroupsToRelease.ToArray(); + foreach (DbConnectionPoolGroup poolGroup in poolGroupsToRelease) + { + if (poolGroup != null) + { + int poolsLeft = poolGroup.Clear(); // may add entries to _poolsToRelease + + if (poolsLeft == 0) + { + _poolGroupsToRelease.Remove(poolGroup); + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, ReleasePoolGroup={1}", ObjectId, poolGroup.ObjectID); + + SqlClientEventSource.Metrics.ExitInactiveConnectionPoolGroup(); + } + } + } + } + } + + // Finally, we walk through the collection of connection pool entries and prune each + // one. This will cause any empty pools to be put into the release list. + lock (this) + { + Dictionary connectionPoolGroups = _connectionPoolGroups; + Dictionary newConnectionPoolGroups = new Dictionary(connectionPoolGroups.Count); + + foreach (KeyValuePair entry in connectionPoolGroups) + { + if (entry.Value != null) + { + Debug.Assert(!entry.Value.IsDisabled, "Disabled pool entry discovered"); + + // entries start active and go idle during prune if all pools are gone + // move idle entries from last prune pass to a queue for pending release + // otherwise process entry which may move it from active to idle + if (entry.Value.Prune()) + { + // may add entries to _poolsToRelease + QueuePoolGroupForRelease(entry.Value); + } + else + { + newConnectionPoolGroups.Add(entry.Key, entry.Value); + } + } + } + _connectionPoolGroups = newConnectionPoolGroups; + } + } private void TryGetConnectionCompletedContinuation(Task task, object state) { From 2027c5d9ad59295e1db2049169981ac726266bf8 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Mon, 16 Jun 2025 17:59:07 -0500 Subject: [PATCH 15/20] Move CompletedTask and merge FindConnectionOptions (and cleanup CreateMetaDataFactory) --- .../Data/ProviderBase/DbConnectionFactory.cs | 34 --------- .../Data/SqlClient/SqlConnectionFactory.cs | 74 ++++++++++++------- 2 files changed, 47 insertions(+), 61 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs index 889f3da750..657bcfc0f8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs @@ -18,35 +18,6 @@ namespace Microsoft.Data.ProviderBase { internal abstract class DbConnectionFactory { - protected virtual DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal internalConnection, out bool cacheMetaDataFactory) - { - // providers that support GetSchema must override this with a method that creates a meta data - // factory appropriate for them. - cacheMetaDataFactory = false; - throw ADP.NotSupported(); - } - - protected DbConnectionOptions FindConnectionOptions(DbConnectionPoolKey key) - { - Debug.Assert(key != null, "key cannot be null"); - if (!string.IsNullOrEmpty(key.ConnectionString)) - { - DbConnectionPoolGroup connectionPoolGroup; - Dictionary connectionPoolGroups = _connectionPoolGroups; - if (connectionPoolGroups.TryGetValue(key, out connectionPoolGroup)) - { - return connectionPoolGroup.ConnectionOptions; - } - } - return null; - } - - protected static Task GetCompletedTask() - { - Debug.Assert(Monitor.IsEntered(s_pendingOpenNonPooled), $"Expected {nameof(s_pendingOpenNonPooled)} lock to be held."); - return s_completedTask ?? (s_completedTask = Task.FromResult(null)); - } - abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous); abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options); @@ -66,10 +37,5 @@ protected static Task GetCompletedTask() abstract internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from); abstract internal void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to); - - virtual internal void Unload() - { - _pruningTimer.Dispose(); - } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 4e1425cec6..4c9851a0af 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -28,13 +28,15 @@ internal sealed class SqlConnectionFactory : DbConnectionFactory private static readonly TimeSpan PruningDueTime = TimeSpan.FromMinutes(4); private static readonly TimeSpan PruningPeriod = TimeSpan.FromSeconds(30); + private static readonly Task CompletedTask = + Task.FromResult(null); // s_pendingOpenNonPooled is an array of tasks used to throttle creation of non-pooled // connections to a maximum of Environment.ProcessorCount at a time. - private static Task s_completedTask; - private static int s_objectTypeCount; - private static Task[] s_pendingOpenNonPooled = + private static readonly Task[] s_pendingOpenNonPooled = new Task[Environment.ProcessorCount]; + + private static int s_objectTypeCount; private static uint s_pendingOpenNonPooledNext = 0; private readonly List _poolGroupsToRelease; @@ -79,7 +81,7 @@ private SqlConnectionFactory() internal void ClearAllPools() { using TryEventScope scope = TryEventScope.Create(nameof(SqlConnectionFactory)); - foreach ((DbConnectionPoolKey _, DbConnectionPoolGroup group) in _connectionPoolGroups) + foreach (DbConnectionPoolGroup group in _connectionPoolGroups.Values) { group?.Clear(); } @@ -368,7 +370,7 @@ internal bool TryGetConnection( Task task = s_pendingOpenNonPooled[idx]; if (task is null) { - s_pendingOpenNonPooled[idx] = GetCompletedTask(); + s_pendingOpenNonPooled[idx] = CompletedTask; break; } @@ -390,7 +392,14 @@ internal bool TryGetConnection( // now that we have an antecedent task, schedule our work when it is completed. // If it is a new slot or a completed task, this continuation will start right away. - newTask = CreateReplaceConnectionContinuation(s_pendingOpenNonPooled[idx], owningConnection, retry, userOptions, oldConnection, poolGroup, cancellationTokenSource); + newTask = CreateReplaceConnectionContinuation( + s_pendingOpenNonPooled[idx], + owningConnection, + retry, + userOptions, + oldConnection, + poolGroup, + cancellationTokenSource); // Place this new task in the slot so any future work will be queued behind it s_pendingOpenNonPooled[idx] = newTask; @@ -523,16 +532,27 @@ internal DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo internal SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKey key) { - SqlConnectionString connectionOptions = (SqlConnectionString)FindConnectionOptions(key); - if (connectionOptions == null) + Debug.Assert(key is not null, "Key cannot be null"); + + DbConnectionOptions connectionOptions = null; + + if (!string.IsNullOrEmpty(key.ConnectionString) && + _connectionPoolGroups.TryGetValue(key, out DbConnectionPoolGroup poolGroup)) + { + connectionOptions = poolGroup.ConnectionOptions; + } + + if (connectionOptions is null) { connectionOptions = new SqlConnectionString(key.ConnectionString); } + if (connectionOptions.IsEmpty) { throw ADP.NoConnectionString(); } - return connectionOptions; + + return (SqlConnectionString)connectionOptions; } // @TODO: All these methods seem redundant ... shouldn't we always have a SqlConnection? @@ -612,20 +632,6 @@ internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnect } } - protected override DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal internalConnection, out bool cacheMetaDataFactory) - { - Debug.Assert(internalConnection != null, "internalConnection may not be null."); - - Stream xmlStream = System.Reflection.Assembly.GetExecutingAssembly().GetManifestResourceStream("Microsoft.Data.SqlClient.SqlMetaData.xml"); - cacheMetaDataFactory = true; - - Debug.Assert(xmlStream != null, nameof(xmlStream) + " may not be null."); - - return new SqlMetaDataFactory(xmlStream, - internalConnection.ServerVersion, - internalConnection.ServerVersion); - } - #region Private Methods // @TODO: I think this could be broken down into methods more specific to use cases above @@ -662,8 +668,7 @@ private static SqlInternalConnectionTds CreateConnection( bool redirectedUserInstance = false; DbConnectionPoolIdentity identity = null; - // Pass DbConnectionPoolIdentity to SqlInternalConnectionTds if using integrated security - // or active directory integrated security. + // Pass DbConnectionPoolIdentity to SqlInternalConnectionTds if using integrated security. // Used by notifications. if (opt.IntegratedSecurity || opt.Authentication == SqlAuthenticationMethod.ActiveDirectoryIntegrated) { @@ -752,6 +757,21 @@ private static SqlInternalConnectionTds CreateConnection( key.AccessTokenCallback); } + private static DbMetaDataFactory CreateMetaDataFactory( + DbConnectionInternal internalConnection, + out bool cacheMetaDataFactory) + { + Debug.Assert(internalConnection is not null, "internalConnection may not be null."); + + Stream xmlStream = Assembly.GetExecutingAssembly().GetManifestResourceStream("Microsoft.Data.SqlClient.SqlMetaData.xml"); + Debug.Assert(xmlStream is not null, $"{nameof(xmlStream)} may not be null."); + + cacheMetaDataFactory = true; + return new SqlMetaDataFactory(xmlStream, + internalConnection.ServerVersion, + internalConnection.ServerVersion); + } + private Task CreateReplaceConnectionContinuation( Task task, DbConnection owningConnection, @@ -884,7 +904,7 @@ private void PruneConnectionPoolGroups(object state) } // Finally, we walk through the collection of connection pool entries and prune each - // one. This will cause any empty pools to be put into the release list. + // one. This will cause any empty pools to be put into the release list. lock (this) { Dictionary connectionPoolGroups = _connectionPoolGroups; @@ -952,7 +972,7 @@ private void Unload(object sender, EventArgs e) { try { - Unload(); + _pruningTimer.Dispose(); } finally { From e35761762891ded712fcec0eb639bb729945290b Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Mon, 16 Jun 2025 18:16:43 -0500 Subject: [PATCH 16/20] Remove DbConnectionFactory --- .../src/Microsoft.Data.SqlClient.csproj | 3 - .../netfx/src/Microsoft.Data.SqlClient.csproj | 3 - .../Data/ProviderBase/DbConnectionFactory.cs | 41 ------ .../Data/SqlClient/SqlConnectionFactory.cs | 123 ++++++++---------- 4 files changed, 55 insertions(+), 115 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.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 89fbdaa759..64bea7d5fb 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -99,9 +99,6 @@ Microsoft\Data\ProviderBase\DbConnectionClosed.cs - - Microsoft\Data\ProviderBase\DbConnectionFactory.cs - Microsoft\Data\SqlClient\ConnectionPool\ChannelDbConnectionPool.cs 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 78466acbc2..3904bfa16c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -285,9 +285,6 @@ Microsoft\Data\ProviderBase\DbConnectionClosed.cs - - Microsoft\Data\ProviderBase\DbConnectionFactory.cs - Microsoft\Data\ProviderBase\DbConnectionInternal.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs deleted file mode 100644 index 657bcfc0f8..0000000000 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs +++ /dev/null @@ -1,41 +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.Collections.Generic; -using System.Data; -using System.Data.Common; -using System.Diagnostics; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Data.Common; -using Microsoft.Data.Common.ConnectionString; -using Microsoft.Data.SqlClient; -using Microsoft.Data.SqlClient.ConnectionPool; - -namespace Microsoft.Data.ProviderBase -{ - internal abstract class DbConnectionFactory - { - abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous); - - abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options); - - abstract internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection); - - abstract internal DbConnectionInternal GetInnerConnection(DbConnection connection); - - abstract protected int GetObjectId(DbConnection connection); - - abstract internal void PermissionDemand(DbConnection outerConnection); - - abstract internal void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup); - - abstract internal void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to); - - abstract internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from); - - abstract internal void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to); - } -} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 4c9851a0af..8d69de4d77 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -22,7 +22,7 @@ namespace Microsoft.Data.SqlClient { - internal sealed class SqlConnectionFactory : DbConnectionFactory + internal sealed class SqlConnectionFactory { #region Member Variables @@ -180,13 +180,7 @@ internal DbConnectionPoolGroup GetConnectionPoolGroup( // our collection of pool entries, then we need to create a // new pool entry and add it to our collection. - DbConnectionOptions connectionOptions = CreateConnectionOptions( - key.ConnectionString, - userConnectionOptions); - if (connectionOptions is null) - { - throw ADP.InternalConnectionError(ADP.ConnectionError.ConnectionOptionsMissing); - } + SqlConnectionString connectionOptions = new SqlConnectionString(key.ConnectionString); if (userConnectionOptions is null) { @@ -474,58 +468,6 @@ internal bool TryGetConnection( #endregion - protected override DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous) - { - Debug.Assert(!string.IsNullOrEmpty(connectionString), "empty connectionString"); - SqlConnectionString result = new SqlConnectionString(connectionString); - return result; - } - - protected override DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions connectionOptions) - { - SqlConnectionString opt = (SqlConnectionString)connectionOptions; - - DbConnectionPoolGroupOptions poolingOptions = null; - - if (opt.Pooling) - { // never pool context connections. - int connectionTimeout = opt.ConnectTimeout; - - if (connectionTimeout > 0 && connectionTimeout < int.MaxValue / 1000) - { - connectionTimeout *= 1000; - } - else if (connectionTimeout >= int.MaxValue / 1000) - { - connectionTimeout = int.MaxValue; - } - - if (opt.Authentication is SqlAuthenticationMethod.ActiveDirectoryInteractive - or SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow) - { - // interactive/device code flow mode will always have pool's CreateTimeout = 10 x ConnectTimeout. - if (connectionTimeout >= Int32.MaxValue / 10) - { - connectionTimeout = Int32.MaxValue; - } - else - { - connectionTimeout *= 10; - } - SqlClientEventSource.Log.TryTraceEvent("SqlConnectionFactory.CreateConnectionPoolGroupOptions | Set connection pool CreateTimeout '{0}' when Authentication mode '{1}' is used.", connectionTimeout, opt.Authentication); - } - - poolingOptions = new DbConnectionPoolGroupOptions( - opt.IntegratedSecurity || opt.Authentication is SqlAuthenticationMethod.ActiveDirectoryIntegrated, - opt.MinPoolSize, - opt.MaxPoolSize, - connectionTimeout, - opt.LoadBalanceTimeout, - opt.Enlist); - } - return poolingOptions; - } - internal DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo( DbConnectionOptions connectionOptions) => new SqlConnectionPoolGroupProviderInfo((SqlConnectionString)connectionOptions); @@ -556,7 +498,7 @@ internal SqlConnectionString FindSqlConnectionOptions(SqlConnectionPoolKey key) } // @TODO: All these methods seem redundant ... shouldn't we always have a SqlConnection? - internal override DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection) + internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -566,7 +508,7 @@ internal override DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection conn return null; } - internal override DbConnectionInternal GetInnerConnection(DbConnection connection) + internal DbConnectionInternal GetInnerConnection(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -576,7 +518,7 @@ internal override DbConnectionInternal GetInnerConnection(DbConnection connectio return null; } - protected override int GetObjectId(DbConnection connection) + internal int GetObjectId(DbConnection connection) { SqlConnection c = (connection as SqlConnection); if (c != null) @@ -586,7 +528,7 @@ protected override int GetObjectId(DbConnection connection) return 0; } - internal override void PermissionDemand(DbConnection outerConnection) + internal void PermissionDemand(DbConnection outerConnection) { SqlConnection c = (outerConnection as SqlConnection); if (c != null) @@ -595,7 +537,7 @@ internal override void PermissionDemand(DbConnection outerConnection) } } - internal override void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup) + internal void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup) { SqlConnection c = (outerConnection as SqlConnection); if (c != null) @@ -604,7 +546,7 @@ internal override void SetConnectionPoolGroup(DbConnection outerConnection, DbCo } } - internal override void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to) + internal void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to) { SqlConnection c = (owningObject as SqlConnection); if (c != null) @@ -613,7 +555,7 @@ internal override void SetInnerConnectionEvent(DbConnection owningObject, DbConn } } - internal override bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from) + internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from) { SqlConnection c = (owningObject as SqlConnection); if (c != null) @@ -623,7 +565,7 @@ internal override bool SetInnerConnectionFrom(DbConnection owningObject, DbConne return false; } - internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to) + internal void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to) { SqlConnection c = (owningObject as SqlConnection); if (c != null) @@ -757,6 +699,51 @@ private static SqlInternalConnectionTds CreateConnection( key.AccessTokenCallback); } + private static DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(SqlConnectionString connectionOptions) + { + SqlConnectionString opt = (SqlConnectionString)connectionOptions; + + DbConnectionPoolGroupOptions poolingOptions = null; + + if (opt.Pooling) + { // never pool context connections. + int connectionTimeout = opt.ConnectTimeout; + + if (connectionTimeout > 0 && connectionTimeout < int.MaxValue / 1000) + { + connectionTimeout *= 1000; + } + else if (connectionTimeout >= int.MaxValue / 1000) + { + connectionTimeout = int.MaxValue; + } + + if (opt.Authentication is SqlAuthenticationMethod.ActiveDirectoryInteractive + or SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow) + { + // interactive/device code flow mode will always have pool's CreateTimeout = 10 x ConnectTimeout. + if (connectionTimeout >= Int32.MaxValue / 10) + { + connectionTimeout = Int32.MaxValue; + } + else + { + connectionTimeout *= 10; + } + SqlClientEventSource.Log.TryTraceEvent("SqlConnectionFactory.CreateConnectionPoolGroupOptions | Set connection pool CreateTimeout '{0}' when Authentication mode '{1}' is used.", connectionTimeout, opt.Authentication); + } + + poolingOptions = new DbConnectionPoolGroupOptions( + opt.IntegratedSecurity || opt.Authentication is SqlAuthenticationMethod.ActiveDirectoryIntegrated, + opt.MinPoolSize, + opt.MaxPoolSize, + connectionTimeout, + opt.LoadBalanceTimeout, + opt.Enlist); + } + return poolingOptions; + } + private static DbMetaDataFactory CreateMetaDataFactory( DbConnectionInternal internalConnection, out bool cacheMetaDataFactory) From f67a6086275caa3e926f8e060d6404486fbd8afd Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 25 Jun 2025 12:58:21 -0500 Subject: [PATCH 17/20] Make timer period times based on milliseconds instead of seconds --- .../src/Microsoft/Data/Common/AdapterUtil.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index cebd223a1c..b59e5cb33d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -130,8 +130,16 @@ internal static Exception ExceptionWithStackTrace(Exception e) } } - internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) => - UnsafeCreateTimer(callback, state, TimeSpan.FromSeconds(dueTime), TimeSpan.FromSeconds(period)); + internal static Timer UnsafeCreateTimer( + TimerCallback callback, + object state, + int dueTimeMilliseconds, + int periodMilliseconds) => + UnsafeCreateTimer( + callback, + state, + TimeSpan.FromMilliseconds(dueTimeMilliseconds), + TimeSpan.FromMilliseconds(periodMilliseconds)); internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period) { From ff21328129f2b1c42ad5daf6da9b709c60b35eff Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 25 Jun 2025 15:52:46 -0500 Subject: [PATCH 18/20] Fix reflection references to Db/SqlConnectionFactory --- .../SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs index 6c828b188b..34b9070946 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs @@ -18,7 +18,6 @@ internal static class ConnectionPoolHelper private static Type s_waitHandleDbConnectionPool = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.WaitHandleDbConnectionPool"); private static Type s_dbConnectionPoolGroup = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolGroup"); private static Type s_dbConnectionPoolIdentity = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolIdentity"); - private static Type s_dbConnectionFactory = s_MicrosoftDotData.GetType("Microsoft.Data.ProviderBase.DbConnectionFactory"); private static Type s_sqlConnectionFactory = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.SqlConnectionFactory"); private static Type s_dbConnectionPoolKey = s_MicrosoftDotData.GetType("Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolKey"); private static Type s_dictStringPoolGroup = typeof(Dictionary<,>).MakeGenericType(s_dbConnectionPoolKey, s_dbConnectionPoolGroup); @@ -26,9 +25,9 @@ internal static class ConnectionPoolHelper private static PropertyInfo s_dbConnectionPoolCount = s_waitHandleDbConnectionPool.GetProperty("Count", BindingFlags.Instance | BindingFlags.Public); private static PropertyInfo s_dictStringPoolGroupGetKeys = s_dictStringPoolGroup.GetProperty("Keys"); private static PropertyInfo s_dictPoolIdentityPoolValues = s_dictPoolIdentityPool.GetProperty("Values"); - private static FieldInfo s_dbConnectionFactoryPoolGroupList = s_dbConnectionFactory.GetField("_connectionPoolGroups", BindingFlags.Instance | BindingFlags.NonPublic); + private static PropertyInfo s_sqlConnectionFactorySingleton = s_sqlConnectionFactory.GetProperty("Instance", BindingFlags.Static | BindingFlags.NonPublic); + private static FieldInfo s_dbConnectionFactoryPoolGroupList = s_sqlConnectionFactory.GetField("_connectionPoolGroups", BindingFlags.Instance | BindingFlags.NonPublic); private static FieldInfo s_dbConnectionPoolGroupPoolCollection = s_dbConnectionPoolGroup.GetField("_poolCollection", BindingFlags.Instance | BindingFlags.NonPublic); - private static FieldInfo s_sqlConnectionFactorySingleton = s_sqlConnectionFactory.GetField("SingletonInstance", BindingFlags.Static | BindingFlags.Public); private static FieldInfo s_dbConnectionPoolStackOld = s_waitHandleDbConnectionPool.GetField("_stackOld", BindingFlags.Instance | BindingFlags.NonPublic); private static FieldInfo s_dbConnectionPoolStackNew = s_waitHandleDbConnectionPool.GetField("_stackNew", BindingFlags.Instance | BindingFlags.NonPublic); private static MethodInfo s_dbConnectionPoolCleanup = s_waitHandleDbConnectionPool.GetMethod("CleanupCallback", BindingFlags.Instance | BindingFlags.NonPublic); From 36ee48cff51588e7595cc2082be7a23928327a3d Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 26 Jun 2025 17:30:24 -0500 Subject: [PATCH 19/20] Addressing called out expression bodies --- .../Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | 6 ++++-- .../Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | 6 ++++-- .../src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index 4c7cd8bf7d..8e3cb1bef3 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -3053,8 +3053,10 @@ internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, TaskCompletionSource retry, - DbConnectionOptions userOptions) => - TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + DbConnectionOptions userOptions) + { + return TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + } } internal sealed class ServerInfo 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 9d55be1dd5..391c204c60 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 @@ -3085,8 +3085,10 @@ internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, TaskCompletionSource retry, - DbConnectionOptions userOptions) => - base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + DbConnectionOptions userOptions) + { + return TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions); + } } internal sealed class ServerInfo diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs index 0c2cf99ce7..8549c48a8d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -43,7 +43,9 @@ protected internal override DataTable GetSchema( DbConnection outerConnection, string collectionName, string[] restrictions) - => throw ADP.ClosedConnectionError(); + { + throw ADP.ClosedConnectionError(); + } protected override DbReferenceCollection CreateReferenceCollection() => throw ADP.ClosedConnectionError(); From 2894c10d45b57dd751b6b27ba751b655216d0811 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 8 Jul 2025 13:52:00 -0500 Subject: [PATCH 20/20] Address @mdaigle's comments --- .../src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs | 4 +--- .../src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs | 7 +++---- .../src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index 0888b400d0..9095c9b454 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -156,9 +156,7 @@ override protected DbCommand CreateDbCommand() { using (TryEventScope.Create(" {0}", ObjectID)) { - DbCommand command = null; - DbProviderFactory providerFactory = SqlConnectionFactory.ProviderFactory; - command = providerFactory.CreateCommand(); + DbCommand command = SqlClientFactory.Instance.CreateCommand(); command.Connection = this; return command; } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs index 3e2204b683..68c7d24ea2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs @@ -203,8 +203,7 @@ override protected DbCommand CreateDbCommand() { using (TryEventScope.Create(" {0}", ObjectID)) { - DbProviderFactory providerFactory = SqlConnectionFactory.ProviderFactory; - DbCommand command = providerFactory.CreateCommand(); + DbCommand command = SqlClientFactory.Instance.CreateCommand(); command.Connection = this; return command; } @@ -212,8 +211,8 @@ override protected DbCommand CreateDbCommand() private static System.Security.CodeAccessPermission CreateExecutePermission() { - DBDataPermission p = (DBDataPermission)SqlConnectionFactory.ProviderFactory.CreatePermission(System.Security.Permissions.PermissionState.None); - p.Add(String.Empty, String.Empty, KeyRestrictionBehavior.AllowOnly); + DBDataPermission p = (DBDataPermission)SqlClientFactory.Instance.CreatePermission(System.Security.Permissions.PermissionState.None); + p.Add(string.Empty, string.Empty, KeyRestrictionBehavior.AllowOnly); return p; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 8d69de4d77..fd2633a4b9 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -22,6 +22,7 @@ namespace Microsoft.Data.SqlClient { + // @TODO: Facade pattern (interface, use interface, add constructor overloads for providing non-default interface, reseal) internal sealed class SqlConnectionFactory { #region Member Variables @@ -67,8 +68,6 @@ private SqlConnectionFactory() #endregion #region Properties - - internal static DbProviderFactory ProviderFactory => SqlClientFactory.Instance; internal static SqlConnectionFactory Instance { get; } = new SqlConnectionFactory();