-
Notifications
You must be signed in to change notification settings - Fork 330
Implement pool shutdown for ChannelDbConnectionPool and harden WaitHandleDbConnectionPool shutdown #4302
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.
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
base: main
Are you sure you want to change the base?
Implement pool shutdown for ChannelDbConnectionPool and harden WaitHandleDbConnectionPool shutdown #4302
Changes from all commits
6431209
b24d4fb
7ba73a8
b8d56d9
9a45a66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,16 +189,16 @@ public void Dispose() | |
|
|
||
| private readonly WaitCallback _poolCreateRequest; | ||
|
|
||
| private int _waitCount; | ||
| internal int _waitCount; | ||
| private readonly PoolWaitHandles _waitHandles; | ||
|
|
||
| private Exception _resError; | ||
| private volatile bool _errorOccurred; | ||
|
|
||
| private int _errorWait; | ||
| private Timer _errorTimer; | ||
| internal Timer _errorTimer; | ||
|
|
||
| private Timer _cleanupTimer; | ||
| internal Timer _cleanupTimer; | ||
|
|
||
| private readonly TransactedConnectionPool _transactedConnectionPool; | ||
|
|
||
|
|
@@ -327,8 +327,16 @@ public bool IsRunning | |
|
|
||
| public TransactedConnectionPool TransactedConnectionPool => _transactedConnectionPool; | ||
|
|
||
| private void CleanupCallback(object state) | ||
| internal void CleanupCallback(object state) | ||
| { | ||
| // If the pool is shutting down, skip work. Shutdown disposes the timer, but | ||
| // a callback may already be in-flight when Shutdown runs; this guard ensures it does | ||
| // not perform pruning or re-arm pool create requests. | ||
| if (State == ShuttingDown) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Called when the cleanup-timer ticks over. | ||
|
|
||
| // This is the automatic pruning method. Every period, we will | ||
|
|
@@ -765,8 +773,15 @@ private void DestroyObject(DbConnectionInternal obj) | |
| } | ||
| } | ||
|
|
||
| private void ErrorCallback(object state) | ||
| internal void ErrorCallback(object state) | ||
| { | ||
| // Skip work if the pool is shutting down. The shutdown path disposes the | ||
| // timer; this guard handles the in-flight-callback race. | ||
| if (State == ShuttingDown) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.ErrorCallback|RES|CPOOL> {0}, Resetting Error handling.", Id); | ||
| _errorOccurred = false; | ||
| _waitHandles.ErrorEvent.Reset(); | ||
|
|
@@ -956,6 +971,31 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj | |
| { | ||
| waitResult = WaitHandle.WaitAny(_waitHandles.GetHandles(allowCreate), unchecked((int)waitForMultipleObjectsTimeout)); | ||
|
|
||
| // After waking, observe shutdown state and bail out so waiters | ||
| // do not spin against a drained pool. If WaitAny consumed a | ||
| // PoolSemaphore slot, release it back so the accounting stays | ||
| // balanced; otherwise the slot would leak and other waiters | ||
| // (or callers that arrive after Shutdown completes its own | ||
| // Release loop) would starve. | ||
| if (State != Running) | ||
| { | ||
| SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Pool is shutting down; abandoning wait.", Id); | ||
| if (waitResult == SEMAPHORE_HANDLE || waitResult == WAIT_ABANDONED + SEMAPHORE_HANDLE) | ||
| { | ||
| try | ||
| { | ||
| _waitHandles.PoolSemaphore.Release(1); | ||
| } | ||
| catch (SemaphoreFullException) | ||
| { | ||
| // Pool semaphore was already saturated by Shutdown's bulk release; safe to ignore. | ||
| } | ||
| } | ||
| Interlocked.Decrement(ref _waitCount); | ||
| connection = null; | ||
| return false; | ||
| } | ||
|
|
||
| // From the WaitAny docs: "If more than one object became signaled during | ||
| // the call, this is the array index of the signaled object with the | ||
| // smallest index value of all the signaled objects." This is important | ||
|
|
@@ -1481,15 +1521,45 @@ public void Startup() | |
| public void Shutdown() | ||
| { | ||
| SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.Shutdown|RES|INFO|CPOOL> {0}", Id); | ||
|
|
||
| // Idempotent: subsequent calls observe ShuttingDown and bail. | ||
| if (State == ShuttingDown) | ||
| { | ||
| return; | ||
| } | ||
| State = ShuttingDown; | ||
|
|
||
| // deactivate timer callbacks | ||
| Timer t = _cleanupTimer; | ||
| _cleanupTimer = null; | ||
| if (t != null) | ||
| // Dispose all background timers so they no longer schedule new work. | ||
| // Note that any timer callback already in flight may still observe State == ShuttingDown | ||
| // and short-circuit (see CleanupCallback / ErrorCallback). | ||
| Timer cleanup = Interlocked.Exchange(ref _cleanupTimer, null); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make sure we've addressed this bug: #1881
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR adds the per-pool Shutdown() machinery that #1881 ultimately needs, but it doesn't actually touch SqlConnectionFactory._pruningTimer — so the symptom (the PruneConnectionPoolGroups trace firing every 30 s after ClearAllPools()) will still reproduce. I'd prefer to land #4302 as the foundation and address #1881 in a small follow-up that adds a "park the timer when there's no work" guard to PruneConnectionPoolGroups and a re-arm in GetConnectionPoolGroup. Happy to file that follow-up issue and link it. |
||
| cleanup?.Dispose(); | ||
| Timer error = Interlocked.Exchange(ref _errorTimer, null); | ||
| error?.Dispose(); | ||
|
|
||
| // Wake any threads parked in WaitHandle.WaitAny by releasing as many semaphore | ||
| // slots as there are recorded waiters. Using _waitCount (rather than MaxPoolSize) | ||
| // avoids ArgumentOutOfRangeException when MaxPoolSize == 0 (unlimited) and ensures | ||
| // we wake every parked waiter even when _waitCount exceeds MaxPoolSize. Waiters | ||
| // observe State != Running after wake-up and bail. | ||
| int waitersToWake = Volatile.Read(ref _waitCount); | ||
| if (waitersToWake > 0) | ||
| { | ||
| t.Dispose(); | ||
| try | ||
| { | ||
| _waitHandles.PoolSemaphore.Release(waitersToWake); | ||
| } | ||
| catch (SemaphoreFullException) | ||
| { | ||
| // Semaphore already saturated; nothing to do. | ||
| } | ||
| } | ||
|
|
||
| // Reuse Clear() to doom every connection (including active checked-out ones), drain | ||
| // both idle stacks, and reclaim emancipated objects. Active connections destroy | ||
| // themselves on return either via the doom flag or via DeactivateObject's | ||
| // State == ShuttingDown branch. | ||
| Clear(); | ||
| } | ||
|
|
||
| // TransactionEnded merely provides the plumbing for DbConnectionInternal to access the transacted pool | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.