Skip to content

Cleanup | Split out TransactedConnectionPool #3746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mdaigle merged 11 commits into from
Nov 7, 2025
Merged

Cleanup | Split out TransactedConnectionPool #3746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fdfea8b
Move TransactedConnectionPool to its own class.
mdaigle Nov 5, 2025
265541a
Enable nullable.
mdaigle Nov 5, 2025
4a63eb4
Clean up properties.
mdaigle Nov 5, 2025
f6b8bcd
Add doc comments.
mdaigle Nov 5, 2025
4f38782
Minor cleanup. Add regions.
mdaigle Nov 5, 2025
edd7b42
Sync connection pool unit test namespace and folder structure.
mdaigle Nov 5, 2025
dcf1f6d
Add unit tests.
mdaigle Nov 6, 2025
09f7e76
Address copilot comments.
mdaigle Nov 6, 2025
92d31de
Address edwardneal's comments.
mdaigle Nov 6, 2025
8c8c3f4
Address paul's comments.
mdaigle Nov 6, 2025
a52142b
Clean up test variable names. Use ConcurrentBag in tests to avoid con…
mdaigle Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\SqlConnectionPoolProviderInfo.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\SqlConnectionPoolProviderInfo.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\TransactedConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\TransactedConnectionPool.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\WaitHandleDbConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\WaitHandleDbConnectionPool.cs</Link>
</Compile>
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\SqlConnectionPoolProviderInfo.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\SqlConnectionPoolProviderInfo.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\TransactedConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\TransactedConnectionPool.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\WaitHandleDbConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\WaitHandleDbConnectionPool.cs</Link>
</Compile>
Expand Down
353 changes: 353 additions & 0 deletions ...ft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/TransactedConnectionPool.cs
View file
Open in desktop

Large diffs are not rendered by default.

260 changes: 0 additions & 260 deletions ....Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,6 @@ internal sealed class WaitHandleDbConnectionPool : IDbConnectionPool
// This class is a way to stash our cloned Tx key for later disposal when it's no longer needed.
// We can't get at the key in the dictionary without enumerating entries, so we stash an extra
// copy as part of the value.
private sealed class TransactedConnectionList : List<DbConnectionInternal>
{
private Transaction _transaction;
internal TransactedConnectionList(int initialAllocation, Transaction tx) : base(initialAllocation)
{
_transaction = tx;
}

internal void Dispose()
{
if (_transaction != null)
{
_transaction.Dispose();
}
}
}

private sealed class PendingGetConnection
{
Expand All @@ -91,250 +75,6 @@ public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSour
public DbConnectionOptions UserOptions { get; private set; }
}

private sealed class TransactedConnectionPool
{
Dictionary<Transaction, TransactedConnectionList> _transactedCxns;

IDbConnectionPool _pool;

private static int _objectTypeCount; // EventSource Counter
internal readonly int _objectID = System.Threading.Interlocked.Increment(ref _objectTypeCount);

internal TransactedConnectionPool(IDbConnectionPool pool)
{
Debug.Assert(pool != null, "null pool?");

_pool = pool;
_transactedCxns = new Dictionary<Transaction, TransactedConnectionList>();
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactedConnectionPool|RES|CPOOL> {0}, Constructed for connection pool {1}", ObjectID, _pool.Id);
}

internal int ObjectID
{
get
{
return _objectID;
}
}

internal IDbConnectionPool Pool
{
get
{
return _pool;
}
}

internal DbConnectionInternal GetTransactedObject(Transaction transaction)
{
Debug.Assert(transaction != null, "null transaction?");

DbConnectionInternal transactedObject = null;

TransactedConnectionList connections;
bool txnFound = false;

lock (_transactedCxns)
{
txnFound = _transactedCxns.TryGetValue(transaction, out connections);
}

// NOTE: GetTransactedObject is only used when AutoEnlist = True and the ambient transaction
// (Sys.Txns.Txn.Current) is still valid/non-null. This, in turn, means that we don't need
// to worry about a pending asynchronous TransactionCompletedEvent to trigger processing in
// TransactionEnded below and potentially wipe out the connections list underneath us. It
// is similarly alright if a pending addition to the connections list in PutTransactedObject
// below is not completed prior to the lock on the connections object here...getting a new
// connection is probably better than unnecessarily locking
if (txnFound)
{
Debug.Assert(connections != null);

// synchronize multi-threaded access with PutTransactedObject (TransactionEnded should
// not be a concern, see comments above)
lock (connections)
{
int i = connections.Count - 1;
if (0 <= i)
{
transactedObject = connections[i];
connections.RemoveAt(i);
}
}
}

if (transactedObject != null)
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.GetTransactedObject|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Popped.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
}
return transactedObject;
}

internal void PutTransactedObject(Transaction transaction, DbConnectionInternal transactedObject)
{
Debug.Assert(transaction != null, "null transaction?");
Debug.Assert(transactedObject != null, "null transactedObject?");

TransactedConnectionList connections;
bool txnFound = false;

// NOTE: because TransactionEnded is an asynchronous notification, there's no guarantee
// around the order in which PutTransactionObject and TransactionEnded are called.

lock (_transactedCxns)
{
// Check if a transacted pool has been created for this transaction
if (txnFound = _transactedCxns.TryGetValue(transaction, out connections))
{
Debug.Assert(connections != null);

// synchronize multi-threaded access with GetTransactedObject
lock (connections)
{
Debug.Assert(0 > connections.IndexOf(transactedObject), "adding to pool a second time?");
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.PutTransactedObject|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Pushing.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
connections.Add(transactedObject);
}
}
}

// CONSIDER: the following code is more complicated than it needs to be to avoid cloning the
// transaction and allocating memory within a lock. Is that complexity really necessary?
if (!txnFound)
{
// create the transacted pool, making sure to clone the associated transaction
// for use as a key in our internal dictionary of transactions and connections
Transaction transactionClone = null;
TransactedConnectionList newConnections = null;

try
{
transactionClone = transaction.Clone();
newConnections = new TransactedConnectionList(2, transactionClone); // start with only two connections in the list; most times we won't need that many.

lock (_transactedCxns)
{
// NOTE: in the interim between the locks on the transacted pool (this) during
// execution of this method, another thread (threadB) may have attempted to
// add a different connection to the transacted pool under the same
// transaction. As a result, threadB may have completed creating the
// transacted pool while threadA was processing the above instructions.
if (txnFound = _transactedCxns.TryGetValue(transaction, out connections))
{
Debug.Assert(connections != null);

// synchronize multi-threaded access with GetTransactedObject
lock (connections)
{
Debug.Assert(0 > connections.IndexOf(transactedObject), "adding to pool a second time?");
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.PutTransactedObject|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Pushing.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
connections.Add(transactedObject);
}
}
else
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.PutTransactedObject|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Adding List to transacted pool.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);

// add the connection/transacted object to the list
newConnections.Add(transactedObject);

_transactedCxns.Add(transactionClone, newConnections);
transactionClone = null; // we've used it -- don't throw it or the TransactedConnectionList that references it away.
}
}
}
finally
{
if (transactionClone != null)
{
if (newConnections != null)
{
// another thread created the transaction pool and thus the new
// TransactedConnectionList was not used, so dispose of it and
// the transaction clone that it incorporates.
newConnections.Dispose();
}
else
{
// memory allocation for newConnections failed...clean up unused transactionClone
transactionClone.Dispose();
}
}
}
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.PutTransactedObject|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Added.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
}

SqlClientEventSource.Metrics.EnterFreeConnection();
}

internal void TransactionEnded(Transaction transaction, DbConnectionInternal transactedObject)
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactionEnded|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Transaction Completed", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
TransactedConnectionList connections;
int entry = -1;

// NOTE: because TransactionEnded is an asynchronous notification, there's no guarantee
// around the order in which PutTransactionObject and TransactionEnded are called. As
// such, it is possible that the transaction does not yet have a pool created.

// TODO: is this a plausible and/or likely scenario? Do we need to have a mechanism to ensure
// TODO: that the pending creation of a transacted pool for this transaction is aborted when
// TODO: PutTransactedObject finally gets some CPU time?

lock (_transactedCxns)
{
if (_transactedCxns.TryGetValue(transaction, out connections))
{
Debug.Assert(connections != null);

bool shouldDisposeConnections = false;

// Lock connections to avoid conflict with GetTransactionObject
lock (connections)
{
entry = connections.IndexOf(transactedObject);

if (entry >= 0)
{
connections.RemoveAt(entry);
}

// Once we've completed all the ended notifications, we can
// safely remove the list from the transacted pool.
if (0 >= connections.Count)
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactionEnded|RES|CPOOL> {0}, Transaction {1}, Removing List from transacted pool.", ObjectID, transaction.GetHashCode());
_transactedCxns.Remove(transaction);

// we really need to dispose our connection list; it may have
// native resources via the tx and GC may not happen soon enough.
shouldDisposeConnections = true;
}
}

if (shouldDisposeConnections)
{
connections.Dispose();
}
}
else
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactionEnded|RES|CPOOL> {0}, Transaction {1}, Connection {2}, Transacted pool not yet created prior to transaction completing. Connection may be leaked.", ObjectID, transaction.GetHashCode(), transactedObject.ObjectID);
}
}

// If (and only if) we found the connection in the list of
// connections, we'll put it back...
if (0 <= entry)
{

SqlClientEventSource.Metrics.ExitFreeConnection();
Pool.PutObjectFromTransactedPool(transactedObject);
}
}

}

private sealed class PoolWaitHandles
{
private readonly Semaphore _poolSemaphore;
Expand Down
0 ...ectionPool/ChannelDbConnectionPoolTest.cs → ...ectionPool/ChannelDbConnectionPoolTest.cs
View file
Open in desktop
File renamed without changes.
0 ...ConnectionPool/ConnectionPoolSlotsTest.cs → ...ConnectionPool/ConnectionPoolSlotsTest.cs
View file
Open in desktop
File renamed without changes.
Loading
Loading