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 568592072d..fb1147a8bc 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -175,6 +175,44 @@ internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, Ti } } + /// + /// Creates an using the supplied without + /// capturing the current . Suppressing the flow prevents the + /// timer from rooting the calling context and its AsyncLocal values for the lifetime + /// of the timer, while routing creation through so callers + /// can inject a test double (e.g. a fake time provider) for deterministic scheduling. + /// + /// The time provider used to create the timer. + /// The delegate invoked when the timer fires. + /// An object passed to , or . + /// The amount of time to wait before the first invocation, or + /// to create the timer disarmed. + /// The interval between invocations, or + /// to disable periodic signaling. + /// An created by . + // 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) + { + // 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); + } + + using (ExecutionContext.SuppressFlow()) + { + return timeProvider.CreateTimer(callback, state, dueTime, period); + } + } + #region COM+ exceptions internal static ArgumentException Argument(string error) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/BlockingPeriodErrorState.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/BlockingPeriodErrorState.cs new file mode 100644 index 0000000000..a74e36c4f5 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/BlockingPeriodErrorState.cs @@ -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 +{ + /// + /// 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. + /// + 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; + + /// + /// Creates a new instance. + /// + /// Identifier of the owning pool, used in trace events. + /// 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. + /// Optional callback invoked (while holding the internal lock) after the + /// state transitions out of the blocking period via the exit timer or . + /// Because it runs under a lock, it must be cheap and non-reentrant. + /// The time provider used to create the exit timer. Defaults to + /// . Inject a test double (e.g. + /// Microsoft.Extensions.Time.Testing.FakeTimeProvider) in unit tests to + /// control timer scheduling deterministically. + internal BlockingPeriodErrorState(int ownerPoolId, Action? onEnter = null, Action? onExit = null, TimeProvider? timeProvider = null) + { + _ownerPoolId = ownerPoolId; + _onEnter = onEnter; + _onExit = onExit; + _timeProvider = timeProvider ?? TimeProvider.System; + } + + /// + /// True while the pool is in the blocking period. Subsequent acquisition attempts + /// should fast-fail with the cached exception. + /// + internal bool HasError => _cachedException is not null; + + /// + /// Throws the cached error if the pool is currently in the blocking period. + /// + 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; + } + + /// + /// 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 . + /// + 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. + ITimer newTimer = ADP.UnsafeCreateTimer( + _timeProvider, + ExitCallback, + null, + wait, + Timeout.InfiniteTimeSpan); + 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( + " {0}, Entering blocking period for {1}ms.", + _ownerPoolId, + (int)wait.TotalMilliseconds); + } + + /// + /// Clears the cached error state, disposes the exit timer, and resets the backoff to + /// its initial value. + /// + 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( + " {0}, Error state cleared.", _ownerPoolId); + } + + /// + /// 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 . + /// + /// The state object passed to the timer callback. Not used, + /// but required for the callback signature. + /// + 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( + " {0}, Exiting blocking period.", _ownerPoolId); + } + + /// + /// 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. + /// + public void Dispose() + { + ITimer? timerToDispose; + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + _cachedException = null; + _inElevatedState = false; + timerToDispose = _exitTimer; + _exitTimer = null; + } + + timerToDispose?.Dispose(); + } + } +} 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 8f62787e30..c1f2b730cc 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 @@ -93,6 +93,26 @@ internal DbConnectionPoolGroupProviderInfo ProviderInfo internal SqlMetaDataFactory MetaDataFactory { get; set; } + /// + /// Determines whether the blocking period is enabled for this pool group based on the + /// configured and the target data source. + /// + internal bool IsBlockingPeriodEnabled() + { + switch (_connectionOptions.PoolBlockingPeriod) + { + case PoolBlockingPeriod.Auto: + return !ADP.IsAzureSqlServerEndpoint(_connectionOptions.DataSource); + case PoolBlockingPeriod.AlwaysBlock: + return true; + case PoolBlockingPeriod.NeverBlock: + return false; + default: + Debug.Fail("Unknown PoolBlockingPeriod. Please specify explicit results in above switch case statement."); + return true; + } + } + internal int Clear() { // must be multi-thread safe with competing calls by Clear and Prune via background thread 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 d2c3b57323..86b23a5137 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 @@ -29,7 +29,7 @@ namespace Microsoft.Data.SqlClient.ConnectionPool /// Transaction-Aware Pooling: Tracks connections enlisted in using TransactedConnectionPool and TransactedConnectionList, ensuring proper context reuse. /// Concurrency and Synchronization: Uses wait handles and semaphores via PoolWaitHandles to coordinate safe multi-threaded access. /// Connection Lifecycle Management: Manages creation (CreateObject), deactivation (DeactivateObject), destruction (DestroyObject), and reclamation (ReclaimEmancipatedObjects) of internal connections. - /// Error Handling and Resilience: Implements retry and exponential backoff in TryGetConnection and handles transient errors using _errorWait. + /// Error Handling and Resilience: Implements retry and exponential backoff in TryGetConnection and delegates blocking-period bookkeeping (cached exception, exit timer) to . /// Minimum Pool Size Enforcement: Maintains the MinPoolSize by spawning background tasks to create new connections when needed. /// Load Balancing Support: Honors LoadBalanceTimeout to clean up idle connections and distribute load evenly. /// Telemetry and Tracing: Uses SqlClientEventSource for extensive diagnostic tracing of connection lifecycle events. @@ -165,8 +165,6 @@ public void Dispose() private const int WAIT_ABANDONED = 0x80; - private const int ERROR_WAIT_DEFAULT = 5 * 1000; // 5 seconds - // we do want a testable, repeatable set of generated random numbers private static readonly Random s_random = new Random(5101977); // Value obtained from Dave Driver @@ -194,11 +192,7 @@ public void Dispose() private int _waitCount; private readonly PoolWaitHandles _waitHandles; - private Exception _resError; - private volatile bool _errorOccurred; - - private int _errorWait; - private Timer _errorTimer; + private readonly BlockingPeriodErrorState _errorState; private Timer _cleanupTimer; @@ -252,8 +246,12 @@ internal WaitHandleDbConnectionPool( _waitHandles = new PoolWaitHandles(); - _errorWait = ERROR_WAIT_DEFAULT; - _errorTimer = null; // No error yet. + // Hook the wait-handle event so any thread blocked in WaitAny over the pool's + // handles wakes up immediately when the blocking period is entered/exited. + _errorState = new BlockingPeriodErrorState( + Id, + onEnter: () => _waitHandles.ErrorEvent.Set(), + onExit: () => _waitHandles.ErrorEvent.Reset()); _objectList = new List(MaxPoolSize); @@ -283,7 +281,7 @@ private int CreationTimeout public SqlConnectionFactory ConnectionFactory => _connectionFactory; - public bool ErrorOccurred => _errorOccurred; + public bool ErrorOccurred => _errorState.HasError; private bool HasTransactionAffinity => PoolGroupOptions.HasTransactionAffinity; @@ -515,39 +513,6 @@ private Timer CreateCleanupTimer() => _cleanupWait, _cleanupWait); - private bool IsBlockingPeriodEnabled() - { - var poolGroupConnectionOptions = _connectionPoolGroup.ConnectionOptions; - if (poolGroupConnectionOptions == null) - { - return true; - } - - var policy = poolGroupConnectionOptions.PoolBlockingPeriod; - - switch (policy) - { - case PoolBlockingPeriod.Auto: - { - return !ADP.IsAzureSqlServerEndpoint(poolGroupConnectionOptions.DataSource); - } - case PoolBlockingPeriod.AlwaysBlock: - { - return true; //Enabled - } - case PoolBlockingPeriod.NeverBlock: - { - return false; //Disabled - } - default: - { - //we should never get into this path. - Debug.Fail("Unknown PoolBlockingPeriod. Please specify explicit results in above switch case statement."); - return true; - } - } - } - private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout) { DbConnectionInternal newObj = null; @@ -573,14 +538,14 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Connection {1}, Added to pool.", Id, newObj?.ObjectID); - // Reset the error wait: - _errorWait = ERROR_WAIT_DEFAULT; + // A successful creation clears any prior error state and resets backoff. + _errorState.Clear(); } catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { ADP.TraceExceptionWithoutRethrow(e); - if (!IsBlockingPeriodEnabled()) + if (!_connectionPoolGroup.IsBlockingPeriodEnabled()) { throw; } @@ -593,33 +558,10 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio newObj = null; // set to null, so we do not return bad new object - // Failed to create instance - _resError = e; - - // Make sure the timer starts even if ThreadAbort occurs after setting the ErrorEvent. - Timer t = new Timer(new TimerCallback(this.ErrorCallback), null, Timeout.Infinite, Timeout.Infinite); - - bool timerIsNotDisposed; - - _waitHandles.ErrorEvent.Set(); - _errorOccurred = true; - - // Enable the timer. - // Note that the timer is created to allow periodic invocation. If ThreadAbort occurs in the middle of ErrorCallback, - // the timer will restart. Otherwise, the timer callback (ErrorCallback) destroys the timer after resetting the error to avoid second callback. - _errorTimer = t; - timerIsNotDisposed = t.Change(_errorWait, _errorWait); - - Debug.Assert(timerIsNotDisposed, "ErrorCallback timer has been disposed"); + // Enter the blocking period: caches the exception, schedules the exit timer, + // and signals the wait-handle error event via the onEnter callback. + _errorState.Enter(e); - if (30000 < _errorWait) - { - _errorWait = 60000; - } - else - { - _errorWait *= 2; - } throw; } return newObj; @@ -792,28 +734,6 @@ private void DestroyObject(DbConnectionInternal obj) } } - private void ErrorCallback(object state) - { - SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Resetting Error handling.", Id); - _errorOccurred = false; - _waitHandles.ErrorEvent.Reset(); - - // the error state is cleaned, destroy the timer to avoid periodic invocation - Timer t = _errorTimer; - _errorTimer = null; - if (t != null) - { - t.Dispose(); // Cancel timer request. - } - } - - - private Exception TryCloneCachedException() - // Cached exception can be of any type, so is not always cloneable. - // This functions clones SqlException - // OleDb and Odbc connections are not passing throw this code - => _resError is SqlException sqlEx ? sqlEx.InternalClone() : _resError; - private void WaitForPendingOpen() { Debug.Assert(!Thread.CurrentThread.IsThreadPoolThread, "This thread may block for a long time. Threadpool threads should not be used."); @@ -1037,7 +957,12 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj // Throw the error that PoolCreateRequest stashed. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Errors are set.", Id); Interlocked.Decrement(ref _waitCount); - throw TryCloneCachedException(); + _errorState.ThrowIfActive(); + // Narrow race: error state cleared between WaitAny observing + // the signal and this check. Re-balance _waitCount and let the + // outer do/while loop retry. + Interlocked.Increment(ref _waitCount); + break; case CREATION_HANDLE: SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Creating new connection.", Id); @@ -1616,25 +1541,20 @@ private DbConnectionInternal UserCreateRequest(DbConnection owningObject, Timeou // instead obtained creation mutex DbConnectionInternal obj = null; - if (ErrorOccurred) - { - throw TryCloneCachedException(); - } - else + _errorState.ThrowIfActive(); + + if ((oldConnection != null) || (Count < MaxPoolSize) || (0 == MaxPoolSize)) { - if ((oldConnection != null) || (Count < MaxPoolSize) || (0 == MaxPoolSize)) - { - // If we have an odd number of total objects, reclaim any dead objects. - // If we did not find any objects to reclaim, create a new one. + // If we have an odd number of total objects, reclaim any dead objects. + // If we did not find any objects to reclaim, create a new one. - // TODO: Consider implement a control knob here; why do we only check for dead objects ever other time? why not every 10th time or every time? - if ((oldConnection != null) || (Count & 0x1) == 0x1 || !ReclaimEmancipatedObjects()) - { - obj = CreateObject(owningObject, oldConnection, timeout); - } + // TODO: Consider implement a control knob here; why do we only check for dead objects ever other time? why not every 10th time or every time? + if ((oldConnection != null) || (Count & 0x1) == 0x1 || !ReclaimEmancipatedObjects()) + { + obj = CreateObject(owningObject, oldConnection, timeout); } - return obj; } + return obj; } } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/BlockingPeriodErrorStateTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/BlockingPeriodErrorStateTest.cs new file mode 100644 index 0000000000..157249f5f9 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/BlockingPeriodErrorStateTest.cs @@ -0,0 +1,688 @@ +// 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.Extensions.Time.Testing; +using Microsoft.Data.SqlClient.ConnectionPool; +using Xunit; + +#nullable enable + +namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool +{ + /// + /// Comprehensive unit tests for covering: + /// - Initial state and error caching + /// - and exception handling + /// - and state reset + /// - Exponential backoff progression (verified with ) + /// - Timer-driven exit behavior (verified with ) + /// - implementation and timer cleanup + /// - Callback invocation and re-entrancy safety + /// + public class BlockingPeriodErrorStateTest + { + #region HasError / initial state + + /// + /// Verifies that a newly constructed has + /// set to false. + /// + [Fact] + public void HasError_InitialState_IsFalse() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + + // Act & Assert + Assert.False(state.HasError); + } + + /// + /// Verifies that does not throw + /// when called on a newly constructed instance with no cached error. + /// + [Fact] + public void ThrowIfActive_InitialState_DoesNotThrow() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + + // Act & Assert + state.ThrowIfActive(); // Should complete without throwing + } + + #endregion + + #region Enter + + /// + /// Verifies that calling sets + /// to true. + /// + [Fact] + public void Enter_SetsHasErrorToTrue() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + + // Act + state.Enter(new InvalidOperationException("test")); + + // Assert + Assert.True(state.HasError); + } + + /// + /// Verifies that throws + /// the exact exception type that was cached by . + /// + [Fact] + public void Enter_ThrowIfActive_ThrowsCachedExceptionType() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + var exception = new InvalidOperationException("boom"); + + // Act + state.Enter(exception); + + // Assert + var ex = Assert.Throws(() => state.ThrowIfActive()); + Assert.Equal("boom", ex.Message); + } + + /// + /// Verifies that when a is cached, + /// throws a cloned instance rather than the original, to avoid sharing stack traces across callers. + /// + [Fact] + public void Enter_WithSqlException_ThrowsClonedInstance() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + var original = SqlExceptionHelper.CreateSqlException("connection failed"); + + // Act + state.Enter(original); + var thrown = Assert.Throws(() => state.ThrowIfActive()); + + // Assert + Assert.NotSame(original, thrown); + Assert.Equal(original.Message, thrown.Message); + } + + /// + /// Verifies that invokes the optional + /// onEnter callback after entering the blocking period. + /// + [Fact] + public void Enter_InvokesOnEnterCallback() + { + // Arrange + int callCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onEnter: () => callCount++); + + // Act + state.Enter(new Exception()); + + // Assert + Assert.Equal(1, callCount); + } + + /// + /// Verifies that calling a second time + /// replaces the cached exception, invokes the callback again, and the new exception is thrown. + /// + [Fact] + public void Enter_CalledTwice_ReplacesExceptionAndInvokesOnEnterAgain() + { + // Arrange + int enterCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onEnter: () => enterCount++); + + // Act + state.Enter(new InvalidOperationException("first")); + state.Enter(new ArgumentException("second")); + + // Assert + Assert.Equal(2, enterCount); + var ex = Assert.Throws(() => state.ThrowIfActive()); + Assert.Equal("second", ex.Message); + } + + /// + /// Verifies that does not invoke + /// the onExit callback (only the onEnter callback or the timer should trigger onExit). + /// + [Fact] + public void Enter_DoesNotInvokeOnExitCallback() + { + // Arrange + int exitCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onExit: () => exitCount++); + + // Act + state.Enter(new Exception()); + + // Assert + Assert.Equal(0, exitCount); + } + + #endregion + + #region Clear + + /// + /// Verifies that resets + /// to false. + /// + [Fact] + public void Clear_AfterEnter_ResetsHasError() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + state.Enter(new Exception()); + + // Act + state.Clear(); + + // Assert + Assert.False(state.HasError); + } + + /// + /// Verifies that after , + /// does not throw because the cached error has been cleared. + /// + [Fact] + public void Clear_AfterEnter_ThrowIfActiveDoesNotThrow() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + state.Enter(new Exception()); + + // Act + state.Clear(); + + // Assert + state.ThrowIfActive(); // Must not throw + } + + /// + /// Verifies that invokes the optional + /// onExit callback after clearing the error state. + /// + [Fact] + public void Clear_AfterEnter_InvokesOnExitCallback() + { + // Arrange + int exitCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onExit: () => exitCount++); + state.Enter(new Exception()); + + // Act + state.Clear(); + + // Assert + Assert.Equal(1, exitCount); + } + + /// + /// Verifies that on an initial (no-error) state + /// does not invoke the onExit callback because there is nothing to clear. + /// + [Fact] + public void Clear_OnInitialState_DoesNotInvokeOnExitCallback() + { + // Arrange + int exitCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onExit: () => exitCount++); + + // Act + state.Clear(); + + // Assert + Assert.Equal(0, exitCount); + } + + /// + /// Verifies that is idempotent: + /// calling it a second time does not invoke the onExit callback again. + /// + [Fact] + public void Clear_CalledTwice_OnExitInvokedOnce() + { + // Arrange + int exitCount = 0; + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, onExit: () => exitCount++); + state.Enter(new Exception()); + + // Act + state.Clear(); + state.Clear(); + + // Assert + Assert.Equal(1, exitCount); + } + + /// + /// Verifies that resets the backoff + /// timeout to its initial value, so the next + /// uses the initial wait duration instead of the accumulated backoff. + /// + [Fact] + public void Clear_ResetsBackoffSoNextEnterUsesInitialWait() + { + // Arrange + using var state = new BlockingPeriodErrorState(ownerPoolId: 1); + state.Enter(new Exception("first")); + + // Act + state.Clear(); + state.Enter(new Exception("second")); + + // Assert + Assert.True(state.HasError); + } + + #endregion + + #region Backoff progression + + /// + /// Verifies that the initial enter schedules the timer with the 5-second initial wait. + /// The error state should persist until the timer fires, after which it clears automatically. + /// + [Fact] + public void Enter_FirstEntry_SchedulesInitialWaitTimer() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, timeProvider: fakeTime); + state.Enter(new Exception()); + + // Act: advance just under the initial 5s wait + fakeTime.Advance(TimeSpan.FromSeconds(4)); + + // Assert: timer has not fired yet + Assert.True(state.HasError); + + // Act: advance past the due time + fakeTime.Advance(TimeSpan.FromSeconds(1)); + + // Assert: timer has fired, error cleared + Assert.False(state.HasError); + } + + /// + /// Verifies that successive timer-driven exits double the backoff each time: + /// 5 s → 10 s → 20 s → 40 s → 60 s (capped at MaxWait). + /// Each Enter schedules the timer for the current accumulated wait and the error + /// persists until exactly that duration elapses. + /// + [Fact] + public void Enter_BackoffDoublesOnSuccessiveTimerFiredEntries() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, timeProvider: fakeTime); + + // (expectedWaitSeconds, _) — the wait used by Enter for that iteration + int[] expectedWaits = [5, 10, 20, 40, 60, 60]; + + // Act & Assert + foreach (int wait in expectedWaits) + { + state.Enter(new Exception($"attempt at wait={wait}s")); + + // One second before due time: error still active + fakeTime.Advance(TimeSpan.FromSeconds(wait - 1)); + Assert.True(state.HasError, $"HasError should be true after {wait - 1}s (scheduled wait={wait}s)"); + + // Final second: timer fires, error clears + fakeTime.Advance(TimeSpan.FromSeconds(1)); + Assert.False(state.HasError, $"HasError should be false after {wait}s (scheduled wait={wait}s)"); + } + } + + /// + /// Verifies that a timer-driven exit does NOT reset the backoff. The accumulated + /// backoff is preserved so the next failure uses the doubled wait, reflecting + /// continued instability. Only resets + /// the backoff to the initial value. In this way, we only reset the backoff when + /// a connection is successfully established. + /// + [Fact] + public void Enter_WhenTimerFires_DoesNotResetBackoff() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, timeProvider: fakeTime); + + // First enter: uses 5s wait; _nextWait advances to 10s + state.Enter(new Exception("first")); + fakeTime.Advance(TimeSpan.FromSeconds(5)); // timer fires + Assert.False(state.HasError); + + // Act: enter again — should use 10s, not the initial 5s + state.Enter(new Exception("second")); + + // Assert: not cleared after 9s + fakeTime.Advance(TimeSpan.FromSeconds(9)); + Assert.True(state.HasError); + + // Assert: cleared after the full 10s + fakeTime.Advance(TimeSpan.FromSeconds(1)); + Assert.False(state.HasError); + } + + /// + /// Verifies that resets the backoff + /// to the initial 5-second wait even after the timer has doubled it, so the next + /// enter cycle starts fresh. + /// + [Fact] + public void Clear_AfterTimerFiredEntry_ResetsBackoffToInitialWait() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, timeProvider: fakeTime); + + // First enter/timer-exit: _nextWait advances from 5s to 10s + state.Enter(new Exception("first")); + fakeTime.Advance(TimeSpan.FromSeconds(5)); + Assert.False(state.HasError); + + // Second enter to accumulate more backoff; then Clear resets it + state.Enter(new Exception("second")); // _nextWait advances to 20s + state.Clear(); // _nextWait resets to 5s + + // Act: enter again — should use the initial 5s wait + state.Enter(new Exception("third")); + + // Assert: not cleared after 4s + fakeTime.Advance(TimeSpan.FromSeconds(4)); + Assert.True(state.HasError); + + // Assert: cleared after the initial 5s + fakeTime.Advance(TimeSpan.FromSeconds(1)); + Assert.False(state.HasError); + } + + #endregion + + #region Timer behavior + + /// + /// Verifies that the timer-driven exit invokes the onExit callback, the same + /// callback path used by . + /// + [Fact] + public void Enter_WhenTimerFires_InvokesOnExitCallback() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + int exitCount = 0; + using var state = new BlockingPeriodErrorState( + ownerPoolId: 1, + onExit: () => exitCount++, + timeProvider: fakeTime); + state.Enter(new Exception()); + + // Act + fakeTime.Advance(TimeSpan.FromSeconds(5)); + + // Assert + Assert.Equal(1, exitCount); + Assert.False(state.HasError); + } + + /// + /// Verifies that the timer does not fire before its due time, confirming the + /// scheduled wait is respected and not fired early. + /// + [Fact] + public void Enter_TimerDoesNotFireBeforeDueTime() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + using var state = new BlockingPeriodErrorState(ownerPoolId: 1, timeProvider: fakeTime); + state.Enter(new Exception()); + + // Act: advance to 1ms before the 5s due time + fakeTime.Advance(TimeSpan.FromMilliseconds(4999)); + + // Assert + Assert.True(state.HasError); + } + + #endregion + + #region IDisposable + + /// + /// Verifies that on an initial state + /// does not throw and completes successfully. + /// + [Fact] + public void Dispose_OnInitialState_DoesNotThrow() + { + // Arrange & Act + var state = new BlockingPeriodErrorState(ownerPoolId: 1); + + // Assert + state.Dispose(); // Should not throw + } + + /// + /// Verifies that clears the cached + /// error state so is false after disposal. + /// + [Fact] + public void Dispose_AfterEnter_ClearsHasError() + { + // Arrange + var state = new BlockingPeriodErrorState(ownerPoolId: 1); + state.Enter(new Exception()); + + // Act + state.Dispose(); + + // Assert + Assert.False(state.HasError); + } + + /// + /// Verifies that is idempotent: + /// calling it multiple times does not throw and completes successfully. + /// + [Fact] + public void Dispose_CalledMultipleTimes_DoesNotThrow() + { + // Arrange & Act + var state = new BlockingPeriodErrorState(ownerPoolId: 1); + state.Dispose(); + + // Assert + state.Dispose(); // Must be idempotent and not throw + } + + /// + /// Verifies that does not invoke + /// the onExit callback because disposal is a resource-cleanup path, not a logical + /// "exit blocking period" event. + /// + [Fact] + public void Dispose_DoesNotInvokeOnExitCallback() + { + // Arrange + int exitCount = 0; + var state = new BlockingPeriodErrorState(ownerPoolId: 1, onExit: () => exitCount++); + state.Enter(new Exception()); + + // Act + state.Dispose(); + + // Assert + Assert.Equal(0, exitCount); + } + + /// + /// Verifies that properly releases + /// and cancels the internal exit timer, preventing stale callbacks from firing after disposal. + /// Uses to advance time deterministically past the timer's due + /// time without relying on real-time sleeps. + /// + [Fact] + public void Dispose_ReleasesTimer_NoCallbackAfterDispose() + { + // Arrange + var fakeTime = new FakeTimeProvider(); + int exitCount = 0; + var state = new BlockingPeriodErrorState( + ownerPoolId: 1, + onExit: () => Interlocked.Increment(ref exitCount), + timeProvider: fakeTime); + state.Enter(new Exception()); + + // Act: dispose cancels the pending timer + state.Dispose(); + + // Advance well past the 5s due time — the cancelled timer must not fire + fakeTime.Advance(TimeSpan.FromSeconds(60)); + + // Assert + Assert.Equal(0, exitCount); + } + + /// + /// Verifies that works correctly in a standard + /// using statement, with no exceptions thrown during disposal. + /// + [Fact] + public void Dispose_WithUsingStatement_DoesNotThrow() + { + // Arrange & Act + using (var state = new BlockingPeriodErrorState(ownerPoolId: 1)) + { + state.Enter(new Exception()); + Assert.True(state.HasError); + } + + // Assert + // No exception expected when the using block exits + } + + #endregion + + #region Callback behaviour + + /// + /// Verifies that both onEnter and onExit callbacks are optional (nullable) + /// and the instance works correctly when neither is provided. + /// + [Fact] + public void Callbacks_AreNotRequiredAndDefaultToNull() + { + // Arrange & Act + using var state = new BlockingPeriodErrorState(ownerPoolId: 42); + state.Enter(new Exception()); + + // Assert + state.Clear(); // Should work without callbacks + } + + /// + /// Verifies that the onEnter callback is invoked after the internal lock is released. + /// The callback reads and calls + /// — operations that are safe only + /// when the lock is not held. If the implementation were changed to hold the lock during + /// the callback invocation, any re-entrant call from the callback that tries to acquire the + /// same lock (on a non-re-entrant lock) would deadlock. + /// + [Fact] + public void OnEnter_CalledOutsideLock_CanCallBackIntoState() + { + // Arrange + bool hasErrorObservedInCallback = false; + BlockingPeriodErrorState? state = null; + using (state = new BlockingPeriodErrorState( + ownerPoolId: 1, + onEnter: () => + { + // Observe state from inside the callback. + // HasError must already be true at this point. + hasErrorObservedInCallback = state!.HasError; + + // Calling ThrowIfActive from the callback must not deadlock. + Assert.Throws(() => state.ThrowIfActive()); + })) + { + + // Act + state.Enter(new InvalidOperationException("test")); + } + + // Assert + Assert.True(hasErrorObservedInCallback); + } + + /// + /// Verifies that the onExit callback is invoked after the internal lock is released. + /// The callback reads — confirming it + /// observes the cleared state — and calls + /// without deadlocking. If the implementation were changed to hold the lock during the + /// callback, any re-entrant call from the callback would deadlock on a non-re-entrant lock. + /// + [Fact] + public void OnExit_CalledOutsideLock_CanCallBackIntoState() + { + // Arrange + bool hasErrorObservedInCallback = true; // initialized to true; must be false after Clear + BlockingPeriodErrorState? state = null; + using (state = new BlockingPeriodErrorState( + ownerPoolId: 1, + onExit: () => + { + // HasError must already be false when onExit is called. + hasErrorObservedInCallback = state!.HasError; + + // Calling ThrowIfActive from the callback must not deadlock. + state.ThrowIfActive(); // must not throw + })) + { + + // Act + state.Enter(new Exception()); + state.Clear(); + } + + // Assert + Assert.False(hasErrorObservedInCallback); + } + + #endregion + } + + /// + /// Test helper for creating instances. Since has + /// an internal constructor, instances must be created via the factory method. + /// + internal static class SqlExceptionHelper + { + /// + /// Creates a with the specified message using the internal factory method. + /// + /// The error message for the exception. + /// A new with the specified message. + internal static SqlException CreateSqlException(string message) + { + var collection = new SqlErrorCollection(); + collection.Add(new SqlError(0, (byte)0, (byte)0, "TestServer", message, "", 0)); + return SqlException.CreateException(collection, ""); + } + } +}