Skip to content

Commit b8d56d9

Browse files
Address PR #4302 review: reuse Clear() from Shutdown() in both pools
Both pool implementations now delegate the connection drain in Shutdown() to the existing Clear() routine instead of duplicating the drain inline. ChannelDbConnectionPool.Shutdown(): keeps the CompareExchange election, State transition and _idleChannel.Complete() call, then calls Clear() (which bumps _clearGeneration and best-effort drains the idle channel). A final unbounded drain loop is retained as a mop-up because Clear() may short-circuit when another Clear() is concurrently in flight; the final loop is safe because the channel is already completed and no new writes can succeed. WaitHandleDbConnectionPool.Shutdown(): keeps the trace, idempotent State check, timer disposal and waiter wake-up, then replaces the inline _stackNew / _stackOld drain loops with a single Clear() call. Clear() additionally dooms every entry in _objectList, decrements the ExitFreeConnection metric per drained connection, and runs ReclaimEmancipatedObjects() — strictly more cleanup than the previous inline drain, with no behaviour change for normal Running-state operations. Per @mdaigle review feedback on #4302.
1 parent 7ba73a8 commit b8d56d9

2 files changed

Lines changed: 15 additions & 15 deletions

File tree

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,17 +282,23 @@ public void Shutdown()
282282
SqlClientEventSource.Log.TryPoolerTraceEvent(
283283
"<prov.DbConnectionPool.Shutdown|RES|INFO|CPOOL> {0}", Id);
284284

285-
// FR-001: transition to ShuttingDown. After this point, ReturnInternalConnection
285+
// Transition to ShuttingDown. After this point, ReturnInternalConnection
286286
// routes returning connections to RemoveConnection.
287287
State = ShuttingDown;
288288

289-
// FR-002 + FR-007: complete the channel writer so:
289+
// Complete the channel writer so:
290290
// - no further idle connections can be enqueued (TryWrite returns false), and
291291
// - in-flight / future async waiters on ReadAsync fault with ChannelClosedException.
292292
_idleChannel.Complete();
293293

294-
// FR-003: drain remaining buffered idle connections and destroy them. The channel is
295-
// unbounded so all already-enqueued items can be drained synchronously.
294+
// Reuse Clear() for the drain. Clear bumps _clearGeneration so any active
295+
// checked-out connection fails IsLiveConnection on return and is removed, and it
296+
// drains the idle channel up to its captured IdleCount.
297+
Clear();
298+
299+
// Clear() may short-circuit if another caller is already draining. Because the
300+
// channel is now completed, no new items can be enqueued, so it is safe to do a
301+
// final unbounded drain to mop up anything Clear() may have skipped.
296302
while (_idleChannel.TryRead(out DbConnectionInternal? connection))
297303
{
298304
if (connection is not null)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,17 +1555,11 @@ public void Shutdown()
15551555
}
15561556
}
15571557

1558-
// Drain idle stacks and destroy contained connections. Active checked-out
1559-
// connections are destroyed when they are returned (see DeactivateObject's
1560-
// State is ShuttingDown branch).
1561-
while (_stackNew.TryPop(out DbConnectionInternal newObj))
1562-
{
1563-
DestroyObject(newObj);
1564-
}
1565-
while (_stackOld.TryPop(out DbConnectionInternal oldObj))
1566-
{
1567-
DestroyObject(oldObj);
1568-
}
1558+
// Reuse Clear() to doom every connection (including active checked-out ones), drain
1559+
// both idle stacks, and reclaim emancipated objects. Active connections destroy
1560+
// themselves on return either via the doom flag or via DeactivateObject's
1561+
// State == ShuttingDown branch.
1562+
Clear();
15691563
}
15701564

15711565
// TransactionEnded merely provides the plumbing for DbConnectionInternal to access the transacted pool

0 commit comments

Comments
 (0)