From d37911c58d91eeaa5003885bb6ff389266603927 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 11 May 2026 15:13:41 -0700 Subject: [PATCH 01/39] Clean up TimeoutTimer Co-authored-by: Copilot --- .../Data/ProviderBase/TimeoutTimer.cs | 263 +++++++++--------- .../Connection/SqlConnectionInternal.cs | 7 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- 3 files changed, 139 insertions(+), 133 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 37c94fe355..11c7c66229 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -8,148 +8,104 @@ namespace Microsoft.Data.ProviderBase { - // Purpose: - // Manages determining and tracking timeouts - // - // Intended use: - // Call StartXXXXTimeout() to get a timer with the given expiration point - // Get remaining time in appropriate format to pass to subsystem timeouts - // Check for timeout via IsExpired for checks in managed code. - // Simply abandon to GC when done. + /// + /// Manages determining and tracking timeouts for use by subsystems that perform + /// time-bounded operations. + /// + /// + /// + /// Intended use: + /// + /// + /// Call to get a timer with the given expiration point. + /// Read the remaining time in the appropriate format to pass to subsystem timeouts. + /// Check for timeout via for checks in managed code. + /// Simply abandon the instance to the GC when done. + /// + /// internal class TimeoutTimer { - //------------------- - // Fields - //------------------- - private long _timerExpire; - private bool _isInfiniteTimeout; - private long _originalTimerTicks; - - //------------------- - // Timeout-setting methods - //------------------- - - // Get a new timer that will expire in the given number of seconds - // For input, a value of zero seconds indicates infinite timeout - internal static TimeoutTimer StartSecondsTimeout(int seconds) - { - //-------------------- - // Preconditions: None (seconds must conform to SetTimeoutSeconds requirements) + #region Fields - //-------------------- - // Method body - var timeout = new TimeoutTimer(); - timeout.SetTimeoutSeconds(seconds); + /// + /// The sentinel value (0) used to indicate an infinite timeout when starting a timer. + /// + internal static readonly long InfiniteTimeout = 0; - //--------------------- - // Postconditions - Debug.Assert(timeout != null); // Need a valid timeouttimer if no error + #endregion - return timeout; - } + #region Constructors - // Get a new timer that will expire in the given number of milliseconds - // No current need to support infinite milliseconds timeout - internal static TimeoutTimer StartMillisecondsTimeout(long milliseconds) + /// + /// Initializes a new instance of the class with the + /// specified expiration duration. + /// + /// + /// The duration before the timer expires. A value whose ticks equal + /// indicates an infinite timeout. + /// + private TimeoutTimer(TimeSpan expiration) { - //-------------------- - // Preconditions - Debug.Assert(0 <= milliseconds); - - //-------------------- - // Method body - var timeout = new TimeoutTimer(); - timeout._originalTimerTicks = milliseconds * TimeSpan.TicksPerMillisecond; - timeout._timerExpire = checked(ADP.TimerCurrent() + timeout._originalTimerTicks); - timeout._isInfiniteTimeout = false; - - //--------------------- - // Postconditions - Debug.Assert(timeout != null); // Need a valid timeouttimer if no error - - return timeout; + OriginalTicks = expiration.Ticks; + IsInfinite = OriginalTicks == InfiniteTimeout; + ExpirationTicks = IsInfinite ? long.MaxValue : checked(ADP.TimerCurrent() + OriginalTicks); } - //------------------- - // Methods for changing timeout - //------------------- - - internal void SetTimeoutSeconds(int seconds) - { - //-------------------- - // Preconditions - Debug.Assert(0 <= seconds || InfiniteTimeout == seconds); // no need to support negative seconds at present - - //-------------------- - // Method body - if (InfiniteTimeout == seconds) - { - _isInfiniteTimeout = true; - } - else - { - // Stash current time + timeout - _originalTimerTicks = ADP.TimerFromSeconds(seconds); - _timerExpire = checked(ADP.TimerCurrent() + _originalTimerTicks); - _isInfiniteTimeout = false; - } - - //--------------------- - // Postconditions:None + #endregion + + #region Properties + + /// + /// Gets the absolute tick value at which this timer is considered expired. + /// + /// + /// The tick count, in units, at which the timer + /// expires; when is + /// . + /// + internal long ExpirationTicks { + get; + //TODO: Remove this when we disable Reset() + private set; } - // Reset timer to original duration. - internal void Reset() - { - if (InfiniteTimeout == _originalTimerTicks) - { - _isInfiniteTimeout = true; - } - else - { - _timerExpire = checked(ADP.TimerCurrent() + _originalTimerTicks); - _isInfiniteTimeout = false; - } - } - - //------------------- - // Timeout info properties - //------------------- - - // Indicator for infinite timeout when starting a timer - internal static readonly long InfiniteTimeout = 0; - - // Is this timer in an expired state? + /// + /// Gets a value indicating whether this timer has expired. + /// + /// + /// if the timer is not infinite and the current time + /// has passed ; otherwise, . + /// internal bool IsExpired { get { - return !IsInfinite && ADP.TimerHasExpired(_timerExpire); - } - } - - // is this an infinite-timeout timer? - internal bool IsInfinite - { - get - { - return _isInfiniteTimeout; - } - } - - // Special accessor for TimerExpire for use when thunking to legacy timeout methods. - public long LegacyTimerExpire - { - get - { - return (_isInfiniteTimeout) ? long.MaxValue : _timerExpire; + return !IsInfinite && ADP.TimerHasExpired(ExpirationTicks); } } - // Returns milliseconds remaining trimmed to zero for none remaining - // and long.MaxValue for infinite - // This method should be preferred for internal calculations that are not - // yet common enough to code into the TimeoutTimer class itself. + /// + /// Gets a value indicating whether this timer represents an infinite timeout. + /// + /// + /// if the timer was created with an expiration whose + /// ticks equal ; otherwise, . + /// + internal bool IsInfinite { get; } + + /// + /// Gets the number of milliseconds remaining before this timer expires, + /// trimmed to 0 when none remain and to + /// when the timer is infinite. + /// + /// + /// A non-negative count of milliseconds remaining; + /// when is . + /// + /// + /// This property should be preferred for internal calculations that are not + /// yet common enough to code into the class itself. + /// internal long MillisecondsRemaining { get @@ -160,13 +116,13 @@ internal long MillisecondsRemaining //------------------- // Method Body long milliseconds; - if (_isInfiniteTimeout) + if (IsInfinite) { milliseconds = long.MaxValue; } else { - milliseconds = ADP.TimerRemainingMilliseconds(_timerExpire); + milliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); if (0 > milliseconds) { milliseconds = 0; @@ -181,7 +137,16 @@ internal long MillisecondsRemaining } } - // Returns milliseconds remaining trimmed to zero for none remaining + /// + /// Gets the number of milliseconds remaining before this timer expires as + /// a 32-bit integer, trimmed to 0 when none remain and saturated to + /// when the remaining time exceeds that value or + /// when the timer is infinite. + /// + /// + /// A non-negative count of milliseconds remaining, never exceeding + /// . + /// internal int MillisecondsRemainingInt { get @@ -189,13 +154,13 @@ internal int MillisecondsRemainingInt //------------------- // Method Body int milliseconds; - if (_isInfiniteTimeout) + if (IsInfinite) { milliseconds = int.MaxValue; } else { - long longMilliseconds = ADP.TimerRemainingMilliseconds(_timerExpire); + long longMilliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); if (0 > longMilliseconds) { milliseconds = 0; @@ -217,5 +182,45 @@ internal int MillisecondsRemainingInt return milliseconds; } } + + /// + /// Gets the original timeout duration, in ticks, that was supplied when the + /// timer was created. Used by to restore the original + /// expiration window. + /// + private long OriginalTicks { get; } + + #endregion + + #region Methods + + /// + /// Creates and starts a new with the specified + /// expiration duration. + /// + /// + /// The duration before the returned timer expires. A value whose ticks equal + /// produces an infinite timer. + /// + /// A new instance that has already started. + internal static TimeoutTimer StartNew(TimeSpan expiration) => new TimeoutTimer(expiration); + + /// + /// Resets the timeout to its original duration. + /// + /// + /// This method is only used to retry after federated authentication timeouts, + /// which can use up the whole timeout due to MFA. Has no effect when + /// is . + /// + internal void Reset() + { + if (!IsInfinite) + { + ExpirationTicks = checked(ADP.TimerCurrent() + OriginalTicks); + } + } + + #endregion } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index e62541137c..466a235152 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -399,7 +399,7 @@ internal SqlConnectionInternal( try { - _timeout = TimeoutTimer.StartSecondsTimeout(connectionOptions.ConnectTimeout); + _timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout)); // If transient fault handling is enabled then we can retry the login up to the // ConnectRetryCount. @@ -2204,6 +2204,7 @@ private void AttemptOneLogin( /// if a cached token exists from a previous auth attempt (see GetFedAuthToken). /// // @TODO: Rename to meet naming conventions + // TODO: if this call timed out, what reason do we have to believe some other call succeeded? why not just fail? private bool AttemptRetryADAuthWithTimeoutError( SqlException sqlex, SqlConnectionOptions connectionOptions, // @TODO: this is not used @@ -3216,7 +3217,7 @@ private void LoginNoFailover( { nextTimeoutInterval = milliseconds; } - intervalTimer = TimeoutTimer.StartMillisecondsTimeout(nextTimeoutInterval); + intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); } // Re-allocate parser each time to make sure state is known. @@ -3501,7 +3502,7 @@ private void LoginWithFailover( nextTimeoutInterval = milliseconds; } - TimeoutTimer intervalTimer = TimeoutTimer.StartMillisecondsTimeout(nextTimeoutInterval); + TimeoutTimer intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); // Re-allocate parser each time to make sure state is known. If parser was created // by previous attempt, dispose it to properly close the socket, if created. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index 53b89fbc90..ded27eeb03 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -542,7 +542,7 @@ bool withFailover } _state = TdsParserState.OpenNotLoggedIn; _physicalStateObj.SniContext = SniContext.Snix_PreLoginBeforeSuccessfulWrite; - _physicalStateObj.TimeoutTime = timeout.LegacyTimerExpire; + _physicalStateObj.TimeoutTime = timeout.ExpirationTicks; bool marsCapable = false; From cdbac4afb5ff9085ec491fb702d0464719f773cb Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 11 May 2026 16:12:29 -0700 Subject: [PATCH 02/39] Pass TimeoutTimer down from pool. Co-authored-by: Copilot --- .../Data/ProviderBase/TimeoutTimer.cs | 30 ++ .../Connection/SqlConnectionInternal.cs | 5 +- .../ConnectionPool/ChannelDbConnectionPool.cs | 30 +- .../Microsoft/Data/SqlClient/SqlConnection.cs | 1 + .../Data/SqlClient/SqlConnectionFactory.cs | 12 +- .../ChannelDbConnectionPoolTest.cs | 323 +++++++++++++++++- ...itHandleDbConnectionPoolTransactionTest.cs | 5 +- 7 files changed, 390 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 11c7c66229..e40c4e3beb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -5,6 +5,7 @@ using Microsoft.Data.Common; using System; using System.Diagnostics; +using System.Threading; namespace Microsoft.Data.ProviderBase { @@ -205,6 +206,35 @@ internal int MillisecondsRemainingInt /// A new instance that has already started. internal static TimeoutTimer StartNew(TimeSpan expiration) => new TimeoutTimer(expiration); + /// + /// Creates a new that will be canceled + /// when this timer expires. + /// + /// + /// A scheduled to cancel after + /// milliseconds. When + /// is , the returned source + /// is never automatically canceled. When the timer has already expired, the + /// returned source is already canceled. + /// + internal CancellationTokenSource CreateCancellationTokenSource() + { + if (IsInfinite) + { + return new CancellationTokenSource(); + } + + int remaining = MillisecondsRemainingInt; + if (remaining == 0) + { + CancellationTokenSource cts = new CancellationTokenSource(); + cts.Cancel(); + return cts; + } + + return new CancellationTokenSource(remaining); + } + /// /// Resets the timeout to its original duration. /// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 466a235152..73a993aebf 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -317,6 +317,7 @@ internal class SqlConnectionInternal : DbConnectionInternal, IDisposable internal SqlConnectionInternal( DbConnectionPoolIdentity identity, SqlConnectionOptions connectionOptions, + TimeoutTimer timeout, SqlCredential credential, DbConnectionPoolGroupProviderInfo providerInfo, string newPassword, @@ -399,8 +400,8 @@ internal SqlConnectionInternal( try { - _timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout)); - + + _timeout = timeout; // If transient fault handling is enabled then we can retry the login up to the // ConnectRetryCount. int connectionEstablishCount = applyTransientFaultHandling diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index b83434cac6..c4cfd5bfa7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -283,7 +283,13 @@ public bool TryGetConnection( TaskCompletionSource? taskCompletionSource, out DbConnectionInternal? connection) { - var timeout = TimeSpan.FromSeconds(owningObject.ConnectionTimeout); + // Create a single TimeoutTimer that represents the overall connection timeout budget. + // This timer is threaded through all layers (pool wait, physical connection creation) so that + // time spent waiting in the pool is deducted from the budget available for physical connection + // establishment. Without this, each layer would start its own fresh timeout and the total wait + // could exceed the configured ConnectTimeout. + // Note: TimeoutTimer treats 0 seconds as infinite timeout, which matches ConnectTimeout=0 semantics. + var timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(owningObject.ConnectionTimeout)); // If taskCompletionSource is null, we are in a sync context. if (taskCompletionSource is null) @@ -372,13 +378,16 @@ public bool TryGetConnection( /// /// The owning connection. /// The cancellation token to cancel the operation. + /// The overall timeout budget. Passed through to the physical connection + /// so it uses the remaining budget rather than starting a fresh timeout. /// A task representing the asynchronous operation, with a result of the new internal connection. /// /// Thrown when the cancellation token is cancelled before the connection operation completes. /// private DbConnectionInternal? OpenNewInternalConnection( DbConnection? owningConnection, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + TimeoutTimer timeout) { cancellationToken.ThrowIfCancellationRequested(); @@ -397,9 +406,11 @@ public bool TryGetConnection( // DbConnectionInternal doesn't support an async open. It's better to block this thread and keep // throughput high than to queue all of our opens onto a single worker thread. Add an async path // when this support is added to DbConnectionInternal. + // TODO: ultimately, the connection factory should also accept our cancellation token. var connection = ConnectionFactory.CreatePooledConnection( owningConnection, - this); + this, + timeout); if (connection is not null) { @@ -492,7 +503,8 @@ private void RemoveConnection(DbConnectionInternal connection) /// /// The DbConnection that will own this internal connection /// A boolean indicating whether the operation should be asynchronous. - /// The timeout for the operation. + /// The overall timeout budget for this connection request. Time spent waiting + /// in the pool is deducted from the budget available for physical connection creation. /// Returns a DbConnectionInternal that is retrieved from the pool. /// /// Thrown when an OperationCanceledException is caught, indicating that the timeout period @@ -505,10 +517,13 @@ private void RemoveConnection(DbConnectionInternal connection) private async Task GetInternalConnection( DbConnection owningConnection, bool async, - TimeSpan timeout) + TimeoutTimer timeout) { DbConnectionInternal? connection = null; - using CancellationTokenSource cancellationTokenSource = new(timeout); + + // Derive a CancellationTokenSource from the TimeoutTimer so pool-internal wait operations + // (channel reads, semaphore waits) are cancelled when the overall budget expires. + using CancellationTokenSource cancellationTokenSource = timeout.CreateCancellationTokenSource(); CancellationToken cancellationToken = cancellationTokenSource.Token; // Continue looping until we create or retrieve a connection @@ -524,7 +539,8 @@ private async Task GetInternalConnection( // If we didn't find an idle connection, try to open a new one. connection ??= OpenNewInternalConnection( owningConnection, - cancellationToken); + cancellationToken, + timeout); // If we're at max capacity and couldn't open a connection. Block on the idle channel with a // timeout. Note that Channels guarantee fair FIFO behavior to callers of ReadAsync diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index aa9d8edc06..25614661de 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2729,6 +2729,7 @@ private static void ChangePassword(string connectionString, SqlConnectionOptions con = new SqlConnectionInternal( identity: null, connectionOptions, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout)), credential, providerInfo: null, newPassword, diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 30010d0d4b..39ea86a58b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -138,7 +138,8 @@ internal DbConnectionInternal CreateNonPooledConnection( internal DbConnectionInternal CreatePooledConnection( DbConnection owningConnection, - IDbConnectionPool pool) + IDbConnectionPool pool, + TimeoutTimer timeout) { Debug.Assert(pool != null, "null pool?"); @@ -147,7 +148,8 @@ internal DbConnectionInternal CreatePooledConnection( pool.PoolGroup.PoolKey, pool.PoolGroup.ProviderInfo, pool, - owningConnection); + owningConnection, + timeout); if (newConnection is null) { @@ -573,7 +575,8 @@ protected virtual DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { SqlConnectionOptions opt = options; ConnectionPoolKey key = poolKey; @@ -688,7 +691,8 @@ protected virtual DbConnectionInternal CreateConnection( key.AccessToken, pool, key.AccessTokenCallback, - key.SspiContextProvider); + key.SspiContextProvider, + timeout); } private static DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(SqlConnectionOptions connectionOptions) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 7c1593af14..3fcc381200 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -896,6 +896,7 @@ out DbConnectionInternal? newConnection #endregion #region Test classes +#nullable disable internal class SuccessfulSqlConnectionFactory : SqlConnectionFactory { protected override DbConnectionInternal CreateConnection( @@ -903,7 +904,8 @@ protected override DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { return new StubDbConnectionInternal(); } @@ -916,12 +918,60 @@ protected override DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout = null) { throw ADP.PooledOpenTimeout(); } } + internal class SlowSqlConnectionFactory : SqlConnectionFactory + { + private readonly TimeSpan _delay; + + internal SlowSqlConnectionFactory(TimeSpan delay) => _delay = delay; + + protected override DbConnectionInternal CreateConnection( + SqlConnectionOptions options, + ConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, + IDbConnectionPool pool, + DbConnection owningConnection, + TimeoutTimer timeout = null) + { + Thread.Sleep(_delay); + return new StubDbConnectionInternal(); + } + } + + /// + /// A factory that captures the TimeoutTimer passed to CreateConnection for test verification. + /// + internal class TimeoutCapturingSqlConnectionFactory : SqlConnectionFactory + { + private readonly TimeSpan _delay; + internal TimeoutTimer CapturedTimeout { get; private set; } + + internal TimeoutCapturingSqlConnectionFactory(TimeSpan delay = default) => _delay = delay; + + protected override DbConnectionInternal CreateConnection( + SqlConnectionOptions options, + ConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, + IDbConnectionPool pool, + DbConnection owningConnection, + TimeoutTimer timeout = null) + { + CapturedTimeout = timeout; + if (_delay > TimeSpan.Zero) + { + Thread.Sleep(_delay); + } + return new StubDbConnectionInternal(); + } + } +#nullable restore + internal class StubDbConnectionInternal : DbConnectionInternal { #region Not Implemented Members @@ -1081,5 +1131,274 @@ public void Constructor_WithValidSmallPoolSizes_WorksCorrectly() Assert.NotNull(pool2); Assert.Equal(0, pool2.Count); } + + #region Connection Timeout Awareness Tests + + [Fact] + public async Task GetConnectionInfiniteTimeout_ShouldNotTimeoutImmediately() + { + // Arrange: pool at max capacity (1) so the next request must wait + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: 0, + maxPoolSize: 1, + creationTimeout: 0, // 0 = infinite timeout + loadBalanceTimeout: 0, + hasTransactionAffinity: true + ); + var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); + + SqlConnection firstOwner = new(); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + Assert.NotNull(firstConnection); + + // Act: request a connection with ConnectTimeout=0 (infinite), then release after a delay + var waiterTask = Task.Run(() => + { + pool.TryGetConnection( + new SqlConnection("Timeout=0"), + taskCompletionSource: null, + out DbConnectionInternal? waitedConnection); + return waitedConnection; + }); + + // Give the waiter time to start blocking, then release the connection + await Task.Delay(TimeSpan.FromSeconds(2)); + Assert.False(waiterTask.IsCompleted, "Waiter should still be waiting (infinite timeout)"); + + pool.ReturnInternalConnection(firstConnection, firstOwner); + var result = await waiterTask; + + // Assert: the waiter should have received the connection without a timeout error + Assert.NotNull(result); + Assert.Same(firstConnection, result); + } + + [Fact] + public async Task GetConnectionAsyncInfiniteTimeout_ShouldNotTimeoutImmediately() + { + // Arrange: pool at max capacity (1) so the next request must wait + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: 0, + maxPoolSize: 1, + creationTimeout: 0, // 0 = infinite timeout + loadBalanceTimeout: 0, + hasTransactionAffinity: true + ); + var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); + + SqlConnection firstOwner = new(); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + Assert.NotNull(firstConnection); + + // Act: request a connection with ConnectTimeout=0 (infinite), then release after a delay + var tcs = new TaskCompletionSource(); + pool.TryGetConnection(new SqlConnection("Timeout=0"), tcs, out _); + + // Give the waiter time to start blocking, then release the connection + await Task.Delay(TimeSpan.FromSeconds(2)); + Assert.False(tcs.Task.IsCompleted, "Waiter should still be waiting (infinite timeout)"); + + pool.ReturnInternalConnection(firstConnection, firstOwner); + var result = await tcs.Task; + + // Assert: the waiter should have received the connection without a timeout error + Assert.NotNull(result); + Assert.Same(firstConnection, result); + } + + [Fact] + public void GetConnectionSlowCreation_ShouldTimeoutAfterCreation() + { + // Arrange: factory that takes 3 seconds to create a connection, but timeout is 1 second + var slowFactory = new SlowSqlConnectionFactory(TimeSpan.FromSeconds(3)); + var pool = ConstructPool(slowFactory); + + // Act & Assert: the creation itself takes 3s but CTS fires at 1s; + // the post-creation cancellation check in ConnectionPoolSlots.Add detects the timeout. + var ex = Assert.Throws(() => + { + pool.TryGetConnection( + new SqlConnection("Timeout=1"), + taskCompletionSource: null, + out DbConnectionInternal? internalConnection); + }); + + Assert.Equal( + "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", + ex.Message); + } + + [Fact] + public async Task GetConnectionAsyncSlowCreation_ShouldTimeoutAfterCreation() + { + // Arrange: factory that takes 3 seconds to create a connection, but timeout is 1 second + var slowFactory = new SlowSqlConnectionFactory(TimeSpan.FromSeconds(3)); + var pool = ConstructPool(slowFactory); + + // Act & Assert + var tcs = new TaskCompletionSource(); + pool.TryGetConnection( + new SqlConnection("Timeout=1"), + tcs, + out _); + + var ex = await Assert.ThrowsAsync(() => tcs.Task); + + Assert.Equal( + "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", + ex.Message); + } + + [Fact] + public async Task ConcurrentCallers_ShouldTimeoutIndependently() + { + // Arrange: pool at max capacity so both callers must wait + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: 0, + maxPoolSize: 1, + creationTimeout: 15, + loadBalanceTimeout: 0, + hasTransactionAffinity: true + ); + var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); + + SqlConnection firstOwner = new(); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + Assert.NotNull(firstConnection); + + // Act: two callers with different timeouts both waiting + // Caller A has a 1-second timeout, Caller B has a 5-second timeout + var callerATask = Task.Run(() => + { + pool.TryGetConnection( + new SqlConnection("Timeout=1"), + taskCompletionSource: null, + out DbConnectionInternal? connectionA); + return connectionA; + }); + + var callerBTask = Task.Run(() => + { + pool.TryGetConnection( + new SqlConnection("Timeout=5"), + taskCompletionSource: null, + out DbConnectionInternal? connectionB); + return connectionB; + }); + + // Wait for caller A to timeout + var exA = await Assert.ThrowsAsync(() => callerATask); + Assert.Equal( + "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", + exA.Message); + + // Caller B should still be waiting (5-second timeout not yet elapsed) + Assert.False(callerBTask.IsCompleted, "Caller B should still be waiting"); + + // Release the connection so caller B can succeed + pool.ReturnInternalConnection(firstConnection, firstOwner); + var resultB = await callerBTask; + + // Assert: caller B got the connection + Assert.NotNull(resultB); + Assert.Same(firstConnection, resultB); + } + + /// + /// Verifies that the pool passes a non-null TimeoutTimer to the factory's CreateConnection + /// method, and that it reflects the remaining budget (not a fresh full timeout). + /// + [Fact] + public void GetConnection_ShouldPassTimeoutTimerToFactory() + { + // Arrange: use a capturing factory + var factory = new TimeoutCapturingSqlConnectionFactory(); + var pool = ConstructPool(factory); + var owner = new SqlConnection("Timeout=30"); + + // Act + pool.TryGetConnection(owner, taskCompletionSource: null, out DbConnectionInternal? connection); + + // Assert: timeout was passed through + Assert.NotNull(connection); + Assert.NotNull(factory.CapturedTimeout); + Assert.False(factory.CapturedTimeout.IsExpired); + // The timer should have been started with ~30 seconds, but some time has elapsed + Assert.True(factory.CapturedTimeout.MillisecondsRemaining <= 30_000); + Assert.True(factory.CapturedTimeout.MillisecondsRemaining > 0); + } + + /// + /// Verifies that the TimeoutTimer passed to CreateConnection has reduced remaining time + /// when the pool spends time waiting (e.g., for a slot to free up). + /// + [Fact] + public void GetConnection_TimeoutTimerReflectsPoolWaitTime() + { + // Arrange: pool at max 1, fill it, then return after a delay + var factory = new TimeoutCapturingSqlConnectionFactory(delay: TimeSpan.FromMilliseconds(100)); + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: 0, + maxPoolSize: 1, + creationTimeout: 30, + loadBalanceTimeout: 0, + hasTransactionAffinity: true + ); + var pool = ConstructPool(factory, poolGroupOptions: poolGroupOptions); + + // Fill the pool + var firstOwner = new SqlConnection("Timeout=30"); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + Assert.NotNull(firstConnection); + long firstRemainingMs = factory.CapturedTimeout.MillisecondsRemaining; + + // Return after a short delay so second caller waits + Task.Run(async () => + { + await Task.Delay(500); + pool.ReturnInternalConnection(firstConnection, firstOwner); + }); + + // Act: second caller must wait for the first to return + var secondFactory = new TimeoutCapturingSqlConnectionFactory(); + // We can't swap factories, but the existing factory captures. Reset. + var secondOwner = new SqlConnection("Timeout=30"); + pool.TryGetConnection(secondOwner, taskCompletionSource: null, out DbConnectionInternal? secondConnection); + + // Assert: connection obtained, remaining time is less because pool wait consumed time + Assert.NotNull(secondConnection); + // The second call reuses the pool's timer from TryGetConnection, which has been running + // during the ~500ms wait. The factory won't be called for the second request since it + // gets the returned idle connection. But the first call's timer should show time passed. + Assert.True(factory.CapturedTimeout.MillisecondsRemaining < 30_000, + "TimeoutTimer should reflect elapsed time since pool entry"); + } + + /// + /// Verifies that ConnectTimeout=0 (infinite) creates an infinite TimeoutTimer. + /// + [Fact] + public void GetConnection_InfiniteTimeout_ShouldPassInfiniteTimeoutTimer() + { + // Arrange + var factory = new TimeoutCapturingSqlConnectionFactory(); + var pool = ConstructPool(factory); + var owner = new SqlConnection("Timeout=0"); + + // Act + pool.TryGetConnection(owner, taskCompletionSource: null, out DbConnectionInternal? connection); + + // Assert + Assert.NotNull(connection); + Assert.NotNull(factory.CapturedTimeout); + Assert.True(factory.CapturedTimeout.IsInfinite); + Assert.False(factory.CapturedTimeout.IsExpired); + } + + #endregion } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index a5e5d0339f..d6c6672660 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -895,6 +895,7 @@ public void SequentialTransactions_CanReuseConnections() #region Mock Classes +#nullable disable internal class MockSqlConnectionFactory : SqlConnectionFactory { protected override DbConnectionInternal CreateConnection( @@ -902,11 +903,13 @@ protected override DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { return new MockDbConnectionInternal(); } } +#nullable restore internal class MockDbConnectionInternal : DbConnectionInternal { From 8abc9d58888658f81a97a4da965ac756e2a93206 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 13:15:52 -0700 Subject: [PATCH 03/39] compilation fixes --- .../ConnectionPool/WaitHandleDbConnectionPool.cs | 9 ++++++++- .../Data/SqlClient/SqlConnectionFactory.cs | 13 ++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) 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 4243e777ba..e0ef45b51f 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 @@ -526,9 +526,16 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio try { + // Legacy pool: start a fresh per-acquisition timeout from the owning connection's + // ConnectionTimeout. Time spent waiting for an idle slot in this pool is tracked + // independently and is not deducted from this budget. + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(owningObject.ConnectionTimeout)); + newObj = _connectionFactory.CreatePooledConnection( owningObject, - this); + this, + timeout); lock (_objectList) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 39ea86a58b..078bd6f8d0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -120,12 +120,18 @@ internal DbConnectionInternal CreateNonPooledConnection( Debug.Assert(owningConnection is not null, "null owningConnection?"); Debug.Assert(poolGroup is not null, "null poolGroup?"); + // Non-pooled connections start a fresh timeout from the owning connection's + // ConnectionTimeout since there is no preceding pool wait to account for. + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(owningConnection.ConnectionTimeout)); + DbConnectionInternal newConnection = CreateConnection( poolGroup.ConnectionOptions, poolGroup.PoolKey, poolGroup.ProviderInfo, pool: null, - owningConnection); + owningConnection, + timeout); if (newConnection is not null) { SqlClientDiagnostics.Metrics.HardConnectRequest(); @@ -631,6 +637,7 @@ protected virtual DbConnectionInternal CreateConnection( SqlConnectionInternal sseConnection = new SqlConnectionInternal( identity, sseopt, + timeout, key.Credential, providerInfo: null, newPassword: string.Empty, @@ -681,6 +688,7 @@ protected virtual DbConnectionInternal CreateConnection( return new SqlConnectionInternal( identity, opt, + timeout, key.Credential, poolGroupProviderInfo, newPassword: string.Empty, @@ -691,8 +699,7 @@ protected virtual DbConnectionInternal CreateConnection( key.AccessToken, pool, key.AccessTokenCallback, - key.SspiContextProvider, - timeout); + key.SspiContextProvider); } private static DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(SqlConnectionOptions connectionOptions) From d8cb7f9dfaa6cb1c8a8b1f47b646a9a0f680942c Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 14:15:33 -0700 Subject: [PATCH 04/39] Lift TimeoutTimer creation up to SqlConnection.TryOpenInner Thread a single TimeoutTimer from SqlConnection.Open through DbConnectionInternal/DbConnectionClosed -> SqlConnectionFactory -> IDbConnectionPool implementations down to physical connection creation. Replaces per-layer TimeoutTimer construction. - DbConnectionInternal: OpenConnection creates the timer; TryOpenConnection, TryReplaceConnection, TryOpenConnectionInternal accept TimeoutTimer. - DbConnectionClosed/SqlConnectionInternal: signature pass-through. - SqlConnectionFactory: TryGetConnection, CreateNonPooledConnection, and CreateReplaceConnectionContinuation accept and forward TimeoutTimer. - IDbConnectionPool: TryGetConnection/ReplaceConnection now require TimeoutTimer. - ChannelDbConnectionPool: removes its local timer creation; uses caller's. - WaitHandleDbConnectionPool: PendingGetConnection carries TimeoutTimer; CreateObject/UserCreateRequest/ReplaceConnection accept it. Maintenance worker uses CreationTimeout-based timer. - SqlConnection.TryOpenInner: creates the TimeoutTimer for the open path. - Tests: TransactedConnectionPoolTest mock updated; new PoolTestExtensions provides legacy 3-arg overloads that derive the timer from owningObject.ConnectionTimeout for existing test call sites. --- .../Data/ProviderBase/DbConnectionClosed.cs | 21 ++++++---- .../Data/ProviderBase/DbConnectionInternal.cs | 15 ++++--- .../Connection/SqlConnectionInternal.cs | 5 ++- .../ConnectionPool/ChannelDbConnectionPool.cs | 9 ++-- .../ConnectionPool/IDbConnectionPool.cs | 7 +++- .../WaitHandleDbConnectionPool.cs | 42 ++++++++++--------- .../Microsoft/Data/SqlClient/SqlConnection.cs | 14 ++++++- .../Data/SqlClient/SqlConnectionFactory.cs | 19 ++++----- .../ConnectionPool/PoolTestExtensions.cs | 41 ++++++++++++++++++ .../TransactedConnectionPoolTest.cs | 4 +- 10 files changed, 123 insertions(+), 54 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs index 96704bacce..f5ae12f5a1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs @@ -62,8 +62,9 @@ protected internal override Task GetSchemaAsync( internal override bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) => - TryOpenConnectionInternal(outerConnection, connectionFactory, retry); + TaskCompletionSource retry, + TimeoutTimer timeout) => + TryOpenConnectionInternal(outerConnection, connectionFactory, retry, timeout); /// internal override void ResetConnection() => throw ADP.ClosedConnectionError(); @@ -78,7 +79,8 @@ protected DbConnectionBusy(ConnectionState state) : base(state, true, false) internal override bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) => throw ADP.ConnectionAlreadyOpen(State); } @@ -119,13 +121,15 @@ internal override void CloseConnection(DbConnection owningObject, SqlConnectionF internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) => - TryOpenConnection(outerConnection, connectionFactory, retry); + TaskCompletionSource retry, + TimeoutTimer timeout) => + TryOpenConnection(outerConnection, connectionFactory, retry, timeout); internal override bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { if (retry == null || !retry.Task.IsCompleted) { @@ -173,7 +177,8 @@ private DbConnectionClosedPreviouslyOpened() : base(ConnectionState.Closed, true internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) => - TryOpenConnection(outerConnection, connectionFactory, retry); + TaskCompletionSource retry, + TimeoutTimer timeout) => + TryOpenConnection(outerConnection, connectionFactory, retry, timeout); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index ac3949c726..9f3f8fcf93 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -688,7 +688,9 @@ internal void MakePooledConnection(IDbConnectionPool connectionPool) internal virtual void OpenConnection(DbConnection outerConnection, SqlConnectionFactory connectionFactory) { - if (!TryOpenConnection(outerConnection, connectionFactory, null)) + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(outerConnection.ConnectionTimeout)); + if (!TryOpenConnection(outerConnection, connectionFactory, null, timeout)) { throw ADP.InternalError(ADP.InternalErrorCode.SynchronousConnectReturnedPending); } @@ -800,7 +802,8 @@ internal void SetInStasis() internal virtual bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { throw ADP.ConnectionAlreadyOpen(State); } @@ -808,7 +811,8 @@ internal virtual bool TryOpenConnection( internal virtual bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { throw ADP.MethodNotImplemented(); } @@ -910,7 +914,8 @@ protected virtual void ReleaseAdditionalLocksForClose(bool lockToken) protected bool TryOpenConnectionInternal( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { // ?->Connecting: prevent set_ConnectionString during Open if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this)) @@ -919,7 +924,7 @@ protected bool TryOpenConnectionInternal( try { connectionFactory.PermissionDemand(outerConnection); - if (!connectionFactory.TryGetConnection(outerConnection, retry, this, out openConnection)) + if (!connectionFactory.TryGetConnection(outerConnection, retry, this, timeout, out openConnection)) { return false; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 73a993aebf..d71d15da64 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -1945,9 +1945,10 @@ internal void OnLoginAck(SqlLoginAck rec) internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { - return TryOpenConnectionInternal(outerConnection, connectionFactory, retry); + return TryOpenConnectionInternal(outerConnection, connectionFactory, retry, timeout); } internal void ValidateConnectionForExecute(SqlCommand command) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index c4cfd5bfa7..252b1bddb7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -224,7 +224,8 @@ public void PutObjectFromTransactedPool(DbConnectionInternal connection) /// public DbConnectionInternal ReplaceConnection( DbConnection owningObject, - DbConnectionInternal oldConnection) + DbConnectionInternal oldConnection, + TimeoutTimer timeout) { throw new NotImplementedException(); } @@ -281,15 +282,15 @@ public void TransactionEnded(Transaction transaction, DbConnectionInternal trans public bool TryGetConnection( DbConnection owningObject, TaskCompletionSource? taskCompletionSource, + TimeoutTimer timeout, out DbConnectionInternal? connection) { - // Create a single TimeoutTimer that represents the overall connection timeout budget. - // This timer is threaded through all layers (pool wait, physical connection creation) so that + // The TimeoutTimer is provided by the caller and represents the overall connection timeout + // budget. It is threaded through all layers (pool wait, physical connection creation) so that // time spent waiting in the pool is deducted from the budget available for physical connection // establishment. Without this, each layer would start its own fresh timeout and the total wait // could exceed the configured ConnectTimeout. // Note: TimeoutTimer treats 0 seconds as infinite timeout, which matches ConnectTimeout=0 semantics. - var timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(owningObject.ConnectionTimeout)); // If taskCompletionSource is null, we are in a sync context. if (taskCompletionSource is null) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs index 43dae3fa0f..2ae8e8ea98 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs @@ -119,17 +119,20 @@ internal interface IDbConnectionPool /// The SqlConnection that will own this internal connection. /// Used when calling this method in an async context. /// The internal connection will be set on completion source rather than passed out via the out parameter. + /// The overall timeout budget for this connection request. Time spent waiting in + /// the pool is deducted from the budget available for physical connection creation. /// The retrieved connection will be passed out via this parameter. /// True if a connection was set in the out parameter, otherwise returns false. - bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, out DbConnectionInternal? connection); + bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, TimeoutTimer timeout, out DbConnectionInternal? connection); /// /// Replaces the internal connection currently associated with owningObject with a new internal connection from the pool. /// /// The connection whose internal connection should be replaced. /// The internal connection currently associated with the owning object. + /// The overall timeout budget for this connection request. /// A reference to the new DbConnectionInternal. - DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection); + DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout); /// /// Returns an internal connection to the pool. 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 e0ef45b51f..50a0fad166 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 @@ -61,15 +61,17 @@ internal sealed class WaitHandleDbConnectionPool : IDbConnectionPool private sealed class PendingGetConnection { - public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSource completion) + public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSource completion, TimeoutTimer timeout) { DueTime = dueTime; Owner = owner; Completion = completion; + Timeout = timeout; } public long DueTime { get; private set; } public DbConnection Owner { get; private set; } public TaskCompletionSource Completion { get; private set; } + public TimeoutTimer Timeout { get; private set; } } private sealed class PoolWaitHandles @@ -520,18 +522,12 @@ private bool IsBlockingPeriodEnabled() } } - private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectionInternal oldConnection) + private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout) { DbConnectionInternal newObj = null; try { - // Legacy pool: start a fresh per-acquisition timeout from the owning connection's - // ConnectionTimeout. Time spent waiting for an idle slot in this pool is tracked - // independently and is not deducted from this budget. - TimeoutTimer timeout = TimeoutTimer.StartNew( - TimeSpan.FromSeconds(owningObject.ConnectionTimeout)); - newObj = _connectionFactory.CreatePooledConnection( owningObject, this, @@ -841,6 +837,7 @@ private void WaitForPendingOpen() delay, allowCreate: true, onlyOneCheckConnection: false, + next.Timeout, out connection); } // @TODO: CER Exception Handling was removed here (see GH#3581) @@ -878,7 +875,7 @@ private void WaitForPendingOpen() } while (_pendingOpens.TryPeek(out next)); } - public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, out DbConnectionInternal connection) + public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, TimeoutTimer timeout, out DbConnectionInternal connection) { uint waitForMultipleObjectsTimeout = 0; bool allowCreate = false; @@ -904,7 +901,7 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource {0}, Creating new connection.", Id); try { - obj = UserCreateRequest(owningObject); + obj = UserCreateRequest(owningObject, timeout); } catch { @@ -1047,7 +1045,7 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj if (semaphoreHolder.Obtained) { SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Creating new connection.", Id); - obj = UserCreateRequest(owningObject); + obj = UserCreateRequest(owningObject, timeout); } else { @@ -1134,11 +1132,12 @@ private void PrepareConnection(DbConnection owningObject, DbConnectionInternal o /// /// Outer connection that currently owns /// Inner connection that will be replaced + /// Overall timeout budget for this connection request. /// A new inner connection that is attached to the - public DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection) + public DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout) { SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, replacing connection.", Id); - DbConnectionInternal newConnection = UserCreateRequest(owningObject, oldConnection); + DbConnectionInternal newConnection = UserCreateRequest(owningObject, timeout, oldConnection); if (newConnection != null) { @@ -1278,7 +1277,12 @@ private void PoolCreateRequest(object state) { try { - newObj = CreateObject(owningObject: null, oldConnection: null); + // Pool replenishment runs on a background worker without an + // owning Open() call, so use a fresh per-attempt timeout based on + // the pool's CreationTimeout (matches the original behavior). + TimeoutTimer replenishTimeout = TimeoutTimer.StartNew( + TimeSpan.FromMilliseconds(CreationTimeout)); + newObj = CreateObject(owningObject: null, oldConnection: null, timeout: replenishTimeout); } catch { @@ -1522,7 +1526,7 @@ public void TransactionEnded(Transaction transaction, DbConnectionInternal trans } } - private DbConnectionInternal UserCreateRequest(DbConnection owningObject, DbConnectionInternal oldConnection = null) + private DbConnectionInternal UserCreateRequest(DbConnection owningObject, TimeoutTimer timeout, DbConnectionInternal oldConnection = null) { // called by user when they were not able to obtain a free object but // instead obtained creation mutex @@ -1542,7 +1546,7 @@ private DbConnectionInternal UserCreateRequest(DbConnection owningObject, DbConn // 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); + obj = CreateObject(owningObject, oldConnection, timeout); } } return obj; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index 25614661de..92d5990a04 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2226,16 +2226,26 @@ private bool TryOpen(TaskCompletionSource retry, SqlConnec private bool TryOpenInner(TaskCompletionSource retry) { + // Create a single TimeoutTimer that represents the overall connection-open budget for this + // attempt. The timer is threaded through every layer (inner connection state transitions, + // connection factory, pool wait, physical connection establishment) so that all of those + // costs are charged against the same budget. This prevents the cumulative wait from exceeding + // the configured ConnectTimeout. A fresh timer is created per TryOpen call so that each + // retry attempt gets its own budget, matching the existing behavior. + // Note: TimeoutTimer treats 0 seconds as infinite timeout, which matches ConnectTimeout=0 semantics. + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(ConnectionOptions?.ConnectTimeout ?? ADP.DefaultConnectionTimeout)); + if (ForceNewConnection) { - if (!InnerConnection.TryReplaceConnection(this, ConnectionFactory, retry)) + if (!InnerConnection.TryReplaceConnection(this, ConnectionFactory, retry, timeout)) { return false; } } else { - if (!InnerConnection.TryOpenConnection(this, ConnectionFactory, retry)) + if (!InnerConnection.TryOpenConnection(this, ConnectionFactory, retry, timeout)) { return false; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs index 078bd6f8d0..4cdee8bdc2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs @@ -115,16 +115,12 @@ internal DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo(SqlConnec internal DbConnectionInternal CreateNonPooledConnection( DbConnection owningConnection, - DbConnectionPoolGroup poolGroup) + DbConnectionPoolGroup poolGroup, + TimeoutTimer timeout) { Debug.Assert(owningConnection is not null, "null owningConnection?"); Debug.Assert(poolGroup is not null, "null poolGroup?"); - // Non-pooled connections start a fresh timeout from the owning connection's - // ConnectionTimeout since there is no preceding pool wait to account for. - TimeoutTimer timeout = TimeoutTimer.StartNew( - TimeSpan.FromSeconds(owningConnection.ConnectionTimeout)); - DbConnectionInternal newConnection = CreateConnection( poolGroup.ConnectionOptions, poolGroup.PoolKey, @@ -321,6 +317,7 @@ internal bool TryGetConnection( DbConnection owningConnection, TaskCompletionSource retry, DbConnectionInternal oldConnection, + TimeoutTimer timeout, out DbConnectionInternal connection) { Debug.Assert(owningConnection is not null, "null owningConnection?"); @@ -392,6 +389,7 @@ internal bool TryGetConnection( retry, oldConnection, poolGroup, + timeout, cancellationTokenSource); // Place this new task in the slot so any future work will be queued behind it @@ -415,7 +413,7 @@ internal bool TryGetConnection( return false; } - connection = CreateNonPooledConnection(owningConnection, poolGroup); + connection = CreateNonPooledConnection(owningConnection, poolGroup, timeout); SqlClientDiagnostics.Metrics.EnterNonPooledConnection(); } @@ -425,11 +423,11 @@ internal bool TryGetConnection( { Debug.Assert(oldConnection is not DbConnectionClosed, "Force new connection, but there is no old connection"); - connection = connectionPool.ReplaceConnection(owningConnection, oldConnection); + connection = connectionPool.ReplaceConnection(owningConnection, oldConnection, timeout); } else { - if (!connectionPool.TryGetConnection(owningConnection, retry, out connection)) + if (!connectionPool.TryGetConnection(owningConnection, retry, timeout, out connection)) { return false; } @@ -763,6 +761,7 @@ private Task CreateReplaceConnectionContinuation( TaskCompletionSource retry, DbConnectionInternal oldConnection, DbConnectionPoolGroup poolGroup, + TimeoutTimer timeout, CancellationTokenSource cancellationTokenSource) { return task.ContinueWith( @@ -773,7 +772,7 @@ private Task CreateReplaceConnectionContinuation( { ADP.SetCurrentTransaction(retry.Task.AsyncState as System.Transactions.Transaction); - DbConnectionInternal newConnection = CreateNonPooledConnection(owningConnection, poolGroup); + DbConnectionInternal newConnection = CreateNonPooledConnection(owningConnection, poolGroup, timeout); if (oldConnection?.State == ConnectionState.Open) { diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs new file mode 100644 index 0000000000..e2e585c35a --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs @@ -0,0 +1,41 @@ +// 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.Data.Common; +using System.Threading.Tasks; +using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.ConnectionPool; + +namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool +{ + /// + /// Test-only extension methods that adapt legacy 3-arg pool calls to the + /// new -aware signatures by deriving the timer + /// from the owning connection's . + /// + internal static class PoolTestExtensions + { + public static bool TryGetConnection( + this IDbConnectionPool pool, + DbConnection owningObject, + TaskCompletionSource? taskCompletionSource, + out DbConnectionInternal? connection) + { + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(owningObject?.ConnectionTimeout ?? 15)); + return pool.TryGetConnection(owningObject!, taskCompletionSource, timeout, out connection); + } + + public static void ReplaceConnection( + this IDbConnectionPool pool, + DbConnection owningObject, + DbConnectionInternal oldConnection) + { + TimeoutTimer timeout = TimeoutTimer.StartNew( + TimeSpan.FromSeconds(owningObject?.ConnectionTimeout ?? 15)); + pool.ReplaceConnection(owningObject!, oldConnection, timeout); + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs index 521ded14b8..a29abd6bbb 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs @@ -676,12 +676,12 @@ internal class MockDbConnectionPool : IDbConnectionPool public void Clear() => throw new NotImplementedException(); - public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, out DbConnectionInternal? connection) + public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, TimeoutTimer timeout, out DbConnectionInternal? connection) { throw new NotImplementedException(); } - public DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection) + public DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout) { throw new NotImplementedException(); } From c2658d3bd25b02490fd62809248e87b05445b274 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 15:33:01 -0700 Subject: [PATCH 05/39] Inject TimeProvider into TimeoutTimer for deterministic tests Add an overload TimeoutTimer.StartNew(TimeSpan, TimeProvider) and route all clock reads (current time, expiration check, remaining ms, Reset) plus CreateCancellationTokenSource through the supplied TimeProvider. The existing parameterless StartNew now delegates to TimeProvider.System, preserving behavior for production call sites. This lets tests inject Microsoft.Extensions.Time.Testing.FakeTimeProvider and trigger timeout / cancellation behavior by advancing virtual time, removing wall-clock dependence and timing flakiness. - TimeoutTimer: store TimeProvider, derive ExpirationTicks via timeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc() (file-time scale preserved for compatibility with TdsParser consumers); CTS scheduled via new CancellationTokenSource(TimeSpan, TimeProvider). - src.csproj: add Microsoft.Bcl.TimeProvider for the net462 TFM (required for the System.TimeProvider type on .NET Framework). - UnitTests.csproj: add Microsoft.Extensions.TimeProvider.Testing for FakeTimeProvider. - Directory.Packages.props: pin both packages. - Add TimeoutTimerTimeProviderTests covering: System default, null guard, IsExpired/MillisecondsRemaining advancement, Reset, CTS firing on virtual-time advance, already-expired CTS, infinite timer, and the TimeProvider property accessor. --- Directory.Packages.props | 2 + .../src/Microsoft.Data.SqlClient.csproj | 1 + .../Data/ProviderBase/DbConnectionInternal.cs | 10 -- .../Data/ProviderBase/TimeoutTimer.cs | 121 ++++++++++---- .../ConnectionPool/ChannelDbConnectionPool.cs | 7 - .../Microsoft.Data.SqlClient.UnitTests.csproj | 2 + .../TimeoutTimerTimeProviderTests.cs | 155 ++++++++++++++++++ 7 files changed, 252 insertions(+), 46 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 62918f7d65..1d4d0047ce 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -51,6 +51,8 @@ + + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj index cd7fdb4b79..4172f7eac6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj @@ -264,6 +264,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index 9f3f8fcf93..b295f84b3d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -686,16 +686,6 @@ internal void MakePooledConnection(IDbConnectionPool connectionPool) Pool = connectionPool; } - internal virtual void OpenConnection(DbConnection outerConnection, SqlConnectionFactory connectionFactory) - { - TimeoutTimer timeout = TimeoutTimer.StartNew( - TimeSpan.FromSeconds(outerConnection.ConnectionTimeout)); - if (!TryOpenConnection(outerConnection, connectionFactory, null, timeout)) - { - throw ADP.InternalError(ADP.InternalErrorCode.SynchronousConnectReturnedPending); - } - } - internal void PostPop(DbConnection newOwner) { // Called by IDbConnectionPool right after it pulls this from its pool, we take this diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index e40c4e3beb..a21d8d1726 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.Data.Common; using System; using System.Diagnostics; using System.Threading; @@ -18,11 +17,20 @@ namespace Microsoft.Data.ProviderBase /// Intended use: /// /// - /// Call to get a timer with the given expiration point. + /// Call (or the overload that accepts a + /// ) to get a timer with the given expiration point. /// Read the remaining time in the appropriate format to pass to subsystem timeouts. /// Check for timeout via for checks in managed code. /// Simply abandon the instance to the GC when done. /// + /// + /// All time reads (current time and remaining time calculations) and any + /// instances created by this timer flow + /// through the supplied . This allows tests to inject + /// a fake time provider (for example + /// Microsoft.Extensions.Time.Testing.FakeTimeProvider) and deterministically + /// trigger expiration without relying on wall-clock delays. + /// /// internal class TimeoutTimer { @@ -33,23 +41,35 @@ internal class TimeoutTimer /// internal static readonly long InfiniteTimeout = 0; + /// + /// The used to read the current time and to schedule + /// any instances produced by this timer. + /// + private readonly TimeProvider _timeProvider; + #endregion #region Constructors /// /// Initializes a new instance of the class with the - /// specified expiration duration. + /// specified expiration duration and time source. /// /// /// The duration before the timer expires. A value whose ticks equal /// indicates an infinite timeout. /// - private TimeoutTimer(TimeSpan expiration) + /// + /// The used to read the current time and schedule + /// cancellation. Must not be . + /// + private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) { + Debug.Assert(timeProvider is not null, "timeProvider must not be null"); + _timeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; - ExpirationTicks = IsInfinite ? long.MaxValue : checked(ADP.TimerCurrent() + OriginalTicks); + ExpirationTicks = IsInfinite ? long.MaxValue : checked(NowTicks() + OriginalTicks); } #endregion @@ -60,11 +80,18 @@ private TimeoutTimer(TimeSpan expiration) /// Gets the absolute tick value at which this timer is considered expired. /// /// - /// The tick count, in units, at which the timer - /// expires; when is - /// . + /// The tick count, in file-time units (100-nanosecond intervals since + /// 1601-01-01 UTC), at which the timer expires; + /// when is . /// - internal long ExpirationTicks { + /// + /// The tick scale is intentionally compatible with + /// so that legacy callers that compare + /// this value against historic ADP.TimerCurrent() readings continue + /// to function correctly. + /// + internal long ExpirationTicks + { get; //TODO: Remove this when we disable Reset() private set; @@ -75,13 +102,14 @@ internal long ExpirationTicks { /// /// /// if the timer is not infinite and the current time - /// has passed ; otherwise, . + /// (as read from the configured ) has passed + /// ; otherwise, . /// internal bool IsExpired { get { - return !IsInfinite && ADP.TimerHasExpired(ExpirationTicks); + return !IsInfinite && NowTicks() > ExpirationTicks; } } @@ -111,11 +139,6 @@ internal long MillisecondsRemaining { get { - //------------------- - // Preconditions: None - - //------------------- - // Method Body long milliseconds; if (IsInfinite) { @@ -123,15 +146,13 @@ internal long MillisecondsRemaining } else { - milliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); + milliseconds = RemainingMilliseconds(); if (0 > milliseconds) { milliseconds = 0; } } - //-------------------- - // Postconditions Debug.Assert(0 <= milliseconds); // This property guarantees no negative return values return milliseconds; @@ -152,8 +173,6 @@ internal int MillisecondsRemainingInt { get { - //------------------- - // Method Body int milliseconds; if (IsInfinite) { @@ -161,7 +180,7 @@ internal int MillisecondsRemainingInt } else { - long longMilliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); + long longMilliseconds = RemainingMilliseconds(); if (0 > longMilliseconds) { milliseconds = 0; @@ -176,8 +195,6 @@ internal int MillisecondsRemainingInt } } - //-------------------- - // Postconditions Debug.Assert(0 <= milliseconds); return milliseconds; @@ -191,24 +208,51 @@ internal int MillisecondsRemainingInt /// private long OriginalTicks { get; } + /// + /// Gets the used by this timer. Exposed for + /// callers that need to construct related timers or schedule cancellation + /// against the same time source. + /// + internal TimeProvider TimeProvider => _timeProvider; + #endregion #region Methods /// /// Creates and starts a new with the specified - /// expiration duration. + /// expiration duration, using as the time + /// source. + /// + /// + /// The duration before the returned timer expires. A value whose ticks equal + /// produces an infinite timer. + /// + /// A new instance that has already started. + internal static TimeoutTimer StartNew(TimeSpan expiration) + => new TimeoutTimer(expiration, TimeProvider.System); + + /// + /// Creates and starts a new with the specified + /// expiration duration and time source. /// /// /// The duration before the returned timer expires. A value whose ticks equal /// produces an infinite timer. /// + /// + /// The used to read the current time and schedule + /// cancellation. Pass a fake provider in tests to deterministically control + /// expiration. Must not be . + /// /// A new instance that has already started. - internal static TimeoutTimer StartNew(TimeSpan expiration) => new TimeoutTimer(expiration); + internal static TimeoutTimer StartNew(TimeSpan expiration, TimeProvider timeProvider) + => new TimeoutTimer(expiration, timeProvider ?? throw new ArgumentNullException(nameof(timeProvider))); /// /// Creates a new that will be canceled - /// when this timer expires. + /// when this timer expires, using the same the + /// timer was constructed with. /// /// /// A scheduled to cancel after @@ -232,7 +276,10 @@ internal CancellationTokenSource CreateCancellationTokenSource() return cts; } - return new CancellationTokenSource(remaining); + // Route the timer through the configured TimeProvider so that fake + // time providers can advance virtual time and trigger cancellation + // deterministically in tests. + return new CancellationTokenSource(TimeSpan.FromMilliseconds(remaining), _timeProvider); } /// @@ -247,10 +294,26 @@ internal void Reset() { if (!IsInfinite) { - ExpirationTicks = checked(ADP.TimerCurrent() + OriginalTicks); + ExpirationTicks = checked(NowTicks() + OriginalTicks); } } + /// + /// Reads the configured 's current UTC time and + /// returns it as file-time ticks (100-nanosecond intervals since + /// 1601-01-01 UTC). This keeps in the same + /// scale historically produced by DateTime.UtcNow.ToFileTimeUtc(). + /// + private long NowTicks() => _timeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); + + /// + /// Computes the remaining time, in milliseconds, between the configured + /// time source's current reading and . May + /// return a negative value when the timer has already expired. + /// + private long RemainingMilliseconds() + => (ExpirationTicks - NowTicks()) / TimeSpan.TicksPerMillisecond; + #endregion } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 252b1bddb7..45e82eda91 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -285,13 +285,6 @@ public bool TryGetConnection( TimeoutTimer timeout, out DbConnectionInternal? connection) { - // The TimeoutTimer is provided by the caller and represents the overall connection timeout - // budget. It is threaded through all layers (pool wait, physical connection creation) so that - // time spent waiting in the pool is deducted from the budget available for physical connection - // establishment. Without this, each layer would start its own fresh timeout and the total wait - // could exceed the configured ConnectTimeout. - // Note: TimeoutTimer treats 0 seconds as infinite timeout, which matches ConnectTimeout=0 semantics. - // If taskCompletionSource is null, we are in a sync context. if (taskCompletionSource is null) { diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj index 8c59d27572..f00a011b87 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj @@ -47,6 +47,7 @@ + @@ -64,6 +65,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs new file mode 100644 index 0000000000..80262ff8b1 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs @@ -0,0 +1,155 @@ +// 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 System.Threading.Tasks; +using Microsoft.Data.ProviderBase; +using Microsoft.Extensions.Time.Testing; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ProviderBase +{ + /// + /// Verifies that reads time, evaluates expiration, and + /// schedules cancellation through the injected , so tests + /// can deterministically trigger timeout behavior without wall-clock delays. + /// + public class TimeoutTimerTimeProviderTests + { + [Fact] + public void StartNew_DefaultsToSystemTimeProvider() + { + // Sanity check: the parameterless overload still exists and produces a + // non-expired timer that uses real time (so MillisecondsRemaining is + // close to the requested duration). + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromMinutes(1)); + + Assert.False(timer.IsExpired); + Assert.False(timer.IsInfinite); + Assert.InRange(timer.MillisecondsRemaining, 1, TimeSpan.FromMinutes(1).Ticks); + } + + [Fact] + public void StartNew_NullTimeProvider_Throws() + { + Assert.Throws( + () => TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), timeProvider: null!)); + } + + [Fact] + public void IsExpired_ReadsFromInjectedTimeProvider() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); + + Assert.False(timer.IsExpired); + + // Advance virtual time past the expiration; no real time elapses. + fake.Advance(TimeSpan.FromSeconds(6)); + + Assert.True(timer.IsExpired); + Assert.Equal(0, timer.MillisecondsRemainingInt); + } + + [Fact] + public void MillisecondsRemaining_ReflectsVirtualTimeAdvancement() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fake); + + Assert.Equal(10_000, timer.MillisecondsRemainingInt); + + fake.Advance(TimeSpan.FromSeconds(3)); + + Assert.Equal(7_000, timer.MillisecondsRemainingInt); + } + + [Fact] + public void Reset_RestoresOriginalDurationFromInjectedProvider() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); + + fake.Advance(TimeSpan.FromSeconds(4)); + Assert.Equal(1_000, timer.MillisecondsRemainingInt); + + timer.Reset(); + + Assert.Equal(5_000, timer.MillisecondsRemainingInt); + } + + [Fact] + public async Task CreateCancellationTokenSource_FiresWhenFakeTimeAdvances() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(2), fake); + + using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + + Assert.False(cts.IsCancellationRequested); + + // Advancing the fake provider past the expiration deadline must cause + // the CTS to fire deterministically; no real time passes. + fake.Advance(TimeSpan.FromSeconds(3)); + + // FakeTimeProvider schedules timer callbacks on the thread pool, so + // yield briefly to let the cancellation propagate before asserting. + await WaitForAsync(() => cts.IsCancellationRequested); + + Assert.True(cts.IsCancellationRequested); + } + + [Fact] + public void CreateCancellationTokenSource_AlreadyExpired_ReturnsCanceledSource() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); + + fake.Advance(TimeSpan.FromSeconds(2)); + + using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + + Assert.True(cts.IsCancellationRequested); + } + + [Fact] + public void CreateCancellationTokenSource_InfiniteTimer_NeverCancels() + { + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.Zero, fake); + + using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + + fake.Advance(TimeSpan.FromHours(1)); + + Assert.True(timer.IsInfinite); + Assert.False(cts.IsCancellationRequested); + } + + [Fact] + public void TimeProvider_Property_ExposesInjectedProvider() + { + var fake = new FakeTimeProvider(); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); + + Assert.Same(fake, timer.TimeProvider); + } + + // Polls the predicate on a short cadence so test runs aren't sensitive + // to thread-pool scheduling latency when FakeTimeProvider fires its + // registered timer callbacks. + private static async Task WaitForAsync(Func predicate) + { + for (int i = 0; i < 50; i++) + { + if (predicate()) + { + return; + } + await Task.Delay(20); + } + } + } +} From 57708b38551272e3ae999629a46371e2e575ad63 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 15:36:57 -0700 Subject: [PATCH 06/39] Enable nullable. Co-authored-by: Copilot --- .../src/Microsoft/Data/ProviderBase/TimeoutTimer.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index a21d8d1726..d21662c3e5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using System.Diagnostics; using System.Threading; @@ -61,11 +63,10 @@ internal class TimeoutTimer /// /// /// The used to read the current time and schedule - /// cancellation. Must not be . + /// cancellation. /// private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) { - Debug.Assert(timeProvider is not null, "timeProvider must not be null"); _timeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; @@ -243,11 +244,11 @@ internal static TimeoutTimer StartNew(TimeSpan expiration) /// /// The used to read the current time and schedule /// cancellation. Pass a fake provider in tests to deterministically control - /// expiration. Must not be . + /// expiration. /// /// A new instance that has already started. internal static TimeoutTimer StartNew(TimeSpan expiration, TimeProvider timeProvider) - => new TimeoutTimer(expiration, timeProvider ?? throw new ArgumentNullException(nameof(timeProvider))); + => new TimeoutTimer(expiration, timeProvider); /// /// Creates a new that will be canceled From c8342a94baa79d221f48761a676ba61f3314e5a1 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:10:39 -0700 Subject: [PATCH 07/39] Remove low value tests Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 189 ++++++------------ .../TimeoutTimerTimeProviderTests.cs | 7 - 2 files changed, 56 insertions(+), 140 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 3fcc381200..4f7b1116c6 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -12,6 +12,7 @@ using Microsoft.Data.Common.ConnectionString; using Microsoft.Data.ProviderBase; using Microsoft.Data.SqlClient.ConnectionPool; +using Microsoft.Extensions.Time.Testing; using Xunit; namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool @@ -124,23 +125,27 @@ out DbConnectionInternal? internalConnection Assert.NotNull(internalConnection); } - try + // Build a timer backed by a fake time provider, then advance virtual time past + // the timer's expiration so the pool's CancellationTokenSource is created + // already-cancelled and the timeout path fires deterministically without any + // wall-clock wait. + var fakeTime = new FakeTimeProvider(); + TimeoutTimer expiredTimer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fakeTime); + fakeTime.Advance(TimeSpan.FromSeconds(2)); + + // Act & Assert + var ex = Assert.Throws(() => { - // Act - var exceeded = pool.TryGetConnection( - new SqlConnection("Timeout=1"), + pool.TryGetConnection( + new SqlConnection(), taskCompletionSource: null, - out DbConnectionInternal? extraConnection - ); - } - catch (Exception ex) - { - // Assert - Assert.IsType(ex); - Assert.Equal("Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message); - } + expiredTimer, + out DbConnectionInternal? extraConnection); + }); - // Assert + Assert.Equal( + "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", + ex.Message); Assert.Equal(pool.PoolGroupOptions.MaxPoolSize, pool.Count); } @@ -162,25 +167,25 @@ out DbConnectionInternal? internalConnection Assert.NotNull(internalConnection); } - try - { - // Act - TaskCompletionSource taskCompletionSource = new(); - var exceeded = pool.TryGetConnection( - new SqlConnection("Timeout=1"), - taskCompletionSource, - out DbConnectionInternal? extraConnection - ); - await taskCompletionSource.Task; - } - catch (Exception ex) - { - // Assert - Assert.IsType(ex); - Assert.Equal("Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message); - } + // Build a timer backed by a fake time provider then advance past expiration so + // the pool's CTS is created already-cancelled. + var fakeTime = new FakeTimeProvider(); + TimeoutTimer expiredTimer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fakeTime); + fakeTime.Advance(TimeSpan.FromSeconds(2)); - // Assert + // Act & Assert + TaskCompletionSource taskCompletionSource = new(); + pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource, + expiredTimer, + out _); + + var ex = await Assert.ThrowsAsync(() => taskCompletionSource.Task); + + Assert.Equal( + "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", + ex.Message); Assert.Equal(pool.PoolGroupOptions.MaxPoolSize, pool.Count); } @@ -1134,80 +1139,6 @@ public void Constructor_WithValidSmallPoolSizes_WorksCorrectly() #region Connection Timeout Awareness Tests - [Fact] - public async Task GetConnectionInfiniteTimeout_ShouldNotTimeoutImmediately() - { - // Arrange: pool at max capacity (1) so the next request must wait - var poolGroupOptions = new DbConnectionPoolGroupOptions( - poolByIdentity: false, - minPoolSize: 0, - maxPoolSize: 1, - creationTimeout: 0, // 0 = infinite timeout - loadBalanceTimeout: 0, - hasTransactionAffinity: true - ); - var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); - - SqlConnection firstOwner = new(); - pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); - Assert.NotNull(firstConnection); - - // Act: request a connection with ConnectTimeout=0 (infinite), then release after a delay - var waiterTask = Task.Run(() => - { - pool.TryGetConnection( - new SqlConnection("Timeout=0"), - taskCompletionSource: null, - out DbConnectionInternal? waitedConnection); - return waitedConnection; - }); - - // Give the waiter time to start blocking, then release the connection - await Task.Delay(TimeSpan.FromSeconds(2)); - Assert.False(waiterTask.IsCompleted, "Waiter should still be waiting (infinite timeout)"); - - pool.ReturnInternalConnection(firstConnection, firstOwner); - var result = await waiterTask; - - // Assert: the waiter should have received the connection without a timeout error - Assert.NotNull(result); - Assert.Same(firstConnection, result); - } - - [Fact] - public async Task GetConnectionAsyncInfiniteTimeout_ShouldNotTimeoutImmediately() - { - // Arrange: pool at max capacity (1) so the next request must wait - var poolGroupOptions = new DbConnectionPoolGroupOptions( - poolByIdentity: false, - minPoolSize: 0, - maxPoolSize: 1, - creationTimeout: 0, // 0 = infinite timeout - loadBalanceTimeout: 0, - hasTransactionAffinity: true - ); - var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); - - SqlConnection firstOwner = new(); - pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); - Assert.NotNull(firstConnection); - - // Act: request a connection with ConnectTimeout=0 (infinite), then release after a delay - var tcs = new TaskCompletionSource(); - pool.TryGetConnection(new SqlConnection("Timeout=0"), tcs, out _); - - // Give the waiter time to start blocking, then release the connection - await Task.Delay(TimeSpan.FromSeconds(2)); - Assert.False(tcs.Task.IsCompleted, "Waiter should still be waiting (infinite timeout)"); - - pool.ReturnInternalConnection(firstConnection, firstOwner); - var result = await tcs.Task; - - // Assert: the waiter should have received the connection without a timeout error - Assert.NotNull(result); - Assert.Same(firstConnection, result); - } - [Fact] public void GetConnectionSlowCreation_ShouldTimeoutAfterCreation() { @@ -1269,13 +1200,21 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); Assert.NotNull(firstConnection); - // Act: two callers with different timeouts both waiting - // Caller A has a 1-second timeout, Caller B has a 5-second timeout + // Use a single fake time provider shared by both callers so we can independently + // expire each caller's timeout via virtual time without any wall-clock waits. + // Build the timers up-front so they are anchored at virtual time t=0. + var fakeTime = new FakeTimeProvider(); + TimeoutTimer timerA = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fakeTime); + TimeoutTimer timerB = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fakeTime); + + // Caller A: 1s virtual timeout, Caller B: 10s virtual timeout. Both run in + // background tasks so the sync pool path can block on the channel as in production. var callerATask = Task.Run(() => { pool.TryGetConnection( - new SqlConnection("Timeout=1"), + new SqlConnection(), taskCompletionSource: null, + timerA, out DbConnectionInternal? connectionA); return connectionA; }); @@ -1283,19 +1222,24 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() var callerBTask = Task.Run(() => { pool.TryGetConnection( - new SqlConnection("Timeout=5"), + new SqlConnection(), taskCompletionSource: null, + timerB, out DbConnectionInternal? connectionB); return connectionB; }); - // Wait for caller A to timeout + // Advance virtual time past A's 1s timeout but well within B's 10s timeout. + // A's CancellationTokenSource fires (cancelling its channel wait), B's does not. + fakeTime.Advance(TimeSpan.FromSeconds(2)); + + // Caller A should observe the timeout var exA = await Assert.ThrowsAsync(() => callerATask); Assert.Equal( "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", exA.Message); - // Caller B should still be waiting (5-second timeout not yet elapsed) + // Caller B should still be waiting (8s of virtual budget remain) Assert.False(callerBTask.IsCompleted, "Caller B should still be waiting"); // Release the connection so caller B can succeed @@ -1378,27 +1322,6 @@ public void GetConnection_TimeoutTimerReflectsPoolWaitTime() "TimeoutTimer should reflect elapsed time since pool entry"); } - /// - /// Verifies that ConnectTimeout=0 (infinite) creates an infinite TimeoutTimer. - /// - [Fact] - public void GetConnection_InfiniteTimeout_ShouldPassInfiniteTimeoutTimer() - { - // Arrange - var factory = new TimeoutCapturingSqlConnectionFactory(); - var pool = ConstructPool(factory); - var owner = new SqlConnection("Timeout=0"); - - // Act - pool.TryGetConnection(owner, taskCompletionSource: null, out DbConnectionInternal? connection); - - // Assert - Assert.NotNull(connection); - Assert.NotNull(factory.CapturedTimeout); - Assert.True(factory.CapturedTimeout.IsInfinite); - Assert.False(factory.CapturedTimeout.IsExpired); - } - #endregion } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs index 80262ff8b1..fde12691bf 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs @@ -31,13 +31,6 @@ public void StartNew_DefaultsToSystemTimeProvider() Assert.InRange(timer.MillisecondsRemaining, 1, TimeSpan.FromMinutes(1).Ticks); } - [Fact] - public void StartNew_NullTimeProvider_Throws() - { - Assert.Throws( - () => TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), timeProvider: null!)); - } - [Fact] public void IsExpired_ReadsFromInjectedTimeProvider() { From 634f350149045cbb921907a11a7eddd9bc51ef00 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:19:32 -0700 Subject: [PATCH 08/39] Remove low value tests. Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 62 ------------------- 1 file changed, 62 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 4f7b1116c6..f7e491905e 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -930,25 +930,6 @@ protected override DbConnectionInternal CreateConnection( } } - internal class SlowSqlConnectionFactory : SqlConnectionFactory - { - private readonly TimeSpan _delay; - - internal SlowSqlConnectionFactory(TimeSpan delay) => _delay = delay; - - protected override DbConnectionInternal CreateConnection( - SqlConnectionOptions options, - ConnectionPoolKey poolKey, - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, - IDbConnectionPool pool, - DbConnection owningConnection, - TimeoutTimer timeout = null) - { - Thread.Sleep(_delay); - return new StubDbConnectionInternal(); - } - } - /// /// A factory that captures the TimeoutTimer passed to CreateConnection for test verification. /// @@ -1139,49 +1120,6 @@ public void Constructor_WithValidSmallPoolSizes_WorksCorrectly() #region Connection Timeout Awareness Tests - [Fact] - public void GetConnectionSlowCreation_ShouldTimeoutAfterCreation() - { - // Arrange: factory that takes 3 seconds to create a connection, but timeout is 1 second - var slowFactory = new SlowSqlConnectionFactory(TimeSpan.FromSeconds(3)); - var pool = ConstructPool(slowFactory); - - // Act & Assert: the creation itself takes 3s but CTS fires at 1s; - // the post-creation cancellation check in ConnectionPoolSlots.Add detects the timeout. - var ex = Assert.Throws(() => - { - pool.TryGetConnection( - new SqlConnection("Timeout=1"), - taskCompletionSource: null, - out DbConnectionInternal? internalConnection); - }); - - Assert.Equal( - "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", - ex.Message); - } - - [Fact] - public async Task GetConnectionAsyncSlowCreation_ShouldTimeoutAfterCreation() - { - // Arrange: factory that takes 3 seconds to create a connection, but timeout is 1 second - var slowFactory = new SlowSqlConnectionFactory(TimeSpan.FromSeconds(3)); - var pool = ConstructPool(slowFactory); - - // Act & Assert - var tcs = new TaskCompletionSource(); - pool.TryGetConnection( - new SqlConnection("Timeout=1"), - tcs, - out _); - - var ex = await Assert.ThrowsAsync(() => tcs.Task); - - Assert.Equal( - "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", - ex.Message); - } - [Fact] public async Task ConcurrentCallers_ShouldTimeoutIndependently() { From 3a7fa1e2430d4501f4741b21eca30c6d074486e7 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:33:32 -0700 Subject: [PATCH 09/39] Inline timer creation in tests. Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 44 ++++++++++++++++--- .../ConnectionPool/PoolTestExtensions.cs | 41 ----------------- ...itHandleDbConnectionPoolTransactionTest.cs | 2 + 3 files changed, 41 insertions(+), 46 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index f7e491905e..0d9e4b673d 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -64,6 +64,7 @@ public void GetConnectionEmptyPool_ShouldCreateNewConnection(int numConnections) var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -93,6 +94,7 @@ public async Task GetConnectionAsyncEmptyPool_ShouldCreateNewConnection(int numC var completed = pool.TryGetConnection( new SqlConnection(), tcs, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -118,6 +120,7 @@ public void GetConnectionMaxPoolSize_ShouldTimeoutAfterPeriod() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -160,6 +163,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldTimeoutAfterPeriod() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -199,6 +203,7 @@ public async Task GetConnectionMaxPoolSize_ShouldReuseAfterConnectionReleased() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -207,6 +212,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -222,6 +228,7 @@ out DbConnectionInternal? internalConnection var exceeded = pool.TryGetConnection( new SqlConnection(""), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? extraConnection ); return extraConnection; @@ -243,6 +250,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldReuseAfterConnectionReleas pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -251,6 +259,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -264,6 +273,7 @@ out DbConnectionInternal? internalConnection var exceeded = pool.TryGetConnection( new SqlConnection(""), taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? recycledConnection ); pool.ReturnInternalConnection(firstConnection!, firstOwningConnection); @@ -284,6 +294,7 @@ public async Task GetConnectionMaxPoolSize_ShouldRespectOrderOfRequest() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -292,6 +303,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -312,6 +324,7 @@ out DbConnectionInternal? internalConnection pool.TryGetConnection( new SqlConnection(""), null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? recycledConnection ); return recycledConnection; @@ -324,6 +337,7 @@ out DbConnectionInternal? recycledConnection pool.TryGetConnection( new SqlConnection("Timeout=1"), null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)), out DbConnectionInternal? failedConnection ); return failedConnection; @@ -349,6 +363,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldRespectOrderOfRequest() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -357,6 +372,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -371,6 +387,7 @@ out DbConnectionInternal? internalConnection var exceeded = pool.TryGetConnection( new SqlConnection(""), recycledTaskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? recycledConnection ); @@ -380,6 +397,7 @@ out DbConnectionInternal? recycledConnection var exceeded2 = pool.TryGetConnection( new SqlConnection("Timeout=1"), failedCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)), out DbConnectionInternal? failedConnection ); @@ -402,6 +420,7 @@ public void ConnectionsAreReused() var completed1 = pool.TryGetConnection( owningConnection, null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection1 ); @@ -416,6 +435,7 @@ out DbConnectionInternal? internalConnection1 var completed2 = pool.TryGetConnection( owningConnection, null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection2 ); @@ -437,6 +457,7 @@ public void GetConnectionTimeout_ShouldThrowTimeoutException() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); }); @@ -457,6 +478,7 @@ public async Task GetConnectionAsyncTimeout_ShouldThrowTimeoutException() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -481,6 +503,7 @@ public void StressTest() var completed = pool.TryGetConnection( owningObject, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); if (completed) @@ -514,6 +537,7 @@ public void StressTestAsync() var completed = pool.TryGetConnection( owningObject, taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); internalConnection = await taskCompletionSource.Task; @@ -671,7 +695,7 @@ public void TestPutObjectFromTransactedPool() public void TestReplaceConnection() { var pool = ConstructPool(SuccessfulConnectionFactory); - Assert.Throws(() => pool.ReplaceConnection(null!, null!)); + Assert.Throws(() => pool.ReplaceConnection(null!, null!, TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)))); } [Fact] @@ -710,6 +734,7 @@ public void Clear_MultipleIdleConnections_AllAreDestroyed() pool.TryGetConnection( owningConnections[i], taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out internalConnections[i] ); Assert.Equal(0, internalConnections[i]!.ClearGeneration); @@ -738,6 +763,7 @@ public void Clear_BusyConnection_NotDestroyedImmediately() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); @@ -761,6 +787,7 @@ public void Clear_BusyConnectionReturned_IsDestroyed() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); @@ -790,11 +817,13 @@ public void Clear_MixedBusyAndIdle_OnlyIdleDestroyedImmediately() pool.TryGetConnection( busyOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? busyConnection ); pool.TryGetConnection( idleOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? idleConnection ); Assert.NotNull(busyConnection); @@ -827,6 +856,7 @@ public void Clear_NewConnectionsAfterClear_ArePooledNormally() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? oldConnection ); Assert.Equal(0, oldConnection!.ClearGeneration); @@ -840,6 +870,7 @@ out DbConnectionInternal? oldConnection pool.TryGetConnection( newOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? newConnection ); Assert.NotNull(newConnection); @@ -857,6 +888,7 @@ out DbConnectionInternal? newConnection pool.TryGetConnection( reuseOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? reusedConnection ); Assert.Same(newConnection, reusedConnection); @@ -873,6 +905,7 @@ public void Clear_MultipleClearCalls_DoNotCorruptState() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? connection ); Assert.Equal(0, connection!.ClearGeneration); @@ -891,6 +924,7 @@ out DbConnectionInternal? connection pool.TryGetConnection( newOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? newConnection ); Assert.NotNull(newConnection); @@ -1135,7 +1169,7 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() var pool = ConstructPool(SuccessfulConnectionFactory, poolGroupOptions: poolGroupOptions); SqlConnection firstOwner = new(); - pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection); Assert.NotNull(firstConnection); // Use a single fake time provider shared by both callers so we can independently @@ -1202,7 +1236,7 @@ public void GetConnection_ShouldPassTimeoutTimerToFactory() var owner = new SqlConnection("Timeout=30"); // Act - pool.TryGetConnection(owner, taskCompletionSource: null, out DbConnectionInternal? connection); + pool.TryGetConnection(owner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? connection); // Assert: timeout was passed through Assert.NotNull(connection); @@ -1234,7 +1268,7 @@ public void GetConnection_TimeoutTimerReflectsPoolWaitTime() // Fill the pool var firstOwner = new SqlConnection("Timeout=30"); - pool.TryGetConnection(firstOwner, taskCompletionSource: null, out DbConnectionInternal? firstConnection); + pool.TryGetConnection(firstOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? firstConnection); Assert.NotNull(firstConnection); long firstRemainingMs = factory.CapturedTimeout.MillisecondsRemaining; @@ -1249,7 +1283,7 @@ public void GetConnection_TimeoutTimerReflectsPoolWaitTime() var secondFactory = new TimeoutCapturingSqlConnectionFactory(); // We can't swap factories, but the existing factory captures. Reset. var secondOwner = new SqlConnection("Timeout=30"); - pool.TryGetConnection(secondOwner, taskCompletionSource: null, out DbConnectionInternal? secondConnection); + pool.TryGetConnection(secondOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? secondConnection); // Assert: connection obtained, remaining time is less because pool wait consumed time Assert.NotNull(secondConnection); diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs deleted file mode 100644 index e2e585c35a..0000000000 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolTestExtensions.cs +++ /dev/null @@ -1,41 +0,0 @@ -// 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.Data.Common; -using System.Threading.Tasks; -using Microsoft.Data.ProviderBase; -using Microsoft.Data.SqlClient.ConnectionPool; - -namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool -{ - /// - /// Test-only extension methods that adapt legacy 3-arg pool calls to the - /// new -aware signatures by deriving the timer - /// from the owning connection's . - /// - internal static class PoolTestExtensions - { - public static bool TryGetConnection( - this IDbConnectionPool pool, - DbConnection owningObject, - TaskCompletionSource? taskCompletionSource, - out DbConnectionInternal? connection) - { - TimeoutTimer timeout = TimeoutTimer.StartNew( - TimeSpan.FromSeconds(owningObject?.ConnectionTimeout ?? 15)); - return pool.TryGetConnection(owningObject!, taskCompletionSource, timeout, out connection); - } - - public static void ReplaceConnection( - this IDbConnectionPool pool, - DbConnection owningObject, - DbConnectionInternal oldConnection) - { - TimeoutTimer timeout = TimeoutTimer.StartNew( - TimeSpan.FromSeconds(owningObject?.ConnectionTimeout ?? 15)); - pool.ReplaceConnection(owningObject!, oldConnection, timeout); - } - } -} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index d6c6672660..93a70d5b3f 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -82,6 +82,7 @@ private DbConnectionInternal GetConnection(SqlConnection owner) _pool.TryGetConnection( owner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? connection); return connection!; } @@ -94,6 +95,7 @@ private async Task GetConnectionAsync( _pool.TryGetConnection( owner, taskCompletionSource: tcs, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? connection); return connection ?? await tcs.Task; } From fc9e85f0fb2a9b84914a9ecec36e1e002e7264a4 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:52:07 -0700 Subject: [PATCH 10/39] test cleanup Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 10 +++--- ...itHandleDbConnectionPoolTransactionTest.cs | 2 -- ...meProviderTests.cs => TimeoutTimerTest.cs} | 31 ++++++------------- 3 files changed, 13 insertions(+), 30 deletions(-) rename src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/{TimeoutTimerTimeProviderTests.cs => TimeoutTimerTest.cs} (78%) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 0d9e4b673d..a16dfc49be 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -935,7 +935,6 @@ out DbConnectionInternal? newConnection #endregion #region Test classes -#nullable disable internal class SuccessfulSqlConnectionFactory : SqlConnectionFactory { protected override DbConnectionInternal CreateConnection( @@ -958,7 +957,7 @@ protected override DbConnectionInternal CreateConnection( DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection, - TimeoutTimer timeout = null) + TimeoutTimer timeout) { throw ADP.PooledOpenTimeout(); } @@ -970,7 +969,7 @@ protected override DbConnectionInternal CreateConnection( internal class TimeoutCapturingSqlConnectionFactory : SqlConnectionFactory { private readonly TimeSpan _delay; - internal TimeoutTimer CapturedTimeout { get; private set; } + internal TimeoutTimer? CapturedTimeout { get; private set; } internal TimeoutCapturingSqlConnectionFactory(TimeSpan delay = default) => _delay = delay; @@ -980,7 +979,7 @@ protected override DbConnectionInternal CreateConnection( DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection, - TimeoutTimer timeout = null) + TimeoutTimer timeout) { CapturedTimeout = timeout; if (_delay > TimeSpan.Zero) @@ -990,7 +989,6 @@ protected override DbConnectionInternal CreateConnection( return new StubDbConnectionInternal(); } } -#nullable restore internal class StubDbConnectionInternal : DbConnectionInternal { @@ -1270,7 +1268,7 @@ public void GetConnection_TimeoutTimerReflectsPoolWaitTime() var firstOwner = new SqlConnection("Timeout=30"); pool.TryGetConnection(firstOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? firstConnection); Assert.NotNull(firstConnection); - long firstRemainingMs = factory.CapturedTimeout.MillisecondsRemaining; + long firstRemainingMs = factory.CapturedTimeout!.MillisecondsRemaining; // Return after a short delay so second caller waits Task.Run(async () => diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index 93a70d5b3f..555de74b29 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -897,7 +897,6 @@ public void SequentialTransactions_CanReuseConnections() #region Mock Classes -#nullable disable internal class MockSqlConnectionFactory : SqlConnectionFactory { protected override DbConnectionInternal CreateConnection( @@ -911,7 +910,6 @@ protected override DbConnectionInternal CreateConnection( return new MockDbConnectionInternal(); } } -#nullable restore internal class MockDbConnectionInternal : DbConnectionInternal { diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs similarity index 78% rename from src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs rename to src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index fde12691bf..da4d8ee4ba 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTimeProviderTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -12,27 +12,14 @@ namespace Microsoft.Data.SqlClient.UnitTests.ProviderBase { /// - /// Verifies that reads time, evaluates expiration, and - /// schedules cancellation through the injected , so tests - /// can deterministically trigger timeout behavior without wall-clock delays. + /// Verifies behavior: expiration evaluation, + /// remaining-time reporting, reset, infinite timers, and the cancellation + /// token source it produces. /// - public class TimeoutTimerTimeProviderTests + public class TimeoutTimerTest { [Fact] - public void StartNew_DefaultsToSystemTimeProvider() - { - // Sanity check: the parameterless overload still exists and produces a - // non-expired timer that uses real time (so MillisecondsRemaining is - // close to the requested duration). - TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromMinutes(1)); - - Assert.False(timer.IsExpired); - Assert.False(timer.IsInfinite); - Assert.InRange(timer.MillisecondsRemaining, 1, TimeSpan.FromMinutes(1).Ticks); - } - - [Fact] - public void IsExpired_ReadsFromInjectedTimeProvider() + public void IsExpired_BecomesTrueAfterDuration() { var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); @@ -47,7 +34,7 @@ public void IsExpired_ReadsFromInjectedTimeProvider() } [Fact] - public void MillisecondsRemaining_ReflectsVirtualTimeAdvancement() + public void MillisecondsRemaining_DecreasesAsTimeElapses() { var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fake); @@ -60,7 +47,7 @@ public void MillisecondsRemaining_ReflectsVirtualTimeAdvancement() } [Fact] - public void Reset_RestoresOriginalDurationFromInjectedProvider() + public void Reset_RestoresOriginalDuration() { var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); @@ -74,7 +61,7 @@ public void Reset_RestoresOriginalDurationFromInjectedProvider() } [Fact] - public async Task CreateCancellationTokenSource_FiresWhenFakeTimeAdvances() + public async Task CreateCancellationTokenSource_FiresWhenTimerExpires() { var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(2), fake); @@ -122,7 +109,7 @@ public void CreateCancellationTokenSource_InfiniteTimer_NeverCancels() } [Fact] - public void TimeProvider_Property_ExposesInjectedProvider() + public void TimeProvider_ReturnsProviderPassedToStartNew() { var fake = new FakeTimeProvider(); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); From c9cf6efe4a67b2fe1608a52e4e5948b543c3665d Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:52:28 -0700 Subject: [PATCH 11/39] clarify cancellation token test --- .../ProviderBase/TimeoutTimerTest.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index da4d8ee4ba..6c512183c4 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -60,11 +60,32 @@ public void Reset_RestoresOriginalDuration() Assert.Equal(5_000, timer.MillisecondsRemainingInt); } + /// + /// Verifies that the produced by + /// is wired to the + /// timer's rather than the system clock. + /// + /// + /// The CTS is constructed with a one-hour delay. If it were backed by real + /// time, the test could not complete within the runner's per-test timeout. + /// Because CreateCancellationTokenSource passes the timer's + /// to the CTS constructor, advancing the + /// by two virtual hours synchronously fires + /// the registered timer callback (queued to the thread pool by the fake + /// provider), which cancels the source. The test then polls briefly via + /// to absorb thread-pool dispatch latency + /// before asserting cancellation. A successful run completes in + /// milliseconds, proving cancellation is driven by virtual time and not + /// by wall-clock elapsed time. + /// [Fact] public async Task CreateCancellationTokenSource_FiresWhenTimerExpires() { var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); - TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(2), fake); + // Use an hour-long timer; if the CTS were backed by real time the test + // would never complete in the runner's timeout. It only finishes + // promptly because the CTS is scheduled through the fake provider. + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromHours(1), fake); using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); @@ -72,7 +93,7 @@ public async Task CreateCancellationTokenSource_FiresWhenTimerExpires() // Advancing the fake provider past the expiration deadline must cause // the CTS to fire deterministically; no real time passes. - fake.Advance(TimeSpan.FromSeconds(3)); + fake.Advance(TimeSpan.FromHours(2)); // FakeTimeProvider schedules timer callbacks on the thread pool, so // yield briefly to let the cancellation propagate before asserting. From d6a26e31e836f5597b88508a89053ab6588a7e85 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 16:57:22 -0700 Subject: [PATCH 12/39] Add AAA comments and doc comments to tests. Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 18 ++++- .../ProviderBase/TimeoutTimerTest.cs | 78 +++++++++++++++---- 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index a16dfc49be..f980338d8e 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -1152,6 +1152,18 @@ public void Constructor_WithValidSmallPoolSizes_WorksCorrectly() #region Connection Timeout Awareness Tests + /// + /// Verifies that two concurrent callers waiting for the same exhausted + /// pool observe their own per-caller deadlines + /// independently: the caller with the shorter timeout fails with the + /// pool-timeout error while the caller with the longer timeout continues + /// to wait and eventually succeeds when a connection is returned. + /// + /// + /// Both callers share a single so that + /// advancing virtual time deterministically expires only the short-timeout + /// caller's CTS without consuming any wall-clock time. + /// [Fact] public async Task ConcurrentCallers_ShouldTimeoutIndependently() { @@ -1199,11 +1211,11 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() return connectionB; }); - // Advance virtual time past A's 1s timeout but well within B's 10s timeout. + // Act: advance virtual time past A's 1s timeout but well within B's 10s timeout. // A's CancellationTokenSource fires (cancelling its channel wait), B's does not. fakeTime.Advance(TimeSpan.FromSeconds(2)); - // Caller A should observe the timeout + // Assert: Caller A should observe the timeout var exA = await Assert.ThrowsAsync(() => callerATask); Assert.Equal( "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", @@ -1216,7 +1228,7 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() pool.ReturnInternalConnection(firstConnection, firstOwner); var resultB = await callerBTask; - // Assert: caller B got the connection + // Caller B got the connection Assert.NotNull(resultB); Assert.Same(firstConnection, resultB); } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index 6c512183c4..2b6910399c 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -18,45 +18,68 @@ namespace Microsoft.Data.SqlClient.UnitTests.ProviderBase /// public class TimeoutTimerTest { + /// + /// Verifies that flips from + /// to once the timer's + /// configured duration has elapsed (as measured by its + /// ), and that + /// reports zero in + /// the expired state. + /// [Fact] public void IsExpired_BecomesTrueAfterDuration() { + // Arrange var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); - Assert.False(timer.IsExpired); - // Advance virtual time past the expiration; no real time elapses. + // Act: advance virtual time past the expiration; no real time elapses. fake.Advance(TimeSpan.FromSeconds(6)); + // Assert Assert.True(timer.IsExpired); Assert.Equal(0, timer.MillisecondsRemainingInt); } + /// + /// Verifies that + /// counts down as virtual time advances, matching the original duration + /// minus the elapsed amount. + /// [Fact] public void MillisecondsRemaining_DecreasesAsTimeElapses() { + // Arrange var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fake); - Assert.Equal(10_000, timer.MillisecondsRemainingInt); + // Act fake.Advance(TimeSpan.FromSeconds(3)); + // Assert Assert.Equal(7_000, timer.MillisecondsRemainingInt); } + /// + /// Verifies that restarts the countdown + /// from the original duration, discarding any time that had already + /// elapsed. + /// [Fact] public void Reset_RestoresOriginalDuration() { + // Arrange var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); - fake.Advance(TimeSpan.FromSeconds(4)); Assert.Equal(1_000, timer.MillisecondsRemainingInt); + // Act timer.Reset(); + // Assert Assert.Equal(5_000, timer.MillisecondsRemainingInt); } @@ -81,60 +104,83 @@ public void Reset_RestoresOriginalDuration() [Fact] public async Task CreateCancellationTokenSource_FiresWhenTimerExpires() { + // Arrange: use an hour-long timer; if the CTS were backed by real + // time the test would never complete in the runner's timeout. It + // only finishes promptly because the CTS is scheduled through the + // fake provider. var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); - // Use an hour-long timer; if the CTS were backed by real time the test - // would never complete in the runner's timeout. It only finishes - // promptly because the CTS is scheduled through the fake provider. TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromHours(1), fake); - using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); - Assert.False(cts.IsCancellationRequested); - // Advancing the fake provider past the expiration deadline must cause - // the CTS to fire deterministically; no real time passes. + // Act: advancing the fake provider past the expiration deadline must + // cause the CTS to fire deterministically; no real time passes. fake.Advance(TimeSpan.FromHours(2)); - // FakeTimeProvider schedules timer callbacks on the thread pool, so - // yield briefly to let the cancellation propagate before asserting. + // Assert: FakeTimeProvider schedules timer callbacks on the thread + // pool, so yield briefly to let the cancellation propagate before + // asserting. await WaitForAsync(() => cts.IsCancellationRequested); - Assert.True(cts.IsCancellationRequested); } + /// + /// Verifies that requesting a from + /// a timer whose deadline has already passed returns a source that is + /// already canceled, rather than scheduling a new timer callback. + /// [Fact] public void CreateCancellationTokenSource_AlreadyExpired_ReturnsCanceledSource() { + // Arrange var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); - fake.Advance(TimeSpan.FromSeconds(2)); + // Act using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + // Assert Assert.True(cts.IsCancellationRequested); } + /// + /// Verifies that an infinite timer (constructed from + /// ) produces a + /// that never auto-cancels, even + /// after a large amount of virtual time has elapsed. + /// [Fact] public void CreateCancellationTokenSource_InfiniteTimer_NeverCancels() { + // Arrange var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.Zero, fake); - using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + // Act fake.Advance(TimeSpan.FromHours(1)); + // Assert Assert.True(timer.IsInfinite); Assert.False(cts.IsCancellationRequested); } + /// + /// Verifies that exposes the + /// exact instance supplied to + /// . + /// [Fact] public void TimeProvider_ReturnsProviderPassedToStartNew() { + // Arrange var fake = new FakeTimeProvider(); + + // Act TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); + // Assert Assert.Same(fake, timer.TimeProvider); } From 220e9451641bbd348e82674764855fc4eeb85e7d Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 17:02:12 -0700 Subject: [PATCH 13/39] use auto property Co-authored-by: Copilot --- .../Microsoft/Data/ProviderBase/TimeoutTimer.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index d21662c3e5..df023d9bcd 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -43,12 +43,6 @@ internal class TimeoutTimer /// internal static readonly long InfiniteTimeout = 0; - /// - /// The used to read the current time and to schedule - /// any instances produced by this timer. - /// - private readonly TimeProvider _timeProvider; - #endregion #region Constructors @@ -67,7 +61,7 @@ internal class TimeoutTimer /// private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) { - _timeProvider = timeProvider; + TimeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; ExpirationTicks = IsInfinite ? long.MaxValue : checked(NowTicks() + OriginalTicks); @@ -214,7 +208,7 @@ internal int MillisecondsRemainingInt /// callers that need to construct related timers or schedule cancellation /// against the same time source. /// - internal TimeProvider TimeProvider => _timeProvider; + internal TimeProvider TimeProvider { get; } #endregion @@ -280,7 +274,7 @@ internal CancellationTokenSource CreateCancellationTokenSource() // Route the timer through the configured TimeProvider so that fake // time providers can advance virtual time and trigger cancellation // deterministically in tests. - return new CancellationTokenSource(TimeSpan.FromMilliseconds(remaining), _timeProvider); + return new CancellationTokenSource(TimeSpan.FromMilliseconds(remaining), TimeProvider); } /// @@ -305,7 +299,7 @@ internal void Reset() /// 1601-01-01 UTC). This keeps in the same /// scale historically produced by DateTime.UtcNow.ToFileTimeUtc(). /// - private long NowTicks() => _timeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); + private long NowTicks() => TimeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); /// /// Computes the remaining time, in milliseconds, between the configured From 03ebdf07efd2332a923dba93f7e826a93a49c5cb Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 12 May 2026 17:07:34 -0700 Subject: [PATCH 14/39] Improve test Co-authored-by: Copilot --- .../ChannelDbConnectionPoolTest.cs | 115 +++++------------- 1 file changed, 28 insertions(+), 87 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index f980338d8e..d4a44a98f8 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -937,6 +937,8 @@ out DbConnectionInternal? newConnection #region Test classes internal class SuccessfulSqlConnectionFactory : SqlConnectionFactory { + internal TimeoutTimer? CapturedTimeout { get; private set; } + protected override DbConnectionInternal CreateConnection( SqlConnectionOptions options, ConnectionPoolKey poolKey, @@ -945,6 +947,7 @@ protected override DbConnectionInternal CreateConnection( DbConnection owningConnection, TimeoutTimer timeout) { + CapturedTimeout = timeout; return new StubDbConnectionInternal(); } } @@ -963,33 +966,6 @@ protected override DbConnectionInternal CreateConnection( } } - /// - /// A factory that captures the TimeoutTimer passed to CreateConnection for test verification. - /// - internal class TimeoutCapturingSqlConnectionFactory : SqlConnectionFactory - { - private readonly TimeSpan _delay; - internal TimeoutTimer? CapturedTimeout { get; private set; } - - internal TimeoutCapturingSqlConnectionFactory(TimeSpan delay = default) => _delay = delay; - - protected override DbConnectionInternal CreateConnection( - SqlConnectionOptions options, - ConnectionPoolKey poolKey, - DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, - IDbConnectionPool pool, - DbConnection owningConnection, - TimeoutTimer timeout) - { - CapturedTimeout = timeout; - if (_delay > TimeSpan.Zero) - { - Thread.Sleep(_delay); - } - return new StubDbConnectionInternal(); - } - } - internal class StubDbConnectionInternal : DbConnectionInternal { #region Not Implemented Members @@ -1234,74 +1210,39 @@ public async Task ConcurrentCallers_ShouldTimeoutIndependently() } /// - /// Verifies that the pool passes a non-null TimeoutTimer to the factory's CreateConnection - /// method, and that it reflects the remaining budget (not a fresh full timeout). + /// Verifies that the the pool hands to the + /// connection factory reports a reduced remaining-time budget once the + /// timer's clock has advanced. This guarantees the factory observes the + /// actual remaining budget at the moment of the call rather than a + /// fresh, full timeout. /// + /// + /// Drives elapsed time deterministically with a + /// so the test does not depend on real + /// wall-clock waits or thread sleeps. + /// [Fact] - public void GetConnection_ShouldPassTimeoutTimerToFactory() + public void GetConnection_TimeoutTimerReflectsPoolWaitTime() { - // Arrange: use a capturing factory - var factory = new TimeoutCapturingSqlConnectionFactory(); + // Arrange: a capturing factory and a fake-time-backed timer with a + // 30-second budget anchored at virtual time t = 0. + var factory = new SuccessfulSqlConnectionFactory(); var pool = ConstructPool(factory); var owner = new SqlConnection("Timeout=30"); + var fakeTime = new FakeTimeProvider(); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime); - // Act - pool.TryGetConnection(owner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? connection); + // Act: advance virtual time by 5 seconds before invoking the pool, + // simulating budget that was consumed elsewhere (e.g., waiting on a + // pool slot) before the factory was called. + fakeTime.Advance(TimeSpan.FromSeconds(5)); + pool.TryGetConnection(owner, taskCompletionSource: null, timer, out DbConnectionInternal? connection); - // Assert: timeout was passed through + // Assert: factory received the same timer, and it reports the + // reduced 25-second remaining budget. Assert.NotNull(connection); - Assert.NotNull(factory.CapturedTimeout); - Assert.False(factory.CapturedTimeout.IsExpired); - // The timer should have been started with ~30 seconds, but some time has elapsed - Assert.True(factory.CapturedTimeout.MillisecondsRemaining <= 30_000); - Assert.True(factory.CapturedTimeout.MillisecondsRemaining > 0); - } - - /// - /// Verifies that the TimeoutTimer passed to CreateConnection has reduced remaining time - /// when the pool spends time waiting (e.g., for a slot to free up). - /// - [Fact] - public void GetConnection_TimeoutTimerReflectsPoolWaitTime() - { - // Arrange: pool at max 1, fill it, then return after a delay - var factory = new TimeoutCapturingSqlConnectionFactory(delay: TimeSpan.FromMilliseconds(100)); - var poolGroupOptions = new DbConnectionPoolGroupOptions( - poolByIdentity: false, - minPoolSize: 0, - maxPoolSize: 1, - creationTimeout: 30, - loadBalanceTimeout: 0, - hasTransactionAffinity: true - ); - var pool = ConstructPool(factory, poolGroupOptions: poolGroupOptions); - - // Fill the pool - var firstOwner = new SqlConnection("Timeout=30"); - pool.TryGetConnection(firstOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? firstConnection); - Assert.NotNull(firstConnection); - long firstRemainingMs = factory.CapturedTimeout!.MillisecondsRemaining; - - // Return after a short delay so second caller waits - Task.Run(async () => - { - await Task.Delay(500); - pool.ReturnInternalConnection(firstConnection, firstOwner); - }); - - // Act: second caller must wait for the first to return - var secondFactory = new TimeoutCapturingSqlConnectionFactory(); - // We can't swap factories, but the existing factory captures. Reset. - var secondOwner = new SqlConnection("Timeout=30"); - pool.TryGetConnection(secondOwner, taskCompletionSource: null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(30)), out DbConnectionInternal? secondConnection); - - // Assert: connection obtained, remaining time is less because pool wait consumed time - Assert.NotNull(secondConnection); - // The second call reuses the pool's timer from TryGetConnection, which has been running - // during the ~500ms wait. The factory won't be called for the second request since it - // gets the returned idle connection. But the first call's timer should show time passed. - Assert.True(factory.CapturedTimeout.MillisecondsRemaining < 30_000, - "TimeoutTimer should reflect elapsed time since pool entry"); + Assert.Same(timer, factory.CapturedTimeout); + Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); } #endregion From ad0649da81e26db73533945ba545f7c163d0b343 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 13 May 2026 14:26:07 -0700 Subject: [PATCH 15/39] Walk back some refactors to reduce changes. --- .../Data/ProviderBase/TimeoutTimer.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index df023d9bcd..5f77ddd61f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -7,6 +7,7 @@ using System; using System.Diagnostics; using System.Threading; +using Microsoft.Data.Common; namespace Microsoft.Data.ProviderBase { @@ -64,7 +65,7 @@ private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) TimeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; - ExpirationTicks = IsInfinite ? long.MaxValue : checked(NowTicks() + OriginalTicks); + ExpirationTicks = IsInfinite ? long.MaxValue : checked(ADP.TimerCurrent() + OriginalTicks); } #endregion @@ -104,7 +105,7 @@ internal bool IsExpired { get { - return !IsInfinite && NowTicks() > ExpirationTicks; + return !IsInfinite && ADP.TimerHasExpired(ExpirationTicks); } } @@ -141,7 +142,7 @@ internal long MillisecondsRemaining } else { - milliseconds = RemainingMilliseconds(); + milliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); if (0 > milliseconds) { milliseconds = 0; @@ -175,7 +176,7 @@ internal int MillisecondsRemainingInt } else { - long longMilliseconds = RemainingMilliseconds(); + long longMilliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); if (0 > longMilliseconds) { milliseconds = 0; @@ -289,7 +290,7 @@ internal void Reset() { if (!IsInfinite) { - ExpirationTicks = checked(NowTicks() + OriginalTicks); + ExpirationTicks = checked(ADP.TimerCurrent() + OriginalTicks); } } @@ -301,14 +302,6 @@ internal void Reset() /// private long NowTicks() => TimeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); - /// - /// Computes the remaining time, in milliseconds, between the configured - /// time source's current reading and . May - /// return a negative value when the timer has already expired. - /// - private long RemainingMilliseconds() - => (ExpirationTicks - NowTicks()) / TimeSpan.TicksPerMillisecond; - #endregion } } From 635ddcf90670ab581145e122766dbdcb63fe16c3 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 13 May 2026 14:39:22 -0700 Subject: [PATCH 16/39] move dependencies and use latest versions. --- Directory.Packages.props | 3 +-- src/Microsoft.Data.SqlClient/tests/Directory.Packages.props | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 1d4d0047ce..3ed7216425 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -51,8 +51,6 @@ - - @@ -91,6 +89,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/tests/Directory.Packages.props b/src/Microsoft.Data.SqlClient/tests/Directory.Packages.props index 72bddcc898..7925952d11 100644 --- a/src/Microsoft.Data.SqlClient/tests/Directory.Packages.props +++ b/src/Microsoft.Data.SqlClient/tests/Directory.Packages.props @@ -4,6 +4,7 @@ + From c18c227d77b08ebcdbe12b177050d7c3a98f6e04 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 13 May 2026 15:48:19 -0700 Subject: [PATCH 17/39] Add missing dependency. --- .../src/Microsoft.Data.SqlClient.csproj | 1 + .../src/Microsoft/Data/ProviderBase/TimeoutTimer.cs | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj index 4172f7eac6..e8ccef931e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj @@ -283,6 +283,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 5f77ddd61f..bf55b6f3e7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -7,6 +7,7 @@ using System; using System.Diagnostics; using System.Threading; +using System.Threading.Tasks; using Microsoft.Data.Common; namespace Microsoft.Data.ProviderBase @@ -275,7 +276,9 @@ internal CancellationTokenSource CreateCancellationTokenSource() // Route the timer through the configured TimeProvider so that fake // time providers can advance virtual time and trigger cancellation // deterministically in tests. - return new CancellationTokenSource(TimeSpan.FromMilliseconds(remaining), TimeProvider); + // Use the extension method rather than the CancellationTokenSource + // constructor overload, which doesn't exist on .NET Framework. + return TimeProvider.CreateCancellationTokenSource(TimeSpan.FromMilliseconds(remaining)); } /// From 7e241b1c7b0e1e009f6f6b6d14be22c6282a5b84 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 10:00:09 -0700 Subject: [PATCH 18/39] Add timeprovider package to manual tests. --- .../ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj index 8ea8a7cecc..565591e4b6 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj @@ -355,6 +355,7 @@ + @@ -385,6 +386,7 @@ + From fed27de52007eb94f115b63b6131d0dc4adff7bc Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 13:20:42 -0700 Subject: [PATCH 19/39] Avoid using ADP time methods. Co-authored-by: Copilot --- .../Data/ProviderBase/TimeoutTimer.cs | 19 +++-- .../ProviderBase/TimeoutTimerTest.cs | 85 +++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index bf55b6f3e7..8dae78da94 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -8,7 +8,6 @@ using System.Diagnostics; using System.Threading; using System.Threading.Tasks; -using Microsoft.Data.Common; namespace Microsoft.Data.ProviderBase { @@ -66,7 +65,7 @@ private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) TimeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; - ExpirationTicks = IsInfinite ? long.MaxValue : checked(ADP.TimerCurrent() + OriginalTicks); + ExpirationTicks = IsInfinite ? long.MaxValue : checked(NowTicks() + OriginalTicks); } #endregion @@ -106,7 +105,7 @@ internal bool IsExpired { get { - return !IsInfinite && ADP.TimerHasExpired(ExpirationTicks); + return !IsInfinite && NowTicks() > ExpirationTicks; } } @@ -143,7 +142,7 @@ internal long MillisecondsRemaining } else { - milliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); + milliseconds = TicksToMilliseconds(ExpirationTicks - NowTicks()); if (0 > milliseconds) { milliseconds = 0; @@ -177,7 +176,7 @@ internal int MillisecondsRemainingInt } else { - long longMilliseconds = ADP.TimerRemainingMilliseconds(ExpirationTicks); + long longMilliseconds = TicksToMilliseconds(ExpirationTicks - NowTicks()); if (0 > longMilliseconds) { milliseconds = 0; @@ -293,7 +292,7 @@ internal void Reset() { if (!IsInfinite) { - ExpirationTicks = checked(ADP.TimerCurrent() + OriginalTicks); + ExpirationTicks = checked(NowTicks() + OriginalTicks); } } @@ -303,7 +302,13 @@ internal void Reset() /// 1601-01-01 UTC). This keeps in the same /// scale historically produced by DateTime.UtcNow.ToFileTimeUtc(). /// - private long NowTicks() => TimeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); + internal long NowTicks() => TimeProvider.GetUtcNow().UtcDateTime.ToFileTimeUtc(); + + /// + /// Converts a tick count (100-nanosecond intervals) to milliseconds, matching + /// the conversion historically performed by ADP.TimerToMilliseconds. + /// + internal static long TicksToMilliseconds(long ticks) => ticks / TimeSpan.TicksPerMillisecond; #endregion } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index 2b6910399c..3519dcdcf3 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -5,6 +5,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using Microsoft.Data.Common; using Microsoft.Data.ProviderBase; using Microsoft.Extensions.Time.Testing; using Xunit; @@ -198,5 +199,89 @@ private static async Task WaitForAsync(Func predicate) await Task.Delay(20); } } + + /// + /// Verifies that the wall-clock reading the timer derives from + /// matches the legacy + /// reading. Both are expected to return + /// UTC "now" expressed in file-time ticks (100 ns since 1601-01-01 UTC), + /// so two back-to-back samples should differ by no more than a small + /// scheduling jitter. + /// + [Fact] + public void SystemTimeProvider_AgreesWithAdpTimerCurrent() + { + // 50 ms in file-time ticks. Generous enough to absorb GC pauses + // and CI jitter while still being far smaller than any meaningful + // timeout this class is used for. + const long ToleranceTicks = 50 * TimeSpan.TicksPerMillisecond; + + // Sample both clocks back-to-back, then bracket the TimeoutTimer + // reading between two ADP readings. + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)); + long adpBefore = ADP.TimerCurrent(); + long providerNow = timer.NowTicks(); + long adpAfter = ADP.TimerCurrent(); + + Assert.InRange(providerNow, adpBefore - ToleranceTicks, adpAfter + ToleranceTicks); + } + + /// + /// Verifies the same equivalence end-to-end: a timer started with + /// places its ExpirationTicks + /// at ADP.TimerCurrent() + duration within scheduling jitter. + /// This is the relationship legacy callers depend on when comparing + /// TimeoutTimer.ExpirationTicks against . + /// + [Fact] + public void StartNew_WithSystemTimeProvider_ExpirationMatchesAdpClock() + { + const long ToleranceTicks = 50 * TimeSpan.TicksPerMillisecond; + TimeSpan duration = TimeSpan.FromSeconds(30); + + long adpBefore = ADP.TimerCurrent(); + TimeoutTimer timer = TimeoutTimer.StartNew(duration); + long adpAfter = ADP.TimerCurrent(); + + Assert.InRange( + timer.ExpirationTicks, + adpBefore + duration.Ticks - ToleranceTicks, + adpAfter + duration.Ticks + ToleranceTicks); + } + + /// + /// Verifies that the new remaining-milliseconds calculation used inside + /// — TicksToMilliseconds(ExpirationTicks - NowTicks()) + /// — produces the same value as the legacy + /// path + /// ((ExpirationTicks - ADP.TimerCurrent()) / TicksPerMillisecond) + /// for the same ExpirationTicks. Exercised across past, near-now, + /// and far-future expirations. + /// + [Theory] + [InlineData(-30L * TimeSpan.TicksPerSecond)] // already expired + [InlineData(-TimeSpan.TicksPerMillisecond)] // just expired + [InlineData(0L)] // expiring now + [InlineData(TimeSpan.TicksPerMillisecond)] // 1 ms remaining + [InlineData(TimeSpan.TicksPerSecond)] // 1 s remaining + [InlineData(30L * TimeSpan.TicksPerSecond)] // 30 s remaining + public void RemainingMilliseconds_MatchesAdpTimerRemainingMilliseconds(long offsetTicks) + { + // Build an absolute expiration relative to "now" so both formulas + // see a meaningful target instead of an arbitrary tick value. + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)); + long expirationTicks = timer.NowTicks() + offsetTicks; + + // Legacy formula: subtracts ADP.TimerCurrent() internally. + long legacy = ADP.TimerRemainingMilliseconds(expirationTicks); + + // New formula used by TimeoutTimer: subtracts NowTicks() (which goes + // through TimeProvider.System) and divides via TicksToMilliseconds. + long updated = TimeoutTimer.TicksToMilliseconds(expirationTicks - timer.NowTicks()); + + // The two formulas read the wall clock at slightly different + // instants, so allow ± 1 ms of slip between them. + Assert.InRange(updated, legacy - 1, legacy + 1); + } } } From aad4c949882a7538d1c1103142e07575073a14e8 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 13:45:24 -0700 Subject: [PATCH 20/39] Fix build warning. Co-authored-by: Copilot --- .../UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj index f00a011b87..99bd38ffee 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj @@ -47,7 +47,14 @@ - + + From 21f56968135b6cd012989fd55a9c989499c15a0d Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 14:14:08 -0700 Subject: [PATCH 21/39] Add comments for future work. Co-authored-by: Copilot --- .../Data/SqlClient/Connection/SqlConnectionInternal.cs | 7 +++++++ .../Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index d71d15da64..683302a646 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -3219,6 +3219,10 @@ private void LoginNoFailover( { nextTimeoutInterval = milliseconds; } + + // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. + // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, + // which is currently duplicated in both LoginNoFailover and LoginWithFailover. intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); } @@ -3504,6 +3508,9 @@ private void LoginWithFailover( nextTimeoutInterval = milliseconds; } + // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. + // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, + // which is currently duplicated in both LoginNoFailover and LoginWithFailover. TimeoutTimer intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); // Re-allocate parser each time to make sure state is known. If parser was created diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs index c7d562750a..1accbbdc10 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs @@ -897,6 +897,9 @@ public override uint Receive(out SniPacket packet, int timeoutInMilliseconds) try { + // TODO: convert these to async versions that accept a cancellation token + // this will let us pass fake time providers all the way down the stack + // and easily test timeout behavior. if (timeoutInMilliseconds > 0) { _socket.ReceiveTimeout = timeoutInMilliseconds; From 535e242a8b4e9bf5839534642667da687e01b6c7 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 14:30:11 -0700 Subject: [PATCH 22/39] Address copilot comments and fix tests. Co-authored-by: Copilot --- .../Connection/SqlConnectionInternal.cs | 20 +++++++++++++++++++ .../WaitHandleDbConnectionPool.cs | 20 +++++++++++-------- .../SQL/InstanceNameTest/InstanceNameTest.cs | 12 +++++------ .../ChannelDbConnectionPoolTest.cs | 5 +++++ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 683302a646..0647dcc989 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -3220,6 +3220,16 @@ private void LoginNoFailover( nextTimeoutInterval = milliseconds; } + // Guard against TimeoutTimer's 0-ticks-means-infinite sentinel: + // if the outer timer has no remaining budget, nextTimeoutInterval + // would be 0 and StartNew(TimeSpan.Zero) would produce an infinite + // child timer. Use 1 ms instead so the child timer is finite and + // expires immediately, matching the outer timer's expired state. + if (nextTimeoutInterval == 0) + { + nextTimeoutInterval = 1; + } + // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, // which is currently duplicated in both LoginNoFailover and LoginWithFailover. @@ -3508,6 +3518,16 @@ private void LoginWithFailover( nextTimeoutInterval = milliseconds; } + // Guard against TimeoutTimer's 0-ticks-means-infinite sentinel: + // if the outer timer has no remaining budget, nextTimeoutInterval + // would be 0 and StartNew(TimeSpan.Zero) would produce an infinite + // child timer. Use 1 ms instead so the child timer is finite and + // expires immediately, matching the outer timer's expired state. + if (nextTimeoutInterval == 0) + { + nextTimeoutInterval = 1; + } + // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, // which is currently duplicated in both LoginNoFailover and LoginWithFailover. 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 50a0fad166..32f5ef2626 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 @@ -882,13 +882,13 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource Date: Thu, 14 May 2026 15:01:33 -0700 Subject: [PATCH 23/39] Clean up child timer creation. Co-authored-by: Copilot --- .../Data/ProviderBase/TimeoutTimer.cs | 87 ++++++++++ .../Connection/SqlConnectionInternal.cs | 49 ++---- .../ProviderBase/TimeoutTimerTest.cs | 163 ++++++++++++++++++ 3 files changed, 260 insertions(+), 39 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 8dae78da94..02eb0df638 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -245,6 +245,93 @@ internal static TimeoutTimer StartNew(TimeSpan expiration) internal static TimeoutTimer StartNew(TimeSpan expiration, TimeProvider timeProvider) => new TimeoutTimer(expiration, timeProvider); + /// + /// Creates a new that is already expired, + /// using as the time source. + /// + /// + /// A finite whose is + /// already and whose + /// is zero. + /// + internal static TimeoutTimer StartExpired() + => StartExpired(TimeProvider.System); + + /// + /// Creates a new that is already expired. + /// + /// + /// The used to read the current time and schedule + /// cancellation. + /// + /// + /// A finite whose is + /// already and whose + /// is zero. Useful when a code path needs to hand off an already-exhausted + /// timeout (for example, a child timer whose parent has no remaining + /// budget) without resorting to negative durations or the + /// sentinel. + /// + /// + /// Implemented by anchoring the expiration one tick before "now" on the + /// supplied . The timer is finite, so + /// is . + /// + internal static TimeoutTimer StartExpired(TimeProvider timeProvider) + => new TimeoutTimer(TimeSpan.FromTicks(-1), timeProvider); + + /// + /// Creates and starts a new nested under this + /// (parent) timer. The child shares the parent's + /// and is capped so that it cannot outlast the parent's remaining time. + /// + /// + /// The desired duration of the child timer, interpreted literally — a + /// value of means "expire immediately" and + /// is not treated as the + /// sentinel. A non-positive value yields an already-expired child. + /// + /// + /// A new that uses this timer's + /// . The child is finite unless the parent is + /// infinite, in which case the requested is + /// honored as-is. When the parent is finite, the child's expiration is + /// capped at the parent's remaining time. + /// + /// + /// Behavior matrix: + /// + /// Parent infinite → finite child with the requested duration (or already-expired when ≤ 0). + /// Parent finite, duration longer than parent's remaining → finite child capped at the parent's remaining time. + /// Parent finite, duration shorter than parent's remaining → finite child with the requested duration. + /// Parent finite with no remaining time, or ≤ 0 → already-expired child (see ). + /// + /// To request an infinite child, call + /// directly with ; this method does not + /// produce infinite children. + /// + internal TimeoutTimer StartChild(TimeSpan duration) + { + long requestedMs = (long)duration.TotalMilliseconds; + + // Caller asked for a non-positive duration: already expired. + if (requestedMs <= 0) + { + return StartExpired(TimeProvider); + } + + // Parent finite: cap at parent's remaining time. If the cap leaves + // no time, return an already-expired timer rather than colliding + // with the 0-ticks-means-infinite sentinel. + long childMs = Math.Min(requestedMs, MillisecondsRemaining); + if (childMs <= 0) + { + return StartExpired(TimeProvider); + } + + return new TimeoutTimer(TimeSpan.FromMilliseconds(childMs), TimeProvider); + } + /// /// Creates a new that will be canceled /// when this timer expires, using the same the diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 0647dcc989..6f83e24066 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -3203,7 +3203,6 @@ private void LoginNoFailover( // Set timeout for this attempt, but don't exceed original timer long nextTimeoutInterval = checked(timeoutUnitInterval * multiplier); - long milliseconds = timeout.MillisecondsRemaining; #if NETFRAMEWORK // If it is the first attempt at TNIR connection, then allow at least 500ms for @@ -3215,25 +3214,11 @@ private void LoginNoFailover( } #endif - if (nextTimeoutInterval > milliseconds) - { - nextTimeoutInterval = milliseconds; - } - - // Guard against TimeoutTimer's 0-ticks-means-infinite sentinel: - // if the outer timer has no remaining budget, nextTimeoutInterval - // would be 0 and StartNew(TimeSpan.Zero) would produce an infinite - // child timer. Use 1 ms instead so the child timer is finite and - // expires immediately, matching the outer timer's expired state. - if (nextTimeoutInterval == 0) - { - nextTimeoutInterval = 1; - } - - // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. - // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, - // which is currently duplicated in both LoginNoFailover and LoginWithFailover. - intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); + // StartChild propagates the parent TimeProvider, caps the + // requested duration at the parent's remaining time, and + // returns an already-expired timer when the parent has no + // remaining budget (no 0-means-infinite ambiguity). + intervalTimer = timeout.StartChild(TimeSpan.FromMilliseconds(nextTimeoutInterval)); } // Re-allocate parser each time to make sure state is known. @@ -3512,26 +3497,12 @@ private void LoginWithFailover( { // Set timeout for this attempt, but don't exceed original timer long nextTimeoutInterval = checked(timeoutUnitInterval * ((attemptNumber / 2) + 1)); - long milliseconds = timeout.MillisecondsRemaining; - if (nextTimeoutInterval > milliseconds) - { - nextTimeoutInterval = milliseconds; - } - - // Guard against TimeoutTimer's 0-ticks-means-infinite sentinel: - // if the outer timer has no remaining budget, nextTimeoutInterval - // would be 0 and StartNew(TimeSpan.Zero) would produce an infinite - // child timer. Use 1 ms instead so the child timer is finite and - // expires immediately, matching the outer timer's expired state. - if (nextTimeoutInterval == 0) - { - nextTimeoutInterval = 1; - } - // TODO: Add a StartChild method to TimeoutTimer to propagate the parent TimeProvider down the stack. - // It can also wrap the logic preventing a child timer from exceeding the parent timer's remaining time, - // which is currently duplicated in both LoginNoFailover and LoginWithFailover. - TimeoutTimer intervalTimer = TimeoutTimer.StartNew(TimeSpan.FromMilliseconds(nextTimeoutInterval)); + // StartChild propagates the parent TimeProvider, caps the + // requested duration at the parent's remaining time, and + // returns an already-expired timer when the parent has no + // remaining budget (no 0-means-infinite ambiguity). + TimeoutTimer intervalTimer = timeout.StartChild(TimeSpan.FromMilliseconds(nextTimeoutInterval)); // Re-allocate parser each time to make sure state is known. If parser was created // by previous attempt, dispose it to properly close the socket, if created. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index 3519dcdcf3..7092ea0ee1 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -185,6 +185,169 @@ public void TimeProvider_ReturnsProviderPassedToStartNew() Assert.Same(fake, timer.TimeProvider); } + /// + /// Verifies that + /// returns a finite timer that is already expired, reports zero + /// remaining time, and uses the supplied . + /// + [Fact] + public void StartExpired_ReturnsFiniteAlreadyExpiredTimer() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + + // Act + TimeoutTimer timer = TimeoutTimer.StartExpired(fake); + + // Assert + Assert.False(timer.IsInfinite); + Assert.True(timer.IsExpired); + Assert.Equal(0, timer.MillisecondsRemainingInt); + Assert.Same(fake, timer.TimeProvider); + } + + /// + /// Verifies that the produced by + /// an expired timer is already canceled. + /// + [Fact] + public void StartExpired_CreateCancellationTokenSource_IsAlreadyCanceled() + { + // Arrange + TimeoutTimer timer = TimeoutTimer.StartExpired(new FakeTimeProvider(DateTimeOffset.UtcNow)); + + // Act + using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + + // Assert + Assert.True(cts.IsCancellationRequested); + } + + /// + /// Verifies that propagates the + /// parent's to the child so child timers see + /// the same virtual clock as their parent. + /// + [Fact] + public void StartChild_PropagatesParentTimeProvider() + { + // Arrange + var fake = new FakeTimeProvider(); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fake); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.FromSeconds(5)); + + // Assert + Assert.Same(fake, child.TimeProvider); + } + + /// + /// Verifies that caps the child's + /// duration at the parent's remaining time when the requested duration + /// would otherwise outlast the parent. + /// + [Fact] + public void StartChild_RequestedDurationLongerThanParent_IsCappedAtParentRemaining() + { + // Arrange: parent has 5 s remaining; caller asks for 30 s. + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.FromSeconds(5), fake); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.FromSeconds(30)); + + // Assert: child remaining should match parent remaining (5 s). + Assert.Equal(parent.MillisecondsRemainingInt, child.MillisecondsRemainingInt); + Assert.False(child.IsInfinite); + } + + /// + /// Verifies that uses the requested + /// duration when it is shorter than the parent's remaining time. + /// + [Fact] + public void StartChild_RequestedDurationShorterThanParent_UsesRequested() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fake); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.FromSeconds(5)); + + // Assert + Assert.Equal(5_000, child.MillisecondsRemainingInt); + Assert.False(child.IsInfinite); + } + + /// + /// Verifies that returns an + /// already-expired child when the parent has already expired. + /// + [Fact] + public void StartChild_ParentExpired_ReturnsAlreadyExpiredChild() + { + // Arrange: parent is already expired. + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); + fake.Advance(TimeSpan.FromSeconds(2)); + Assert.True(parent.IsExpired); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.FromSeconds(30)); + + // Assert + Assert.False(child.IsInfinite); + Assert.True(child.IsExpired); + Assert.Equal(0, child.MillisecondsRemainingInt); + } + + /// + /// Verifies that with an infinite + /// parent honors the requested finite duration rather than producing + /// another infinite timer. + /// + [Fact] + public void StartChild_InfiniteParent_UsesRequestedDuration() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.Zero, fake); + Assert.True(parent.IsInfinite); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.FromSeconds(5)); + + // Assert + Assert.False(child.IsInfinite); + Assert.Equal(5_000, child.MillisecondsRemainingInt); + } + + /// + /// Verifies that interprets + /// literally as "expire immediately" + /// rather than as the infinite-timeout sentinel, even when the parent + /// is infinite. + /// + [Fact] + public void StartChild_ZeroDuration_IsLiteralAndReturnsAlreadyExpiredChild() + { + // Arrange: an infinite parent so the only way Zero could become + // "infinite" would be via the sentinel; verify it does not. + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer parent = TimeoutTimer.StartNew(TimeSpan.Zero, fake); + Assert.True(parent.IsInfinite); + + // Act + TimeoutTimer child = parent.StartChild(TimeSpan.Zero); + + // Assert + Assert.False(child.IsInfinite); + Assert.True(child.IsExpired); + Assert.Equal(0, child.MillisecondsRemainingInt); + } + // Polls the predicate on a short cadence so test runs aren't sensitive // to thread-pool scheduling latency when FakeTimeProvider fires its // registered timer callbacks. From dc7bfa1b06e693086c421fceaf1287ccfcb99e42 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 15:10:53 -0700 Subject: [PATCH 24/39] Improve error message checking. Co-authored-by: Copilot --- .../ConnectionPool/ChannelDbConnectionPoolTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index f5cf9dbaf1..1fea8aede1 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -462,7 +462,9 @@ out DbConnectionInternal? internalConnection ); }); - Assert.Equal("Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message); + // Use the resource-backed message rather than a hardcoded English + // string so the assertion stays meaningful under any localized build. + Assert.Equal(ADP.PooledOpenTimeout().Message, ex.Message); } [Fact] @@ -485,7 +487,9 @@ out DbConnectionInternal? internalConnection await taskCompletionSource.Task; }); - Assert.Equal("Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message); + // Use the resource-backed message rather than a hardcoded English + // string so the assertion stays meaningful under any localized build. + Assert.Equal(ADP.PooledOpenTimeout().Message, ex.Message); } [Fact] From 0ed523e8400c6d63840ac2fac103110dcef0daa9 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 14 May 2026 16:21:13 -0700 Subject: [PATCH 25/39] Add legacy pool unit tests for timeout propagation. --- .../WaitHandleDbConnectionPoolBudgetTest.cs | 196 ++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs new file mode 100644 index 0000000000..7a77c733c2 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs @@ -0,0 +1,196 @@ +// 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.Data.Common; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using System.Transactions; +using Microsoft.Data.Common.ConnectionString; +using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.ConnectionPool; +using Microsoft.Extensions.Time.Testing; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool; + +/// +/// Verifies that propagates the +/// caller's overall budget through both the pool +/// wait and the physical connection-creation factory call, mirroring the +/// budget-propagation coverage already in place for +/// ChannelDbConnectionPool. +/// +public class WaitHandleDbConnectionPoolBudgetTest : IDisposable +{ + private const int DefaultMaxPoolSize = 50; + private const int DefaultMinPoolSize = 0; + private const int DefaultCreationTimeoutInMilliseconds = 15_000; + + private WaitHandleDbConnectionPool? _pool; + + public void Dispose() + { + _pool?.Shutdown(); + _pool?.Clear(); + } + + private WaitHandleDbConnectionPool CreatePool( + SqlConnectionFactory connectionFactory, + int maxPoolSize = DefaultMaxPoolSize, + int creationTimeoutMs = DefaultCreationTimeoutInMilliseconds) + { + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: DefaultMinPoolSize, + maxPoolSize: maxPoolSize, + creationTimeout: creationTimeoutMs, + loadBalanceTimeout: 0, + hasTransactionAffinity: true); + + var dbConnectionPoolGroup = new DbConnectionPoolGroup( + new SqlConnectionOptions("Data Source=localhost;"), + new ConnectionPoolKey("TestDataSource", credential: null, accessToken: null, accessTokenCallback: null, sspiContextProvider: null), + poolGroupOptions); + + var pool = new WaitHandleDbConnectionPool( + connectionFactory, + dbConnectionPoolGroup, + DbConnectionPoolIdentity.NoIdentity, + new DbConnectionPoolProviderInfo()); + + pool.Startup(); + _pool = pool; + return pool; + } + + /// + /// Verifies that the the pool hands to the + /// connection factory on the synchronous path reports a reduced + /// remaining-time budget when the timer's clock has advanced before the + /// pool was entered. Mirrors + /// ChannelDbConnectionPoolTest.GetConnection_TimeoutTimerReflectsPoolWaitTime. + /// + [Fact] + public void GetConnection_Sync_TimeoutTimerReflectsTimeAlreadyConsumed() + { + // Arrange: a capturing factory and a fake-time-backed timer with a + // 30-second budget anchored at virtual time t = 0. + var factory = new MockSqlConnectionFactory(); + var pool = CreatePool(factory); + var owner = new SqlConnection("Timeout=30"); + var fakeTime = new FakeTimeProvider(); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime); + + // Act: simulate 5s of budget consumed elsewhere (e.g., higher-level + // Open() work) before the pool is entered. + fakeTime.Advance(TimeSpan.FromSeconds(5)); + bool completed = pool.TryGetConnection( + owner, + taskCompletionSource: null, + timer, + out DbConnectionInternal? connection); + + // Assert: factory received the same timer, and it reports the reduced + // 25-second remaining budget rather than the original 30s or the + // pool's static 15s CreationTimeout. + Assert.True(completed); + Assert.NotNull(connection); + Assert.Same(timer, factory.CapturedTimeout); + Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); + } + + /// + /// Async counterpart of . + /// Verifies that the async pool path also forwards the caller's + /// already-advanced to the factory. + /// + [Fact] + public async Task GetConnection_Async_TimeoutTimerReflectsTimeAlreadyConsumed() + { + // Arrange + var factory = new MockSqlConnectionFactory(); + var pool = CreatePool(factory); + var owner = new SqlConnection("Timeout=30"); + var fakeTime = new FakeTimeProvider(); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime); + var tcs = new TaskCompletionSource(); + + // Act: 5s consumed before entering the pool, then an async request. + fakeTime.Advance(TimeSpan.FromSeconds(5)); + pool.TryGetConnection( + owner, + taskCompletionSource: tcs, + timer, + out DbConnectionInternal? connection); + + // Bound the await so a regression in the pool can't hang the suite. + Task completed = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same(tcs.Task, completed); + DbConnectionInternal result = await tcs.Task; + + // Assert: factory got the caller's timer with the reduced budget. + Assert.NotNull(result); + Assert.Same(timer, factory.CapturedTimeout); + Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); + } + + /// + /// SqlConnectionFactory test double that captures the + /// handed to CreateConnection so tests + /// can assert the pool propagated the caller's budget rather than + /// constructing a fresh timer from CreationTimeout. + /// + internal sealed class MockSqlConnectionFactory : SqlConnectionFactory + { + internal TimeoutTimer? CapturedTimeout { get; private set; } + + protected override DbConnectionInternal CreateConnection( + SqlConnectionOptions options, + ConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, + IDbConnectionPool pool, + DbConnection owningConnection, + TimeoutTimer timeout) + { + CapturedTimeout = timeout; + return new MockDbConnectionInternal(); + } + } + + /// + /// Minimal stub. Mirrors the helper in + /// WaitHandleDbConnectionPoolTransactionTest but is duplicated + /// locally so this test file remains self-contained. + /// + internal sealed class MockDbConnectionInternal : DbConnectionInternal + { + public override string ServerVersion => "Mock"; + + public override DbTransaction BeginTransaction(System.Data.IsolationLevel il) + => throw new NotImplementedException(); + + public override void EnlistTransaction(Transaction? transaction) + { + if (transaction != null) + { + EnlistedTransaction = transaction; + } + } + + protected override void Activate(Transaction? transaction) + { + EnlistedTransaction = transaction; + } + + protected override void Deactivate() + { + } + + internal override void ResetConnection() + { + } + } +} From df97c19c3af5f81a542b2a150dab7df73ad3a2ba Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 20 May 2026 14:57:58 -0700 Subject: [PATCH 26/39] Make test more reliable and add comments --- .../AsyncCancelledConnectionsTest.cs | 136 +++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs index 9dd87d8f21..82234aabae 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs @@ -9,6 +9,58 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests { + /// + /// Stress test that verifies cancelling a command mid-stream does not + /// corrupt the underlying connection, the connection pool, or (when MARS + /// is enabled) the MARS session state machine. + /// + /// + /// For each of parallel tasks, the test: + /// + /// + /// + /// Runs a single "poisoned" command: a 4-batch query with + /// WAITFOR DELAY '00:00:01' between batches, paired with a + /// background task that fires + /// after a random 100-3000 ms delay. + /// The streaming is expected to throw + /// either or a + /// whose message contains + /// "operation cancelled/canceled"; that exception is swallowed. + /// + /// + /// Runs up to follow-up commands on + /// the same physical resources (the same MARS connection when + /// useMars is true, otherwise fresh pooled connections). These + /// must all complete cleanly; any failure here indicates the prior + /// cancellation corrupted shared state. + /// + /// + /// + /// + /// The test asserts implicitly: any exception that is not the expected + /// cancellation is rethrown and fails the test. The known regression + /// signature for a desynchronized MARS framing buffer + /// ("The MARS TDS header contained errors.") is treated as a hard stop + /// via _continue. + /// + /// + /// + /// What this test is not. It is not a coverage test for + /// OpenAsync, Connect Timeout, or pool queue-wait behavior. + /// OpenAsync is treated as pre-work that must succeed so the + /// cancellation/poisoning scenario can run. Because + /// simultaneous opens race against an empty + /// pool whose connection-creation path is serialized internally + /// (WaitHandleDbConnectionPool.WaitForPendingOpen), the caller's + /// per-open Connect Timeout budget must be generous enough to + /// cover queue-wait + physical connect for the last open in the burst. + /// The test therefore overrides Connect Timeout on the builder + /// (see ) rather than relying on the + /// default 15 s; bump that constant if slow CI agents are seeing + /// pool-timeout failures on otherwise healthy runs. + /// + /// public class AsyncCancelledConnectionsTest { /// @@ -21,9 +73,28 @@ public class AsyncCancelledConnectionsTest /// private const int NumberOfNonPoisoned = 10; + /// + /// Per-open Connect Timeout applied to every connection in + /// this test. Sized to comfortably cover the serialized + /// connection-creation queue depth produced by + /// simultaneous opens on slow CI agents. + /// Note: with strict timeout propagation through the pool, the + /// caller's budget covers both pool queue wait and physical connect, + /// so the default 15 s is too tight for this burst pattern. + /// + private const int ConnectTimeoutSeconds = 60; + private bool _continue = true; private Random _random; + /// + /// Drives parallel + /// runs against the configured TCP test server and waits for all of + /// them to complete. The theory matrix toggles MARS so that both the + /// shared-connection (MARS) and per-call-connection (non-MARS) paths + /// are exercised. The test passes if every task either succeeds or + /// fails only with the expected cancellation exception. + /// // Disabled on Azure since this test fails on concurrent runs on same database. [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [InlineData(true)] @@ -33,6 +104,12 @@ public async Task CancelAsyncConnections(bool useMars) // Arrange SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); builder.MultipleActiveResultSets = useMars; + // The pool serializes physical connection creation, and with + // strict Connect Timeout propagation through the pool, the + // caller's budget must cover queue-wait time for the last open + // in a NumberOfTasks-wide burst. Bump the default 15 s budget so + // a slow CI agent doesn't time out legitimately-queued opens. + builder.ConnectTimeout = ConnectTimeoutSeconds; SqlConnection.ClearAllPools(); @@ -50,7 +127,16 @@ public async Task CancelAsyncConnections(bool useMars) // Assert - If test runs to completion, it is successful } - // This is the main body that our Tasks run + /// + /// Body run by each parallel task. When + /// is enabled, opens a single long-lived MARS connection that is + /// reused by every call in this task so that + /// cancellation effects on the shared MARS session are observable. + /// Then runs exactly one poisoned attempt followed by up to + /// non-poisoned attempts (gated by + /// , which is cleared on a MARS-header + /// corruption signature). + /// private async Task DoManyAsync(SqlConnectionStringBuilder connectionStringBuilder) { string connectionString = connectionStringBuilder.ToString(); @@ -71,6 +157,24 @@ private async Task DoManyAsync(SqlConnectionStringBuilder connectionStringBuilde } } + /// + /// Executes one 4-batch query and reads every result set. When + /// is true, the batches are interleaved + /// with WAITFOR DELAY '00:00:01' so the command runs long + /// enough for to cancel it mid-stream; + /// the resulting cancellation exception is expected and swallowed. + /// When is false the command must complete + /// cleanly - this is the assertion that prior cancellation did not + /// corrupt shared state (the MARS session or the pooled connection). + /// + /// Shared MARS connection to reuse when + /// open; otherwise a fresh per-call is + /// opened from . + /// Connection string used for the + /// non-MARS path. + /// If true, schedules a time-bomb + /// and expects the cancellation + /// exception; if false, the command must succeed. private async Task DoOneAsync(SqlConnection marsConnection, string connectionString, bool poison) { // This will do our work, open a connection, and run a query (that returns 4 results sets) @@ -118,6 +222,14 @@ private async Task DoOneAsync(SqlConnection marsConnection, string connectionStr } } + /// + /// Recognizes the two exception shapes that a mid-stream + /// can surface as: a managed + /// , or a + /// whose message reports the server-side + /// "operation cancelled/canceled" error. Any other exception is, by + /// design, treated as a real failure of the test. + /// private static bool IsExpectedCancellation(Exception ex) { switch (ex) @@ -132,6 +244,19 @@ private static bool IsExpectedCancellation(Exception ex) } } + /// + /// Issues on + /// via + /// and drains every + /// row of every result set. When is true a + /// is started in parallel to cancel the + /// command mid-read, and the inner catch (SqlException) + /// deliberately tries to drain remaining result sets after the + /// initial failure - this simulates the realistic dispose-on-error + /// pattern where a caller may attempt cleanup reads on a reader that + /// already faulted, and ensures it doesn't itself wedge the + /// connection. + /// private async Task RunCommand(SqlConnection connection, string commandText, bool poison) { using SqlCommand command = connection.CreateCommand(); @@ -184,6 +309,15 @@ private async Task RunCommand(SqlConnection connection, string commandText, bool } } + /// + /// Waits a random 100-3000 ms and then calls + /// on the supplied command. The + /// randomized delay is intentional: it spreads cancellations across + /// different points in the reader's lifecycle (pre-execute, + /// mid-first-result, between result sets, etc.) to exercise more of + /// the cancellation state machine across the + /// parallel runs. + /// private async Task TimeBombAsync(SqlCommand command) { // Sleep a random amount between 100 and 3000 ms. From 0358280b9ad1c68cc1f667089372cd45c2d04ba0 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 21 May 2026 15:18:05 -0700 Subject: [PATCH 27/39] Doc comment improvements. --- .../Data/ProviderBase/TimeoutTimer.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 02eb0df638..f8b29870b2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -42,7 +42,7 @@ internal class TimeoutTimer /// /// The sentinel value (0) used to indicate an infinite timeout when starting a timer. /// - internal static readonly long InfiniteTimeout = 0; + internal const long InfiniteTimeout = 0; #endregion @@ -60,12 +60,17 @@ internal class TimeoutTimer /// The used to read the current time and schedule /// cancellation. /// + /// + /// Thrown when computing the absolute expiration point in checked arithmetic, + /// if the sum of the current file-time ticks and + /// ticks falls outside the range. + /// private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) { TimeProvider = timeProvider; OriginalTicks = expiration.Ticks; IsInfinite = OriginalTicks == InfiniteTimeout; - ExpirationTicks = IsInfinite ? long.MaxValue : checked(NowTicks() + OriginalTicks); + ExpirationTicks = checked(NowTicks() + OriginalTicks); } #endregion @@ -73,18 +78,17 @@ private TimeoutTimer(TimeSpan expiration, TimeProvider timeProvider) #region Properties /// - /// Gets the absolute tick value at which this timer is considered expired. + /// Gets the tick value at which this timer is considered expired. + /// Do not use this value directly; instead, use to check if the timer has expired. + /// Does not return a meaningful value when the timer is infinite. /// /// /// The tick count, in file-time units (100-nanosecond intervals since - /// 1601-01-01 UTC), at which the timer expires; - /// when is . + /// 1601-01-01 UTC), at which the timer expires. /// /// /// The tick scale is intentionally compatible with - /// so that legacy callers that compare - /// this value against historic ADP.TimerCurrent() readings continue - /// to function correctly. + /// /// internal long ExpirationTicks { @@ -120,7 +124,7 @@ internal bool IsExpired /// /// Gets the number of milliseconds remaining before this timer expires, - /// trimmed to 0 when none remain and to + /// truncated to 0 when none remain, and approximated to /// when the timer is infinite. /// /// @@ -157,7 +161,7 @@ internal long MillisecondsRemaining /// /// Gets the number of milliseconds remaining before this timer expires as - /// a 32-bit integer, trimmed to 0 when none remain and saturated to + /// a 32-bit integer, trimmed to 0 when none remain and approximated to /// when the remaining time exceeds that value or /// when the timer is infinite. /// @@ -306,7 +310,7 @@ internal static TimeoutTimer StartExpired(TimeProvider timeProvider) /// Parent finite, duration shorter than parent's remaining → finite child with the requested duration. /// Parent finite with no remaining time, or ≤ 0 → already-expired child (see ). /// - /// To request an infinite child, call + /// To request a truly infinite timeout, call /// directly with ; this method does not /// produce infinite children. /// From e0161df29bdb7376cd3c9bb85b2172e661487399 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 21 May 2026 15:18:31 -0700 Subject: [PATCH 28/39] Isolate funky TdsParser timeout logic. --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index ded27eeb03..d89f18824e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -542,7 +542,7 @@ bool withFailover } _state = TdsParserState.OpenNotLoggedIn; _physicalStateObj.SniContext = SniContext.Snix_PreLoginBeforeSuccessfulWrite; - _physicalStateObj.TimeoutTime = timeout.ExpirationTicks; + _physicalStateObj.TimeoutTime = timeout.IsInfinite ? long.MaxValue : timeout.ExpirationTicks; bool marsCapable = false; From 70db0227a4c1b1a41dcc8c71bc08dbd363805f7e Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 21 May 2026 15:29:04 -0700 Subject: [PATCH 29/39] Shift logic inside PendingGetConnection --- .../SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 32f5ef2626..1fb07a18be 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 @@ -61,9 +61,9 @@ internal sealed class WaitHandleDbConnectionPool : IDbConnectionPool private sealed class PendingGetConnection { - public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSource completion, TimeoutTimer timeout) + public PendingGetConnection(DbConnection owner, TaskCompletionSource completion, TimeoutTimer timeout) { - DueTime = dueTime; + DueTime = timeout.IsInfinite ? System.Threading.Timeout.Infinite : timeout.ExpirationTicks; Owner = owner; Completion = completion; Timeout = timeout; @@ -917,7 +917,6 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource Date: Thu, 21 May 2026 15:56:04 -0700 Subject: [PATCH 30/39] Add feature switch to enable new timeout lifecycle. --- .../Connection/SqlConnectionInternal.cs | 14 ++++- .../WaitHandleDbConnectionPool.cs | 52 ++++++++++++++----- .../Data/SqlClient/LocalAppContextSwitches.cs | 26 ++++++++++ .../Common/LocalAppContextSwitchesHelper.cs | 15 ++++++ .../SqlClient/LocalAppContextSwitchesTest.cs | 1 + 5 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 6f83e24066..f77e8e9e04 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -400,8 +400,18 @@ internal SqlConnectionInternal( try { - - _timeout = timeout; + // If we want to consider pool operations against the overall connect timeout, + // use the provided timeout. Otherwise, start a fresh timeout to receive the full + // connect timeout. + if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait) + { + _timeout = timeout; + } + else + { + _timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout)); + } + // If transient fault handling is enabled then we can retry the login up to the // ConnectRetryCount. int connectionEstablishCount = applyTransientFaultHandling 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 1fb07a18be..b93ac9b64b 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 @@ -61,9 +61,9 @@ internal sealed class WaitHandleDbConnectionPool : IDbConnectionPool private sealed class PendingGetConnection { - public PendingGetConnection(DbConnection owner, TaskCompletionSource completion, TimeoutTimer timeout) + public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSource completion, TimeoutTimer timeout) { - DueTime = timeout.IsInfinite ? System.Threading.Timeout.Infinite : timeout.ExpirationTicks; + DueTime = dueTime; Owner = owner; Completion = completion; Timeout = timeout; @@ -882,13 +882,26 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource + /// The name of the app context switch that controls whether pool operations + /// should count against the caller's overall ConnectTimeout budget. + /// + private const string UseOverallConnectTimeoutForPoolWaitString = + "Switch.Microsoft.Data.SqlClient.UseOverallConnectTimeoutForPoolWait"; + #if NET && _WINDOWS /// /// The name of the app context switch that controls whether to use the @@ -222,6 +229,11 @@ private enum SwitchValue : byte /// private static SwitchValue s_useConnectionPoolV2 = SwitchValue.None; + /// + /// The cached value of the UseOverallConnectTimeoutForPoolWait switch. + /// + private static SwitchValue s_useOverallConnectTimeoutForPoolWait = SwitchValue.None; + #if NET && _WINDOWS /// /// The cached value of the UseManagedNetworking switch. @@ -539,6 +551,20 @@ public static bool UseCompatibilityAsyncBehaviour defaultValue: false, ref s_useConnectionPoolV2); + /// + /// When set to true, pool operations count against the + /// caller's ConnectTimeout budget. This includes waits and async operations. + /// When false, pool operations receive a full ConnectTimeout and + /// network calls receive a further full ConnectTimeout. + /// + /// The default value of this switch is false. + /// + public static bool UseOverallConnectTimeoutForPoolWait => + AcquireAndReturn( + UseOverallConnectTimeoutForPoolWaitString, + defaultValue: false, + ref s_useOverallConnectTimeoutForPoolWait); + #if NET && _WINDOWS /// /// When set to true, .NET on Windows will use the managed SNI diff --git a/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs b/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs index 8102f47d51..4624b9c835 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs @@ -55,6 +55,7 @@ public sealed class LocalAppContextSwitchesHelper : IDisposable private readonly bool? _useCompatibilityAsyncBehaviourOriginal; private readonly bool? _useCompatibilityProcessSniOriginal; private readonly bool? _useConnectionPoolV2Original; + private readonly bool? _useOverallConnectTimeoutForPoolWaitOriginal; #if NET && _WINDOWS private readonly bool? _useManagedNetworkingOriginal; #endif @@ -114,6 +115,8 @@ public LocalAppContextSwitchesHelper() GetSwitchValue("s_useCompatibilityProcessSni"); _useConnectionPoolV2Original = GetSwitchValue("s_useConnectionPoolV2"); + _useOverallConnectTimeoutForPoolWaitOriginal = + GetSwitchValue("s_useOverallConnectTimeoutForPoolWait"); #if NET && _WINDOWS _useManagedNetworkingOriginal = GetSwitchValue("s_useManagedNetworking"); @@ -178,6 +181,9 @@ public void Dispose() SetSwitchValue( "s_useConnectionPoolV2", _useConnectionPoolV2Original); + SetSwitchValue( + "s_useOverallConnectTimeoutForPoolWait", + _useOverallConnectTimeoutForPoolWaitOriginal); #if NET && _WINDOWS SetSwitchValue( "s_useManagedNetworking", @@ -314,6 +320,15 @@ public bool? UseConnectionPoolV2 set => SetSwitchValue("s_useConnectionPoolV2", value); } + /// + /// Get or set the UseOverallConnectTimeoutForPoolWait switch value. + /// + public bool? UseOverallConnectTimeoutForPoolWait + { + get => GetSwitchValue("s_useOverallConnectTimeoutForPoolWait"); + set => SetSwitchValue("s_useOverallConnectTimeoutForPoolWait", value); + } + #if NET && _WINDOWS /// /// Get or set the UseManagedNetworking switch value. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs index a5db02faa0..81e3c4c77d 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs @@ -26,6 +26,7 @@ public void TestDefaultAppContextSwitchValues() Assert.True(LocalAppContextSwitches.UseCompatibilityProcessSni); Assert.True(LocalAppContextSwitches.UseCompatibilityAsyncBehaviour); Assert.False(LocalAppContextSwitches.UseConnectionPoolV2); + Assert.False(LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait); Assert.False(LocalAppContextSwitches.TruncateScaledDecimal); Assert.False(LocalAppContextSwitches.IgnoreServerProvidedFailoverPartner); Assert.False(LocalAppContextSwitches.EnableMultiSubnetFailoverByDefault); From 16d1133384a722b771214b7cb21fea63a0604c38 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 22 May 2026 14:59:19 -0700 Subject: [PATCH 31/39] Make tests more reliable. Mark one as flaky and add a condition for implicit distributed transactions. --- .../SQL/TransactionTest/DistributedTransactionTest.cs | 5 ++++- .../tests/UnitTests/ProviderBase/TimeoutTimerTest.cs | 5 +++-- .../SimulatedServerTests/ConnectionFailoverTests.cs | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.cs index 28aa62ccf5..9ed25a6e74 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.cs @@ -55,7 +55,10 @@ public async Task Delegated_transaction_deadlock_in_SinglePhaseCommit() public async Task Test_EnlistedTransactionPreservedWhilePooled() { #if NET - TransactionManager.ImplicitDistributedTransactions = true; + if (OperatingSystem.IsWindows()) + { + TransactionManager.ImplicitDistributedTransactions = true; + } #endif await RunTestSet(EnlistedTransactionPreservedWhilePooled); diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index 7092ea0ee1..a104a345e2 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -433,14 +433,15 @@ public void RemainingMilliseconds_MatchesAdpTimerRemainingMilliseconds(long offs // Build an absolute expiration relative to "now" so both formulas // see a meaningful target instead of an arbitrary tick value. TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)); - long expirationTicks = timer.NowTicks() + offsetTicks; + long nowTicks = timer.NowTicks(); + long expirationTicks = nowTicks + offsetTicks; // Legacy formula: subtracts ADP.TimerCurrent() internally. long legacy = ADP.TimerRemainingMilliseconds(expirationTicks); // New formula used by TimeoutTimer: subtracts NowTicks() (which goes // through TimeProvider.System) and divides via TicksToMilliseconds. - long updated = TimeoutTimer.TicksToMilliseconds(expirationTicks - timer.NowTicks()); + long updated = TimeoutTimer.TicksToMilliseconds(expirationTicks - nowTicks); // The two formulas read the wall clock at slightly different // instants, so allow ± 1 ms of slip between them. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs index d3a0bcd07b..a15bfef9b1 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs @@ -477,6 +477,7 @@ public void TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary(uint e Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount); } + [Trait("Category", "flaky")] [Theory] [InlineData(40613)] [InlineData(42108)] From fcb6acfce882887ed256ea563ed5594a2af845c6 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 22 May 2026 15:55:42 -0700 Subject: [PATCH 32/39] Remove flaky test. Improve test coverage of milliseconds remaining. --- .../ProviderBase/TimeoutTimerTest.cs | 118 +++++++++++++----- 1 file changed, 90 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs index a104a345e2..5d98837806 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -413,39 +413,101 @@ public void StartNew_WithSystemTimeProvider_ExpirationMatchesAdpClock() } /// - /// Verifies that the new remaining-milliseconds calculation used inside - /// — TicksToMilliseconds(ExpirationTicks - NowTicks()) - /// — produces the same value as the legacy - /// path - /// ((ExpirationTicks - ADP.TimerCurrent()) / TicksPerMillisecond) - /// for the same ExpirationTicks. Exercised across past, near-now, - /// and far-future expirations. + /// Verifies that + /// (the variant) counts down as virtual time + /// elapses and reports the full original duration when no time has + /// passed yet. /// - [Theory] - [InlineData(-30L * TimeSpan.TicksPerSecond)] // already expired - [InlineData(-TimeSpan.TicksPerMillisecond)] // just expired - [InlineData(0L)] // expiring now - [InlineData(TimeSpan.TicksPerMillisecond)] // 1 ms remaining - [InlineData(TimeSpan.TicksPerSecond)] // 1 s remaining - [InlineData(30L * TimeSpan.TicksPerSecond)] // 30 s remaining - public void RemainingMilliseconds_MatchesAdpTimerRemainingMilliseconds(long offsetTicks) + [Fact] + public void MillisecondsRemaining_Long_DecreasesAsTimeElapses() { - // Build an absolute expiration relative to "now" so both formulas - // see a meaningful target instead of an arbitrary tick value. - TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)); - long nowTicks = timer.NowTicks(); - long expirationTicks = nowTicks + offsetTicks; + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fake); + Assert.Equal(10_000L, timer.MillisecondsRemaining); - // Legacy formula: subtracts ADP.TimerCurrent() internally. - long legacy = ADP.TimerRemainingMilliseconds(expirationTicks); + // Act + fake.Advance(TimeSpan.FromSeconds(3)); - // New formula used by TimeoutTimer: subtracts NowTicks() (which goes - // through TimeProvider.System) and divides via TicksToMilliseconds. - long updated = TimeoutTimer.TicksToMilliseconds(expirationTicks - nowTicks); + // Assert + Assert.Equal(7_000L, timer.MillisecondsRemaining); + } - // The two formulas read the wall clock at slightly different - // instants, so allow ± 1 ms of slip between them. - Assert.InRange(updated, legacy - 1, legacy + 1); + /// + /// Verifies that + /// floors at zero (rather than going negative) once the timer's + /// deadline has passed. + /// + [Fact] + public void MillisecondsRemaining_Long_IsZeroAfterExpiration() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(1), fake); + + // Act + fake.Advance(TimeSpan.FromSeconds(5)); + + // Assert + Assert.True(timer.IsExpired); + Assert.Equal(0L, timer.MillisecondsRemaining); + } + + /// + /// Verifies that + /// reports for an infinite timer, even + /// after a large amount of virtual time has elapsed. + /// + [Fact] + public void MillisecondsRemaining_Long_InfiniteTimer_ReturnsMaxValue() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.Zero, fake); + Assert.True(timer.IsInfinite); + + // Act + fake.Advance(TimeSpan.FromHours(1)); + + // Assert + Assert.Equal(long.MaxValue, timer.MillisecondsRemaining); + } + + /// + /// Verifies that + /// reports for an infinite timer. + /// + [Fact] + public void MillisecondsRemainingInt_InfiniteTimer_ReturnsMaxValue() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.Zero, fake); + Assert.True(timer.IsInfinite); + + // Assert + Assert.Equal(int.MaxValue, timer.MillisecondsRemainingInt); + } + + /// + /// Verifies that + /// saturates at when the remaining time + /// would otherwise overflow a 32-bit integer (anything beyond ~24.8 + /// days), while + /// reports the full value. + /// + [Fact] + public void MillisecondsRemainingInt_LargeDuration_SaturatesAtMaxValue() + { + // Arrange: a finite duration that exceeds int.MaxValue ms. + TimeSpan longDuration = TimeSpan.FromMilliseconds((long)int.MaxValue + 1_000L); + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(longDuration, fake); + + // Assert + Assert.False(timer.IsInfinite); + Assert.Equal(int.MaxValue, timer.MillisecondsRemainingInt); + Assert.Equal((long)longDuration.TotalMilliseconds, timer.MillisecondsRemaining); } } } From c795a724fd1b5a2f66f8d23e6cf04d4b3408aa36 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Tue, 26 May 2026 12:18:01 -0700 Subject: [PATCH 33/39] Remove unused using statements. --- .../ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs index 7a77c733c2..7872c006f5 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs @@ -4,11 +4,8 @@ using System; using System.Data.Common; -using System.Diagnostics; -using System.Threading; using System.Threading.Tasks; using System.Transactions; -using Microsoft.Data.Common.ConnectionString; using Microsoft.Data.ProviderBase; using Microsoft.Data.SqlClient.ConnectionPool; using Microsoft.Extensions.Time.Testing; From 57219fdd97424d2637168b997258a5bf2cd51237 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 10:14:10 -0700 Subject: [PATCH 34/39] Update nuspec --- .../src/Microsoft.Data.SqlClient.nuspec | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec index da3a17bb9c..f634b4a6a8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec @@ -37,6 +37,7 @@ + @@ -55,6 +56,7 @@ + @@ -67,6 +69,7 @@ + @@ -79,6 +82,7 @@ + From fc084635d5fdb7c32abf02d794434bf6761f338a Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 10:27:18 -0700 Subject: [PATCH 35/39] Remove unnecessary references. --- .../ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj index fa84bf0d04..1145fa4790 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj @@ -357,7 +357,6 @@ - @@ -387,7 +386,6 @@ - From 867c9c27fbd96db906d1928538b480c57a1c0438 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 11:57:05 -0700 Subject: [PATCH 36/39] Add tests for timeout evaluation using switch. --- .../Data/ProviderBase/TimeoutTimer.cs | 2 +- .../Connection/SqlConnectionInternal.cs | 28 +++-- .../WaitHandleDbConnectionPool.cs | 51 ++++---- .../WaitHandleDbConnectionPoolBudgetTest.cs | 109 ++++++++++++------ .../SqlConnectionInternalTimeoutTests.cs | 61 ++++++++++ 5 files changed, 183 insertions(+), 68 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index f8b29870b2..8a14645f98 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -206,7 +206,7 @@ internal int MillisecondsRemainingInt /// timer was created. Used by to restore the original /// expiration window. /// - private long OriginalTicks { get; } + internal long OriginalTicks { get; } /// /// Gets the used by this timer. Exposed for diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index f1b7a6ab33..50706b35a2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -403,14 +403,7 @@ internal SqlConnectionInternal( // If we want to consider pool operations against the overall connect timeout, // use the provided timeout. Otherwise, start a fresh timeout to receive the full // connect timeout. - if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait) - { - _timeout = timeout; - } - else - { - _timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout)); - } + _timeout = ResolveLoginTimeout(timeout, connectionOptions.ConnectTimeout); // If transient fault handling is enabled then we can retry the login up to the // ConnectRetryCount. @@ -777,6 +770,25 @@ private SqlInternalTransaction AvailableInternalTransaction // @TODO: Make private field. private bool IsAzureSqlConnection { get; set; } + internal TimeoutTimer Timeout => _timeout; + + /// + /// Selects the that governs the login phase based on + /// . + /// + /// + /// When the switch is enabled the caller-supplied + /// is returned as-is so any time already consumed (e.g., waiting for the pool) counts + /// against the overall ConnectTimeout. When disabled, a fresh timer is started from + /// , preserving legacy behavior. Extracted so + /// this branch can be unit-tested without standing up a real connection. + /// + internal static TimeoutTimer ResolveLoginTimeout(TimeoutTimer callerTimeout, int connectTimeoutSeconds) + { + return LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait + ? callerTimeout + : TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectTimeoutSeconds)); + } #endregion #region Public and Internal Methods 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 b93ac9b64b..109266c27c 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 @@ -875,6 +875,31 @@ private void WaitForPendingOpen() } while (_pendingOpens.TryPeek(out next)); } + /// + /// Resolves the WaitHandle.WaitAny timeout (milliseconds) for a synchronous + /// pool acquire. + /// + /// + /// When + /// is enabled the caller's remaining budget is used so + /// the pool wait participates in the overall ConnectTimeout. Otherwise the legacy + /// behavior is preserved: the static pool CreationTimeout is used, with + /// 0 mapped to . Extracted so this branch can + /// be unit-tested without timing-based assertions. + /// + internal static uint ResolvePoolWaitTimeoutMs(TimeoutTimer timeout, int creationTimeoutMs) + { + if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait) + { + return timeout.IsInfinite + ? unchecked((uint)Timeout.Infinite) + : (uint)timeout.MillisecondsRemainingInt; + } + + uint legacy = (uint)creationTimeoutMs; + return legacy == 0 ? unchecked((uint)Timeout.Infinite) : legacy; + } + public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, TimeoutTimer timeout, out DbConnectionInternal connection) { uint waitForMultipleObjectsTimeout = 0; @@ -882,31 +907,9 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource {0}, DbConnectionInternal State != Running.", Id); connection = null; diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs index 7872c006f5..6c00f8e38e 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs @@ -4,10 +4,12 @@ using System; using System.Data.Common; +using System.Threading; using System.Threading.Tasks; using System.Transactions; using Microsoft.Data.ProviderBase; using Microsoft.Data.SqlClient.ConnectionPool; +using Microsoft.Data.SqlClient.Tests.Common; using Microsoft.Extensions.Time.Testing; using Xunit; @@ -65,73 +67,110 @@ private WaitHandleDbConnectionPool CreatePool( /// /// Verifies that the the pool hands to the - /// connection factory on the synchronous path reports a reduced - /// remaining-time budget when the timer's clock has advanced before the - /// pool was entered. Mirrors + /// connection factory reports a reduced remaining-time budget when the + /// timer's clock has advanced before the pool was entered. Both the + /// synchronous (taskCompletionSource == null) and asynchronous + /// paths must forward the caller's already-advanced timer rather than + /// constructing a fresh one from CreationTimeout. Mirrors /// ChannelDbConnectionPoolTest.GetConnection_TimeoutTimerReflectsPoolWaitTime. /// - [Fact] - public void GetConnection_Sync_TimeoutTimerReflectsTimeAlreadyConsumed() + [Theory] + [InlineData(false)] // sync + [InlineData(true)] // async + public async Task GetConnection_TimeoutTimerReflectsTimeAlreadyConsumed(bool async) { - // Arrange: a capturing factory and a fake-time-backed timer with a + // Arrange: capturing factory and a fake-time-backed timer with a // 30-second budget anchored at virtual time t = 0. var factory = new MockSqlConnectionFactory(); var pool = CreatePool(factory); var owner = new SqlConnection("Timeout=30"); var fakeTime = new FakeTimeProvider(); TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime); + TaskCompletionSource? tcs = + async ? new TaskCompletionSource() : null; // Act: simulate 5s of budget consumed elsewhere (e.g., higher-level - // Open() work) before the pool is entered. + // Open() work) before the pool is entered, then request a connection. fakeTime.Advance(TimeSpan.FromSeconds(5)); bool completed = pool.TryGetConnection( owner, - taskCompletionSource: null, + tcs, timer, out DbConnectionInternal? connection); + if (async) + { + // Bound the await so a regression in the pool can't hang the suite. + Task winner = await Task.WhenAny(tcs!.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same(tcs.Task, winner); + connection = await tcs.Task; + } + else + { + Assert.True(completed); + } + // Assert: factory received the same timer, and it reports the reduced // 25-second remaining budget rather than the original 30s or the // pool's static 15s CreationTimeout. - Assert.True(completed); Assert.NotNull(connection); Assert.Same(timer, factory.CapturedTimeout); Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); } /// - /// Async counterpart of . - /// Verifies that the async pool path also forwards the caller's - /// already-advanced to the factory. + /// Identifies which kind of caller-supplied a + /// parameterized test should construct. Used because + /// cannot carry a live + /// instance. + /// + public enum TimerKind + { + Expired, + Infinite, + } + + /// + /// Verifies the resolution matrix for the synchronous + /// WaitHandle.WaitAny timeout: + /// + /// switch ON → use the caller timer's remaining budget + /// (expired → 0, infinite → ); + /// switch OFF → ignore the caller timer and use + /// CreationTimeout, treating 0 as + /// per legacy behavior. + /// /// - [Fact] - public async Task GetConnection_Async_TimeoutTimerReflectsTimeAlreadyConsumed() + [Theory] + [InlineData(true, TimerKind.Expired, 5_000, 0u)] + [InlineData(true, TimerKind.Infinite, 5_000, unchecked((uint)Timeout.Infinite))] + [InlineData(false, TimerKind.Expired, 1_500, 1_500u)] + [InlineData(false, TimerKind.Expired, 0, unchecked((uint)Timeout.Infinite))] + public void ResolvePoolWaitTimeoutMs_ReturnsExpected( + bool switchEnabled, + TimerKind timerKind, + int creationTimeoutMs, + uint expected) { // Arrange - var factory = new MockSqlConnectionFactory(); - var pool = CreatePool(factory); - var owner = new SqlConnection("Timeout=30"); - var fakeTime = new FakeTimeProvider(); - TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime); - var tcs = new TaskCompletionSource(); + using LocalAppContextSwitchesHelper switches = new() + { + UseOverallConnectTimeoutForPoolWait = switchEnabled, + }; + TimeoutTimer timer = timerKind switch + { + TimerKind.Expired => TimeoutTimer.StartExpired(), + TimerKind.Infinite => TimeoutTimer.StartNew(TimeSpan.Zero), + _ => throw new ArgumentOutOfRangeException(nameof(timerKind)), + }; - // Act: 5s consumed before entering the pool, then an async request. - fakeTime.Advance(TimeSpan.FromSeconds(5)); - pool.TryGetConnection( - owner, - taskCompletionSource: tcs, + // Act + uint result = WaitHandleDbConnectionPool.ResolvePoolWaitTimeoutMs( timer, - out DbConnectionInternal? connection); - - // Bound the await so a regression in the pool can't hang the suite. - Task completed = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(30))); - Assert.Same(tcs.Task, completed); - DbConnectionInternal result = await tcs.Task; + creationTimeoutMs); - // Assert: factory got the caller's timer with the reduced budget. - Assert.NotNull(result); - Assert.Same(timer, factory.CapturedTimeout); - Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); + // Assert + Assert.Equal(expected, result); } /// diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs new file mode 100644 index 0000000000..f2c3e9b138 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs @@ -0,0 +1,61 @@ +// 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 Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.Connection; +using Microsoft.Data.SqlClient.Tests.Common; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests; + +/// +/// Verifies how selects the +/// that governs the login phase, driven by the +/// UseOverallConnectTimeoutForPoolWait AppContext switch. +/// +/// When the switch is enabled the connection must reuse the caller-supplied +/// timer as-is so the remaining budget (already reflecting any time spent +/// waiting for the pool) is honored during login. When disabled it must +/// construct a fresh timer from ConnectTimeout, preserving legacy +/// behavior. Asserting against the extracted +/// helper keeps this +/// branch coverage free of any real network connection. +/// +public class SqlConnectionInternalTimeoutTests +{ + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ResolveLoginTimeout_HonorsSwitch(bool switchEnabled) + { + // Arrange + using LocalAppContextSwitchesHelper switches = new() + { + UseOverallConnectTimeoutForPoolWait = switchEnabled, + }; + const int ConnectTimeoutSeconds = 30; + TimeoutTimer callerTimeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(ConnectTimeoutSeconds)); + + // Act + TimeoutTimer resolved = SqlConnectionInternal.ResolveLoginTimeout( + callerTimeout, + ConnectTimeoutSeconds); + + // Assert + if (switchEnabled) + { + // Switch on: caller's timer must flow through unchanged so any + // time already consumed counts against the overall budget. + Assert.Same(callerTimeout, resolved); + } + else + { + // Switch off (legacy): a fresh timer must be started from + // ConnectTimeout, independent of the caller's timer. + Assert.NotSame(callerTimeout, resolved); + Assert.Equal(TimeSpan.FromSeconds(ConnectTimeoutSeconds).Ticks, resolved.OriginalTicks); + } + } +} From 8c249a6860d12e2197e7b92d4fb5d94ddfea7033 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 12:03:13 -0700 Subject: [PATCH 37/39] Cleanup --- .../ConnectionPool/WaitHandleDbConnectionPool.cs | 4 +++- .../SqlClient/SqlConnectionInternalTimeoutTests.cs | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) 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 109266c27c..c724e5a25b 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 @@ -909,7 +909,9 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource {0}, DbConnectionInternal State != Running.", Id); connection = null; diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs index f2c3e9b138..acf6e27622 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs @@ -25,6 +25,19 @@ namespace Microsoft.Data.SqlClient.UnitTests; /// public class SqlConnectionInternalTimeoutTests { + /// + /// Verifies the branch selection in + /// : + /// + /// switch ON → the caller's instance + /// flows through unchanged (asserted by reference identity), so any + /// time already consumed counts against the overall ConnectTimeout; + /// switch OFF → a fresh timer is started from + /// ConnectTimeout (asserted by inspecting + /// ), preserving legacy + /// behavior where login always gets the full configured budget. + /// + /// [Theory] [InlineData(true)] [InlineData(false)] From 884d3b4e3e24a23e27fa949dbd7bafaee6016aae Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 14:14:06 -0700 Subject: [PATCH 38/39] Update package versions to match runtime band. --- Directory.Packages.props | 3 ++- .../src/Microsoft.Data.SqlClient.nuspec | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index ff8b0dd226..a74caf225a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -108,7 +108,6 @@ - @@ -125,12 +124,14 @@ + + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec index f634b4a6a8..32e699da90 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec @@ -37,7 +37,7 @@ - + @@ -56,7 +56,7 @@ - + @@ -69,7 +69,7 @@ - + @@ -82,7 +82,7 @@ - + From 05f21f108d619f6c7ed5c3ae1ee61358f1e4a952 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 28 May 2026 15:42:28 -0700 Subject: [PATCH 39/39] Remove TimeProvider for .NET --- .../src/Microsoft.Data.SqlClient.csproj | 1 - .../src/Microsoft.Data.SqlClient.nuspec | 2 -- .../src/Microsoft/Data/ProviderBase/TimeoutTimer.cs | 12 ++++++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj index 5aed02b238..8fcb125c2b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj @@ -295,7 +295,6 @@ - diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec index 32e699da90..a302359ea0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec @@ -56,7 +56,6 @@ - @@ -69,7 +68,6 @@ - diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 8a14645f98..fdb6a52705 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -366,9 +366,17 @@ internal CancellationTokenSource CreateCancellationTokenSource() // Route the timer through the configured TimeProvider so that fake // time providers can advance virtual time and trigger cancellation // deterministically in tests. - // Use the extension method rather than the CancellationTokenSource - // constructor overload, which doesn't exist on .NET Framework. +#if NET + // On .NET 8+ the BCL provides this constructor directly; avoid the + // Microsoft.Bcl.TimeProvider extension so the produced assembly + // doesn't carry a hard reference to the polyfill package, which the + // .NET SDK prunes from downstream consumers' restore graphs. + return new CancellationTokenSource(TimeSpan.FromMilliseconds(remaining), TimeProvider); +#else + // .NET Framework lacks the constructor overload; use the extension + // method shipped by Microsoft.Bcl.TimeProvider. return TimeProvider.CreateCancellationTokenSource(TimeSpan.FromMilliseconds(remaining)); +#endif } ///