diff --git a/Directory.Packages.props b/Directory.Packages.props index 37b72ca205..a74caf225a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -124,12 +124,14 @@ + + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj index d7a7ef060a..8fcb125c2b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj @@ -276,6 +276,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec b/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec index da3a17bb9c..a302359ea0 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 @@ + @@ -79,6 +80,7 @@ + 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..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,14 +686,6 @@ internal void MakePooledConnection(IDbConnectionPool connectionPool) Pool = connectionPool; } - internal virtual void OpenConnection(DbConnection outerConnection, SqlConnectionFactory connectionFactory) - { - if (!TryOpenConnection(outerConnection, connectionFactory, null)) - { - 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 @@ -800,7 +792,8 @@ internal void SetInStasis() internal virtual bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { throw ADP.ConnectionAlreadyOpen(State); } @@ -808,7 +801,8 @@ internal virtual bool TryOpenConnection( internal virtual bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { throw ADP.MethodNotImplemented(); } @@ -910,7 +904,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 +914,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/ProviderBase/TimeoutTimer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs index 37c94fe355..fdb6a52705 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs @@ -2,200 +2,185 @@ // 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; +#nullable enable + using System; using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; 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 (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 { - //------------------- - // 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 const 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 and time source. + /// + /// + /// The duration before the timer expires. A value whose ticks equal + /// indicates an infinite timeout. + /// + /// + /// 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) { - //-------------------- - // 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; + TimeProvider = timeProvider; + OriginalTicks = expiration.Ticks; + IsInfinite = OriginalTicks == InfiniteTimeout; + ExpirationTicks = checked(NowTicks() + 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 + #endregion - //-------------------- - // Method body - if (InfiniteTimeout == seconds) - { - _isInfiniteTimeout = true; - } - else - { - // Stash current time + timeout - _originalTimerTicks = ADP.TimerFromSeconds(seconds); - _timerExpire = checked(ADP.TimerCurrent() + _originalTimerTicks); - _isInfiniteTimeout = false; - } + #region Properties - //--------------------- - // Postconditions:None - } - - // Reset timer to original duration. - internal void Reset() + /// + /// 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. + /// + /// + /// The tick scale is intentionally compatible with + /// + /// + internal long ExpirationTicks { - if (InfiniteTimeout == _originalTimerTicks) - { - _isInfiniteTimeout = true; - } - else - { - _timerExpire = checked(ADP.TimerCurrent() + _originalTimerTicks); - _isInfiniteTimeout = false; - } + get; + //TODO: Remove this when we disable Reset() + private set; } - //------------------- - // 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 + /// (as read from the configured ) has passed + /// ; otherwise, . + /// internal bool IsExpired { get { - return !IsInfinite && ADP.TimerHasExpired(_timerExpire); - } - } - - // is this an infinite-timeout timer? - internal bool IsInfinite - { - get - { - return _isInfiniteTimeout; + return !IsInfinite && NowTicks() > ExpirationTicks; } } - // Special accessor for TimerExpire for use when thunking to legacy timeout methods. - public long LegacyTimerExpire - { - get - { - return (_isInfiniteTimeout) ? long.MaxValue : _timerExpire; - } - } + /// + /// 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; } - // 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 the number of milliseconds remaining before this timer expires, + /// truncated to 0 when none remain, and approximated 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 { - //------------------- - // Preconditions: None - - //------------------- - // Method Body long milliseconds; - if (_isInfiniteTimeout) + if (IsInfinite) { milliseconds = long.MaxValue; } else { - milliseconds = ADP.TimerRemainingMilliseconds(_timerExpire); + milliseconds = TicksToMilliseconds(ExpirationTicks - NowTicks()); if (0 > milliseconds) { milliseconds = 0; } } - //-------------------- - // Postconditions Debug.Assert(0 <= milliseconds); // This property guarantees no negative return values return milliseconds; } } - // 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 approximated 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 { - //------------------- - // Method Body int milliseconds; - if (_isInfiniteTimeout) + if (IsInfinite) { milliseconds = int.MaxValue; } else { - long longMilliseconds = ADP.TimerRemainingMilliseconds(_timerExpire); + long longMilliseconds = TicksToMilliseconds(ExpirationTicks - NowTicks()); if (0 > longMilliseconds) { milliseconds = 0; @@ -210,12 +195,220 @@ internal int MillisecondsRemainingInt } } - //-------------------- - // Postconditions Debug.Assert(0 <= milliseconds); 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. + /// + internal 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 { get; } + + #endregion + + #region Methods + + /// + /// Creates and starts a new with the specified + /// 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. + /// + /// A new instance that has already started. + 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 a truly infinite timeout, 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 + /// timer was constructed with. + /// + /// + /// 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; + } + + // Route the timer through the configured TimeProvider so that fake + // time providers can advance virtual time and trigger cancellation + // deterministically in tests. +#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 + } + + /// + /// 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(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(). + /// + 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/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index 6d067bab7f..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 @@ -317,6 +317,7 @@ internal class SqlConnectionInternal : DbConnectionInternal, IDisposable internal SqlConnectionInternal( DbConnectionPoolIdentity identity, SqlConnectionOptions connectionOptions, + TimeoutTimer timeout, SqlCredential credential, DbConnectionPoolGroupProviderInfo providerInfo, string newPassword, @@ -399,7 +400,10 @@ internal SqlConnectionInternal( try { - _timeout = TimeoutTimer.StartSecondsTimeout(connectionOptions.ConnectTimeout); + // 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. + _timeout = ResolveLoginTimeout(timeout, connectionOptions.ConnectTimeout); // If transient fault handling is enabled then we can retry the login up to the // ConnectRetryCount. @@ -766,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 @@ -1944,9 +1967,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) @@ -2204,6 +2228,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 @@ -3200,7 +3225,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 @@ -3212,11 +3236,11 @@ private void LoginNoFailover( } #endif - if (nextTimeoutInterval > milliseconds) - { - nextTimeoutInterval = milliseconds; - } - intervalTimer = TimeoutTimer.StartMillisecondsTimeout(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. @@ -3495,13 +3519,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; - } - TimeoutTimer intervalTimer = TimeoutTimer.StartMillisecondsTimeout(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/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index b83434cac6..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 @@ -224,7 +224,8 @@ public void PutObjectFromTransactedPool(DbConnectionInternal connection) /// public DbConnectionInternal ReplaceConnection( DbConnection owningObject, - DbConnectionInternal oldConnection) + DbConnectionInternal oldConnection, + TimeoutTimer timeout) { throw new NotImplementedException(); } @@ -281,10 +282,9 @@ public void TransactionEnded(Transaction transaction, DbConnectionInternal trans public bool TryGetConnection( DbConnection owningObject, TaskCompletionSource? taskCompletionSource, + TimeoutTimer timeout, out DbConnectionInternal? connection) { - var timeout = TimeSpan.FromSeconds(owningObject.ConnectionTimeout); - // If taskCompletionSource is null, we are in a sync context. if (taskCompletionSource is null) { @@ -372,13 +372,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 +400,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 +497,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 +511,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 +533,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/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 4243e777ba..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 @@ -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,7 +522,7 @@ private bool IsBlockingPeriodEnabled() } } - private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectionInternal oldConnection) + private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectionInternal oldConnection, TimeoutTimer timeout) { DbConnectionInternal newObj = null; @@ -528,7 +530,8 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio { newObj = _connectionFactory.CreatePooledConnection( owningObject, - this); + this, + timeout); lock (_objectList) { @@ -834,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) @@ -871,24 +875,42 @@ private void WaitForPendingOpen() } while (_pendingOpens.TryPeek(out next)); } - public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, out DbConnectionInternal connection) + /// + /// 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; bool allowCreate = false; if (taskCompletionSource == null) { - waitForMultipleObjectsTimeout = (uint)CreationTimeout; - - // Set the wait timeout to INFINITE (-1) if the SQL connection timeout is 0 (== infinite) - if (waitForMultipleObjectsTimeout == 0) - { - waitForMultipleObjectsTimeout = unchecked((uint)Timeout.Infinite); - } - + waitForMultipleObjectsTimeout = ResolvePoolWaitTimeoutMs(timeout, CreationTimeout); allowCreate = true; - } - + } + if (State is not Running) { SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, DbConnectionInternal State != Running.", Id); @@ -897,7 +919,7 @@ public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource {0}, Creating new connection.", Id); try { - obj = UserCreateRequest(owningObject); + obj = UserCreateRequest(owningObject, timeout); } catch { @@ -1040,7 +1079,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 { @@ -1127,11 +1166,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) { @@ -1271,7 +1311,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 { @@ -1515,7 +1560,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 @@ -1535,7 +1580,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/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 0177338bf2..df762ed7ae 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -125,6 +125,13 @@ internal static class LocalAppContextSwitches private const string UseConnectionPoolV2String = "Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2"; + /// + /// 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 @@ -234,6 +241,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. @@ -564,6 +576,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/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; 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 9021d3e259..6e6a33087b 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; } @@ -2740,6 +2750,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..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,7 +115,8 @@ 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?"); @@ -125,7 +126,8 @@ internal DbConnectionInternal CreateNonPooledConnection( poolGroup.PoolKey, poolGroup.ProviderInfo, pool: null, - owningConnection); + owningConnection, + timeout); if (newConnection is not null) { SqlClientDiagnostics.Metrics.HardConnectRequest(); @@ -138,7 +140,8 @@ internal DbConnectionInternal CreateNonPooledConnection( internal DbConnectionInternal CreatePooledConnection( DbConnection owningConnection, - IDbConnectionPool pool) + IDbConnectionPool pool, + TimeoutTimer timeout) { Debug.Assert(pool != null, "null pool?"); @@ -147,7 +150,8 @@ internal DbConnectionInternal CreatePooledConnection( pool.PoolGroup.PoolKey, pool.PoolGroup.ProviderInfo, pool, - owningConnection); + owningConnection, + timeout); if (newConnection is null) { @@ -313,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?"); @@ -384,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 @@ -407,7 +413,7 @@ internal bool TryGetConnection( return false; } - connection = CreateNonPooledConnection(owningConnection, poolGroup); + connection = CreateNonPooledConnection(owningConnection, poolGroup, timeout); SqlClientDiagnostics.Metrics.EnterNonPooledConnection(); } @@ -417,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; } @@ -573,7 +579,8 @@ protected virtual DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { SqlConnectionOptions opt = options; ConnectionPoolKey key = poolKey; @@ -628,6 +635,7 @@ protected virtual DbConnectionInternal CreateConnection( SqlConnectionInternal sseConnection = new SqlConnectionInternal( identity, sseopt, + timeout, key.Credential, providerInfo: null, newPassword: string.Empty, @@ -678,6 +686,7 @@ protected virtual DbConnectionInternal CreateConnection( return new SqlConnectionInternal( identity, opt, + timeout, key.Credential, poolGroupProviderInfo, newPassword: string.Empty, @@ -752,6 +761,7 @@ private Task CreateReplaceConnectionContinuation( TaskCompletionSource retry, DbConnectionInternal oldConnection, DbConnectionPoolGroup poolGroup, + TimeoutTimer timeout, CancellationTokenSource cancellationTokenSource) { return task.ContinueWith( @@ -762,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/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index 66e9c3e52e..9cdeaccaab 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.IsInfinite ? long.MaxValue : timeout.ExpirationTicks; bool marsCapable = false; diff --git a/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs b/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs index 11523b3f83..3ee71097db 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs @@ -56,6 +56,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 @@ -117,6 +118,8 @@ public LocalAppContextSwitchesHelper() GetSwitchValue("s_useCompatibilityProcessSni"); _useConnectionPoolV2Original = GetSwitchValue("s_useConnectionPoolV2"); + _useOverallConnectTimeoutForPoolWaitOriginal = + GetSwitchValue("s_useOverallConnectTimeoutForPoolWait"); #if NET && _WINDOWS _useManagedNetworkingOriginal = GetSwitchValue("s_useManagedNetworking"); @@ -184,6 +187,9 @@ public void Dispose() SetSwitchValue( "s_useConnectionPoolV2", _useConnectionPoolV2Original); + SetSwitchValue( + "s_useOverallConnectTimeoutForPoolWait", + _useOverallConnectTimeoutForPoolWaitOriginal); #if NET && _WINDOWS SetSwitchValue( "s_useManagedNetworking", @@ -329,6 +335,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/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 @@ + 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 00bba56fc2..26f65b6fae 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. // Disabled on Kerberos and Managed Instance pipelines due to environment-specific instability. [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), @@ -36,6 +107,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(); @@ -53,7 +130,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(); @@ -74,6 +160,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) @@ -121,6 +225,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) @@ -135,6 +247,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(); @@ -187,6 +312,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. diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 42a2331be6..ca22d6eafd 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -172,12 +172,14 @@ private static string GetSPNInfo(string dataSource, string inInstanceName) Type[] getPortByInstanceNameTypesArray = new Type[] { typeof(string), typeof(string), timeoutTimerType, typeof(bool), typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference) }; - Type[] startSecondsTimeoutTypesArray = new Type[] { typeof(int) }; + // The current TimeoutTimer API exposes only a static StartNew(TimeSpan) + // factory; the legacy parameterless constructor + StartSecondsTimeout(int) + // shape no longer exists. + Type[] startNewTypesArray = new Type[] { typeof(TimeSpan) }; ConstructorInfo sniProxyConstructor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); ConstructorInfo SSRPConstructor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); ConstructorInfo dataSourceConstructor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - ConstructorInfo timeoutTimerConstructor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); object sniProxyObj = sniProxyConstructor.Invoke(new object[] { }); @@ -185,11 +187,9 @@ private static string GetSPNInfo(string dataSource, string inInstanceName) object ssrpObj = SSRPConstructor.Invoke(new object[] { }); - object timeoutTimerObj = timeoutTimerConstructor.Invoke(new object[] { }); + MethodInfo startNewInfo = timeoutTimerType.GetMethod("StartNew", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startNewTypesArray, null); - MethodInfo startSecondsTimeoutInfo = timeoutTimerObj.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); - - timeoutTimerObj = startSecondsTimeoutInfo.Invoke(dataSourceObj, new object[] { 30 }); + object timeoutTimerObj = startNewInfo.Invoke(null, new object[] { TimeSpan.FromSeconds(30) }); MethodInfo parseServerNameInfo = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); object dataSrcInfo = parseServerNameInfo.Invoke(dataSourceObj, new object[] { dataSource }); 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/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 7c1593af14..1fea8aede1 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 @@ -63,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 ); @@ -92,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 ); @@ -117,6 +120,7 @@ public void GetConnectionMaxPoolSize_ShouldTimeoutAfterPeriod() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -124,23 +128,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); } @@ -155,6 +163,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldTimeoutAfterPeriod() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -162,25 +171,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); } @@ -194,6 +203,7 @@ public async Task GetConnectionMaxPoolSize_ShouldReuseAfterConnectionReleased() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -202,6 +212,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -217,6 +228,7 @@ out DbConnectionInternal? internalConnection var exceeded = pool.TryGetConnection( new SqlConnection(""), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? extraConnection ); return extraConnection; @@ -238,6 +250,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldReuseAfterConnectionReleas pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -246,6 +259,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -259,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); @@ -279,6 +294,7 @@ public async Task GetConnectionMaxPoolSize_ShouldRespectOrderOfRequest() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -287,6 +303,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -307,6 +324,7 @@ out DbConnectionInternal? internalConnection pool.TryGetConnection( new SqlConnection(""), null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? recycledConnection ); return recycledConnection; @@ -319,6 +337,7 @@ out DbConnectionInternal? recycledConnection pool.TryGetConnection( new SqlConnection("Timeout=1"), null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)), out DbConnectionInternal? failedConnection ); return failedConnection; @@ -344,6 +363,7 @@ public async Task GetConnectionAsyncMaxPoolSize_ShouldRespectOrderOfRequest() pool.TryGetConnection( firstOwningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? firstConnection ); @@ -352,6 +372,7 @@ out DbConnectionInternal? firstConnection var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); @@ -366,6 +387,7 @@ out DbConnectionInternal? internalConnection var exceeded = pool.TryGetConnection( new SqlConnection(""), recycledTaskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? recycledConnection ); @@ -375,6 +397,7 @@ out DbConnectionInternal? recycledConnection var exceeded2 = pool.TryGetConnection( new SqlConnection("Timeout=1"), failedCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(1)), out DbConnectionInternal? failedConnection ); @@ -397,6 +420,7 @@ public void ConnectionsAreReused() var completed1 = pool.TryGetConnection( owningConnection, null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection1 ); @@ -411,6 +435,7 @@ out DbConnectionInternal? internalConnection1 var completed2 = pool.TryGetConnection( owningConnection, null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection2 ); @@ -432,11 +457,14 @@ public void GetConnectionTimeout_ShouldThrowTimeoutException() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), 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] @@ -452,13 +480,16 @@ public async Task GetConnectionAsyncTimeout_ShouldThrowTimeoutException() var completed = pool.TryGetConnection( new SqlConnection(), taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), 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] @@ -476,6 +507,7 @@ public void StressTest() var completed = pool.TryGetConnection( owningObject, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); if (completed) @@ -509,6 +541,7 @@ public void StressTestAsync() var completed = pool.TryGetConnection( owningObject, taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); internalConnection = await taskCompletionSource.Task; @@ -666,7 +699,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] @@ -705,6 +738,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); @@ -733,6 +767,7 @@ public void Clear_BusyConnection_NotDestroyedImmediately() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); @@ -756,6 +791,7 @@ public void Clear_BusyConnectionReturned_IsDestroyed() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? busyConnection ); Assert.NotNull(busyConnection); @@ -785,11 +821,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); @@ -822,6 +860,7 @@ public void Clear_NewConnectionsAfterClear_ArePooledNormally() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? oldConnection ); Assert.Equal(0, oldConnection!.ClearGeneration); @@ -835,6 +874,7 @@ out DbConnectionInternal? oldConnection pool.TryGetConnection( newOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? newConnection ); Assert.NotNull(newConnection); @@ -852,6 +892,7 @@ out DbConnectionInternal? newConnection pool.TryGetConnection( reuseOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? reusedConnection ); Assert.Same(newConnection, reusedConnection); @@ -868,6 +909,7 @@ public void Clear_MultipleClearCalls_DoNotCorruptState() pool.TryGetConnection( owningConnection, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? connection ); Assert.Equal(0, connection!.ClearGeneration); @@ -886,6 +928,7 @@ out DbConnectionInternal? connection pool.TryGetConnection( newOwner, taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? newConnection ); Assert.NotNull(newConnection); @@ -898,13 +941,17 @@ out DbConnectionInternal? newConnection #region Test classes internal class SuccessfulSqlConnectionFactory : SqlConnectionFactory { + internal TimeoutTimer? CapturedTimeout { get; private set; } + protected override DbConnectionInternal CreateConnection( SqlConnectionOptions options, ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { + CapturedTimeout = timeout; return new StubDbConnectionInternal(); } } @@ -916,7 +963,8 @@ protected override DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { throw ADP.PooledOpenTimeout(); } @@ -1081,5 +1129,131 @@ public void Constructor_WithValidSmallPoolSizes_WorksCorrectly() Assert.NotNull(pool2); Assert.Equal(0, pool2.Count); } + + #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() + { + // 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, 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 + // 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(), + taskCompletionSource: null, + timerA, + out DbConnectionInternal? connectionA); + return connectionA; + }); + + var callerBTask = Task.Run(() => + { + pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource: null, + timerB, + out DbConnectionInternal? connectionB); + return connectionB; + }); + + // 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)); + + // 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.", + exA.Message); + + // 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 + pool.ReturnInternalConnection(firstConnection, firstOwner); + + // Bound the wait so a regression in the pool can't hang the test suite + // indefinitely; a real success completes well under this budget. + Task completed = await Task.WhenAny(callerBTask, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same(callerBTask, completed); + var resultB = await callerBTask; + + // Caller B got the connection + Assert.NotNull(resultB); + Assert.Same(firstConnection, resultB); + } + + /// + /// 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_TimeoutTimerReflectsPoolWaitTime() + { + // 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: 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: factory received the same timer, and it reports the + // reduced 25-second remaining budget. + Assert.NotNull(connection); + Assert.Same(timer, factory.CapturedTimeout); + Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); + } + + #endregion } } 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(); } 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..6c00f8e38e --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs @@ -0,0 +1,232 @@ +// 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; +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; + +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 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. + /// + [Theory] + [InlineData(false)] // sync + [InlineData(true)] // async + public async Task GetConnection_TimeoutTimerReflectsTimeAlreadyConsumed(bool async) + { + // 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, then request a connection. + fakeTime.Advance(TimeSpan.FromSeconds(5)); + bool completed = pool.TryGetConnection( + owner, + 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.NotNull(connection); + Assert.Same(timer, factory.CapturedTimeout); + Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt); + } + + /// + /// 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. + /// + /// + [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 + 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 + uint result = WaitHandleDbConnectionPool.ResolvePoolWaitTimeoutMs( + timer, + creationTimeoutMs); + + // Assert + Assert.Equal(expected, result); + } + + /// + /// 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() + { + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index a5e5d0339f..555de74b29 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; } @@ -902,7 +904,8 @@ protected override DbConnectionInternal CreateConnection( ConnectionPoolKey poolKey, DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, IDbConnectionPool pool, - DbConnection owningConnection) + DbConnection owningConnection, + TimeoutTimer timeout) { return new MockDbConnectionInternal(); } 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 82e900e54c..f9a0f9574e 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 @@ -58,6 +58,14 @@ + + @@ -74,6 +82,7 @@ + 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 a468d8fd37..6df3fe44fc 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.UseLegacyFailoverAlternationOnLoginSqlErrors); 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..acf6e27622 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionInternalTimeoutTests.cs @@ -0,0 +1,74 @@ +// 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 +{ + /// + /// 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)] + 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); + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs new file mode 100644 index 0000000000..5d98837806 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ProviderBase/TimeoutTimerTest.cs @@ -0,0 +1,513 @@ +// 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.Common; +using Microsoft.Data.ProviderBase; +using Microsoft.Extensions.Time.Testing; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ProviderBase +{ + /// + /// Verifies behavior: expiration evaluation, + /// remaining-time reporting, reset, infinite timers, and the cancellation + /// token source it produces. + /// + 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); + + // 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); + } + + /// + /// 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() + { + // 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); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromHours(1), fake); + using CancellationTokenSource cts = timer.CreateCancellationTokenSource(); + Assert.False(cts.IsCancellationRequested); + + // 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)); + + // 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); + } + + /// + /// 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. + private static async Task WaitForAsync(Func predicate) + { + for (int i = 0; i < 50; i++) + { + if (predicate()) + { + return; + } + 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 variant) counts down as virtual time + /// elapses and reports the full original duration when no time has + /// passed yet. + /// + [Fact] + public void MillisecondsRemaining_Long_DecreasesAsTimeElapses() + { + // Arrange + var fake = new FakeTimeProvider(DateTimeOffset.UtcNow); + TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(10), fake); + Assert.Equal(10_000L, timer.MillisecondsRemaining); + + // Act + fake.Advance(TimeSpan.FromSeconds(3)); + + // Assert + Assert.Equal(7_000L, timer.MillisecondsRemaining); + } + + /// + /// 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); + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs index f2c50285ed..9306b1d611 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs @@ -494,6 +494,7 @@ public void TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary(uint e Assert.Equal(0, failoverServer.PreLoginCount); } + [Trait("Category", "flaky")] [Theory] [InlineData(40613)] [InlineData(42108)]