-
Notifications
You must be signed in to change notification settings - Fork 330
Extract pool blocking-period error state into BlockingPeriodErrorState #4395
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?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -175,6 +175,44 @@ internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, Ti | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates an <see cref="ITimer"/> using the supplied <see cref="TimeProvider"/> without | ||
| /// capturing the current <see cref="ExecutionContext"/>. Suppressing the flow prevents the | ||
| /// timer from rooting the calling context and its <c>AsyncLocal</c> values for the lifetime | ||
| /// of the timer, while routing creation through <paramref name="timeProvider"/> so callers | ||
| /// can inject a test double (e.g. a fake time provider) for deterministic scheduling. | ||
| /// </summary> | ||
| /// <param name="timeProvider">The time provider used to create the timer.</param> | ||
| /// <param name="callback">The delegate invoked when the timer fires.</param> | ||
| /// <param name="state">An object passed to <paramref name="callback"/>, or <see langword="null"/>.</param> | ||
| /// <param name="dueTime">The amount of time to wait before the first invocation, or | ||
| /// <see cref="Timeout.InfiniteTimeSpan"/> to create the timer disarmed.</param> | ||
| /// <param name="period">The interval between invocations, or | ||
| /// <see cref="Timeout.InfiniteTimeSpan"/> to disable periodic signaling.</param> | ||
| /// <returns>An <see cref="ITimer"/> created by <paramref name="timeProvider"/>.</returns> | ||
| // TODO: Route the other UnsafeCreateTimer overloads through this method (passing | ||
| // TimeProvider.System) so execution-context suppression lives in a single place. | ||
| internal static ITimer UnsafeCreateTimer( | ||
| TimeProvider timeProvider, | ||
| TimerCallback callback, | ||
| object state, | ||
| TimeSpan dueTime, | ||
| TimeSpan period) | ||
|
Comment on lines
+195
to
+200
|
||
| { | ||
| // Don't capture the current ExecutionContext and its AsyncLocals onto | ||
| // a timer causing them to live forever. Honor the supplied TimeProvider | ||
| // so callers can inject a test double for deterministic scheduling. | ||
| if (ExecutionContext.IsFlowSuppressed()) | ||
| { | ||
| return timeProvider.CreateTimer(callback, state, dueTime, period); | ||
|
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. Creating a timer via a TimeProvider allows for faking time in unit tests. I chose not to change existing overloads to feed into this one because there are many spots already use them and it would be a more extensive refactor. |
||
| } | ||
|
|
||
| using (ExecutionContext.SuppressFlow()) | ||
| { | ||
| return timeProvider.CreateTimer(callback, state, dueTime, period); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| #region COM+ exceptions | ||
| internal static ArgumentException Argument(string error) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,253 @@ | ||
| // 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.Threading; | ||
| using Microsoft.Data.Common; | ||
| using Microsoft.Data.SqlClient.Internal; | ||
|
|
||
| #nullable enable | ||
|
|
||
| namespace Microsoft.Data.SqlClient.ConnectionPool | ||
| { | ||
| /// <summary> | ||
| /// Encapsulates a connection pool's blocking-period error state: cached exception, exponential | ||
| /// backoff timer, and synchronization. Kept as a separate class so the pool's | ||
| /// connection-acquisition path remains focused on capacity/queue concerns and stays | ||
| /// decoupled from the (independent) rate limiting policy. | ||
| /// </summary> | ||
| internal sealed class BlockingPeriodErrorState : IDisposable | ||
| { | ||
| // Backoff interval used the first time the pool enters the blocking period, and the | ||
| // value the backoff resets to on a successful create or Clear(). | ||
| private static readonly TimeSpan InitialWait = TimeSpan.FromSeconds(5); | ||
|
|
||
| // Upper bound the exponential backoff is capped at; further failures never wait longer. | ||
| private static readonly TimeSpan MaxWait = TimeSpan.FromSeconds(60); | ||
|
|
||
| // Identifier of the owning pool, included in trace events for diagnostics. | ||
| private readonly int _ownerPoolId; | ||
|
|
||
| // Optional callback invoked (under _lock) when the state enters the blocking period; | ||
| // used by the legacy wait-handle pool to signal its error wait handle. Must be cheap | ||
| // and non-reentrant. | ||
| private readonly Action? _onEnter; | ||
|
|
||
| // Optional callback invoked (under _lock) when the state leaves the blocking period | ||
| // via the exit timer or Clear(). Must be cheap and non-reentrant. | ||
| private readonly Action? _onExit; | ||
|
|
||
| // Time source used to create the exit timer; overridable so tests can drive scheduling | ||
| // deterministically. Defaults to TimeProvider.System. | ||
| private readonly TimeProvider _timeProvider; | ||
|
|
||
| // Guards the mutable error state (_cachedException, _exitTimer, _nextWait, _disposed). | ||
| private readonly object _lock = new(); | ||
|
|
||
| // Non-null while the pool is in the blocking period. Doubles as the "has error" | ||
| // flag, so callers don't need a separate bool. Volatile so other threads observe | ||
| // entry/exit transitions without acquiring _lock. | ||
| private volatile Exception? _cachedException; | ||
|
|
||
| // True from the moment Enter() activates the blocking period until Clear()/Dispose() | ||
| // fully resets it. The exit timer clears _cachedException but leaves this set so a | ||
| // later Clear() still resets the backoff. Volatile so Clear() can take a | ||
| // lock-free path when there is nothing to do. This allows Clear() to be called on hot paths. | ||
| private volatile bool _inElevatedState; | ||
|
|
||
| // The armed exit timer that ends the current blocking period; null when no period is | ||
| // active. Replaced (and the old one disposed) each time Enter() is called. | ||
| private ITimer? _exitTimer; | ||
|
|
||
| // The backoff interval to use for the next Enter(); doubles per failure up to MaxWait | ||
| // and resets to InitialWait on a successful create or Clear(). | ||
| private TimeSpan _nextWait = InitialWait; | ||
|
|
||
| // True once Dispose() has run, so repeated disposal and post-teardown work are no-ops. | ||
| private bool _disposed; | ||
|
|
||
| /// <summary> | ||
| /// Creates a new instance. | ||
| /// </summary> | ||
| /// <param name="ownerPoolId">Identifier of the owning pool, used in trace events.</param> | ||
| /// <param name="onEnter">Optional callback invoked (while holding the internal lock) after | ||
| /// the state transitions into the blocking period. Used by the legacy wait-handle pool to | ||
| /// signal its error wait handle. Because it runs under a lock, it must be cheap and | ||
| /// non-reentrant.</param> | ||
| /// <param name="onExit">Optional callback invoked (while holding the internal lock) after the | ||
| /// state transitions out of the blocking period via the exit timer or <see cref="Clear"/>. | ||
| /// Because it runs under a lock, it must be cheap and non-reentrant.</param> | ||
| /// <param name="timeProvider">The time provider used to create the exit timer. Defaults to | ||
| /// <see cref="TimeProvider.System"/>. Inject a test double (e.g. | ||
| /// <c>Microsoft.Extensions.Time.Testing.FakeTimeProvider</c>) in unit tests to | ||
| /// control timer scheduling deterministically.</param> | ||
| internal BlockingPeriodErrorState(int ownerPoolId, Action? onEnter = null, Action? onExit = null, TimeProvider? timeProvider = null) | ||
| { | ||
| _ownerPoolId = ownerPoolId; | ||
| _onEnter = onEnter; | ||
| _onExit = onExit; | ||
| _timeProvider = timeProvider ?? TimeProvider.System; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// True while the pool is in the blocking period. Subsequent acquisition attempts | ||
| /// should fast-fail with the cached exception. | ||
| /// </summary> | ||
| internal bool HasError => _cachedException is not null; | ||
|
|
||
| /// <summary> | ||
| /// Throws the cached error if the pool is currently in the blocking period. | ||
| /// </summary> | ||
| internal void ThrowIfActive() | ||
| { | ||
| Exception? cached = _cachedException; | ||
| if (cached is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Clone SqlExceptions so stack traces are not shared across callers; other | ||
| // exception types are rethrown as-is. | ||
| throw cached is SqlException sqlEx ? sqlEx.InternalClone() : cached; | ||
|
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 was the existing cloning logic |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Enters the blocking period, caching the supplied exception and scheduling a timer | ||
| /// to exit the period after the current backoff interval. Subsequent failures double | ||
| /// the backoff up to <see cref="MaxWait"/>. | ||
| /// </summary> | ||
| internal void Enter(Exception ex) | ||
| { | ||
| TimeSpan wait; | ||
| ITimer? oldTimer; | ||
|
|
||
| // If we call this, we're already in an exception path. Prefer correctness over performance. | ||
| lock (_lock) | ||
| { | ||
| _inElevatedState = true; | ||
| _cachedException = ex; | ||
| wait = _nextWait; | ||
|
|
||
| // Create the exit timer disarmed (infinite due time); it is armed below, still | ||
| // under the lock. ADP.UnsafeCreateTimer suppresses execution-context flow so the timer | ||
| // doesn't capture and pin the current ExecutionContext and its AsyncLocals for its | ||
| // lifetime, while still honoring the injected TimeProvider for testability. | ||
|
Comment on lines
+132
to
+135
|
||
| ITimer newTimer = ADP.UnsafeCreateTimer( | ||
| _timeProvider, | ||
| ExitCallback, | ||
| null, | ||
| wait, | ||
| Timeout.InfiniteTimeSpan); | ||
|
Comment on lines
+137
to
+141
|
||
| oldTimer = _exitTimer; | ||
| _exitTimer = newTimer; | ||
|
|
||
| // Bump the backoff for the next failure, capped at MaxWait. FR-008. | ||
| TimeSpan doubled = _nextWait + _nextWait; | ||
| _nextWait = doubled >= MaxWait ? MaxWait : doubled; | ||
|
|
||
| // Signal the enter event while still holding the lock so the external signal order | ||
| // (onEnter/onExit) can never diverge from the internal state transitions under | ||
| // concurrent Enter/Clear/exit-timer activity. The callbacks are expected to be | ||
| // cheap, non-reentrant operations. | ||
| _onEnter?.Invoke(); | ||
| } | ||
|
|
||
| oldTimer?.Dispose(); | ||
|
|
||
| SqlClientEventSource.Log.TryPoolerTraceEvent( | ||
| "<prov.DbConnectionPool.EnterErrorState|RES|CPOOL> {0}, Entering blocking period for {1}ms.", | ||
| _ownerPoolId, | ||
| (int)wait.TotalMilliseconds); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Clears the cached error state, disposes the exit timer, and resets the backoff to | ||
| /// its initial value. | ||
| /// </summary> | ||
| internal void Clear() | ||
| { | ||
| // Fast path: the create flow calls Clear() after every successful create, so avoid | ||
| // taking the lock in the common (no-error) case where there is nothing to reset. | ||
| if (!_inElevatedState) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| ITimer? oldTimer; | ||
|
|
||
| lock (_lock) | ||
| { | ||
| _cachedException = null; | ||
| _nextWait = InitialWait; | ||
| _inElevatedState = false; | ||
| oldTimer = _exitTimer; | ||
| _exitTimer = null; | ||
|
|
||
| // Signal and trace under the lock so the exit signal is ordered consistently | ||
| // with the state transition relative to concurrent Enter/exit-timer callbacks. | ||
| _onExit?.Invoke(); | ||
| } | ||
|
|
||
| oldTimer?.Dispose(); | ||
|
|
||
| SqlClientEventSource.Log.TryPoolerTraceEvent( | ||
| "<prov.DbConnectionPool.ClearErrorState|RES|CPOOL> {0}, Error state cleared.", _ownerPoolId); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Timer callback that exits the blocking period by clearing the cached exception, | ||
| /// allowing the next caller to attempt a fresh connection creation. The current | ||
| /// backoff is left intact so that, if the next attempt fails, the backoff continues | ||
| /// to grow rather than resetting. The backoff is reset only on <see cref="Clear"/>. | ||
| /// </summary> | ||
| /// <param name="state">The state object passed to the timer callback. Not used, | ||
| /// but required for the callback signature. | ||
| /// </param> | ||
| private void ExitCallback(object? state) | ||
| { | ||
| ITimer? oldTimer; | ||
|
|
||
| lock (_lock) | ||
| { | ||
| _cachedException = null; | ||
| oldTimer = _exitTimer; | ||
| _exitTimer = null; | ||
|
|
||
| // Signal and trace under the lock so the exit signal is ordered consistently | ||
| // with the state transition relative to concurrent Enter/Clear callbacks. | ||
| _onExit?.Invoke(); | ||
| } | ||
|
|
||
| oldTimer?.Dispose(); | ||
|
|
||
| SqlClientEventSource.Log.TryPoolerTraceEvent( | ||
| "<prov.DbConnectionPool.ExitErrorStateCallback|RES|CPOOL> {0}, Exiting blocking period.", _ownerPoolId); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Disposes the instance, releasing the exit timer if one is active. Clears the | ||
| /// error state so that any waiting callers do not observe a stale exception after | ||
| /// the owning pool is torn down. | ||
| /// </summary> | ||
| public void Dispose() | ||
| { | ||
| ITimer? timerToDispose; | ||
| lock (_lock) | ||
| { | ||
| if (_disposed) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _disposed = true; | ||
| _cachedException = null; | ||
| _inElevatedState = false; | ||
| timerToDispose = _exitTimer; | ||
| _exitTimer = null; | ||
| } | ||
|
|
||
| timerToDispose?.Dispose(); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.