Skip to content

Commit 867c9c2

Browse files
committed
Add tests for timeout evaluation using switch.
1 parent fc08463 commit 867c9c2

5 files changed

Lines changed: 183 additions & 68 deletions

File tree

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ internal int MillisecondsRemainingInt
206206
/// timer was created. Used by <see cref="Reset"/> to restore the original
207207
/// expiration window.
208208
/// </summary>
209-
private long OriginalTicks { get; }
209+
internal long OriginalTicks { get; }
210210

211211
/// <summary>
212212
/// Gets the <see cref="TimeProvider"/> used by this timer. Exposed for

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -403,14 +403,7 @@ internal SqlConnectionInternal(
403403
// If we want to consider pool operations against the overall connect timeout,
404404
// use the provided timeout. Otherwise, start a fresh timeout to receive the full
405405
// connect timeout.
406-
if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait)
407-
{
408-
_timeout = timeout;
409-
}
410-
else
411-
{
412-
_timeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectionOptions.ConnectTimeout));
413-
}
406+
_timeout = ResolveLoginTimeout(timeout, connectionOptions.ConnectTimeout);
414407

415408
// If transient fault handling is enabled then we can retry the login up to the
416409
// ConnectRetryCount.
@@ -777,6 +770,25 @@ private SqlInternalTransaction AvailableInternalTransaction
777770
// @TODO: Make private field.
778771
private bool IsAzureSqlConnection { get; set; }
779772

773+
internal TimeoutTimer Timeout => _timeout;
774+
775+
/// <summary>
776+
/// Selects the <see cref="TimeoutTimer"/> that governs the login phase based on
777+
/// <see cref="LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait"/>.
778+
/// </summary>
779+
/// <remarks>
780+
/// When the switch is enabled the caller-supplied <paramref name="callerTimeout"/>
781+
/// is returned as-is so any time already consumed (e.g., waiting for the pool) counts
782+
/// against the overall ConnectTimeout. When disabled, a fresh timer is started from
783+
/// <paramref name="connectTimeoutSeconds"/>, preserving legacy behavior. Extracted so
784+
/// this branch can be unit-tested without standing up a real connection.
785+
/// </remarks>
786+
internal static TimeoutTimer ResolveLoginTimeout(TimeoutTimer callerTimeout, int connectTimeoutSeconds)
787+
{
788+
return LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait
789+
? callerTimeout
790+
: TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectTimeoutSeconds));
791+
}
780792
#endregion
781793

782794
#region Public and Internal Methods

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -875,38 +875,41 @@ private void WaitForPendingOpen()
875875
} while (_pendingOpens.TryPeek(out next));
876876
}
877877

878+
/// <summary>
879+
/// Resolves the <c>WaitHandle.WaitAny</c> timeout (milliseconds) for a synchronous
880+
/// pool acquire.
881+
/// </summary>
882+
/// <remarks>
883+
/// When <see cref="LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait"/>
884+
/// is enabled the caller's remaining <see cref="TimeoutTimer"/> budget is used so
885+
/// the pool wait participates in the overall ConnectTimeout. Otherwise the legacy
886+
/// behavior is preserved: the static pool <c>CreationTimeout</c> is used, with
887+
/// <c>0</c> mapped to <see cref="Timeout.Infinite"/>. Extracted so this branch can
888+
/// be unit-tested without timing-based assertions.
889+
/// </remarks>
890+
internal static uint ResolvePoolWaitTimeoutMs(TimeoutTimer timeout, int creationTimeoutMs)
891+
{
892+
if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait)
893+
{
894+
return timeout.IsInfinite
895+
? unchecked((uint)Timeout.Infinite)
896+
: (uint)timeout.MillisecondsRemainingInt;
897+
}
898+
899+
uint legacy = (uint)creationTimeoutMs;
900+
return legacy == 0 ? unchecked((uint)Timeout.Infinite) : legacy;
901+
}
902+
878903
public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource<DbConnectionInternal> taskCompletionSource, TimeoutTimer timeout, out DbConnectionInternal connection)
879904
{
880905
uint waitForMultipleObjectsTimeout = 0;
881906
bool allowCreate = false;
882907

883908
if (taskCompletionSource == null)
884909
{
885-
if (LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait)
886-
{
887-
// Use the caller's remaining timeout budget (rather than the static
888-
// CreationTimeout) so synchronous pool waits respect any time already
889-
// consumed earlier in the open path and don't exceed the overall
890-
// ConnectTimeout.
891-
waitForMultipleObjectsTimeout = timeout.IsInfinite
892-
? unchecked((uint)Timeout.Infinite)
893-
: (uint)timeout.MillisecondsRemainingInt;
894-
}
895-
else
896-
{
897-
waitForMultipleObjectsTimeout = (uint)CreationTimeout;
898-
899-
// Set the wait timeout to INFINITE (-1) if the pool CreationTimeout is 0.
900-
if (waitForMultipleObjectsTimeout == 0)
901-
{
902-
waitForMultipleObjectsTimeout = unchecked((uint)Timeout.Infinite);
903-
}
904-
}
905-
910+
waitForMultipleObjectsTimeout = ResolvePoolWaitTimeoutMs(timeout, CreationTimeout);
906911
allowCreate = true;
907-
}
908-
909-
if (State is not Running)
912+
} if (State is not Running)
910913
{
911914
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, DbConnectionInternal State != Running.", Id);
912915
connection = null;

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBudgetTest.cs

Lines changed: 74 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44

55
using System;
66
using System.Data.Common;
7+
using System.Threading;
78
using System.Threading.Tasks;
89
using System.Transactions;
910
using Microsoft.Data.ProviderBase;
1011
using Microsoft.Data.SqlClient.ConnectionPool;
12+
using Microsoft.Data.SqlClient.Tests.Common;
1113
using Microsoft.Extensions.Time.Testing;
1214
using Xunit;
1315

@@ -65,73 +67,110 @@ private WaitHandleDbConnectionPool CreatePool(
6567

6668
/// <summary>
6769
/// Verifies that the <see cref="TimeoutTimer"/> the pool hands to the
68-
/// connection factory on the synchronous path reports a reduced
69-
/// remaining-time budget when the timer's clock has advanced before the
70-
/// pool was entered. Mirrors
70+
/// connection factory reports a reduced remaining-time budget when the
71+
/// timer's clock has advanced before the pool was entered. Both the
72+
/// synchronous (<c>taskCompletionSource == null</c>) and asynchronous
73+
/// paths must forward the caller's already-advanced timer rather than
74+
/// constructing a fresh one from <c>CreationTimeout</c>. Mirrors
7175
/// <c>ChannelDbConnectionPoolTest.GetConnection_TimeoutTimerReflectsPoolWaitTime</c>.
7276
/// </summary>
73-
[Fact]
74-
public void GetConnection_Sync_TimeoutTimerReflectsTimeAlreadyConsumed()
77+
[Theory]
78+
[InlineData(false)] // sync
79+
[InlineData(true)] // async
80+
public async Task GetConnection_TimeoutTimerReflectsTimeAlreadyConsumed(bool async)
7581
{
76-
// Arrange: a capturing factory and a fake-time-backed timer with a
82+
// Arrange: capturing factory and a fake-time-backed timer with a
7783
// 30-second budget anchored at virtual time t = 0.
7884
var factory = new MockSqlConnectionFactory();
7985
var pool = CreatePool(factory);
8086
var owner = new SqlConnection("Timeout=30");
8187
var fakeTime = new FakeTimeProvider();
8288
TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime);
89+
TaskCompletionSource<DbConnectionInternal>? tcs =
90+
async ? new TaskCompletionSource<DbConnectionInternal>() : null;
8391

8492
// Act: simulate 5s of budget consumed elsewhere (e.g., higher-level
85-
// Open() work) before the pool is entered.
93+
// Open() work) before the pool is entered, then request a connection.
8694
fakeTime.Advance(TimeSpan.FromSeconds(5));
8795
bool completed = pool.TryGetConnection(
8896
owner,
89-
taskCompletionSource: null,
97+
tcs,
9098
timer,
9199
out DbConnectionInternal? connection);
92100

101+
if (async)
102+
{
103+
// Bound the await so a regression in the pool can't hang the suite.
104+
Task winner = await Task.WhenAny(tcs!.Task, Task.Delay(TimeSpan.FromSeconds(30)));
105+
Assert.Same(tcs.Task, winner);
106+
connection = await tcs.Task;
107+
}
108+
else
109+
{
110+
Assert.True(completed);
111+
}
112+
93113
// Assert: factory received the same timer, and it reports the reduced
94114
// 25-second remaining budget rather than the original 30s or the
95115
// pool's static 15s CreationTimeout.
96-
Assert.True(completed);
97116
Assert.NotNull(connection);
98117
Assert.Same(timer, factory.CapturedTimeout);
99118
Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt);
100119
}
101120

102121
/// <summary>
103-
/// Async counterpart of <see cref="GetConnection_Sync_TimeoutTimerReflectsTimeAlreadyConsumed"/>.
104-
/// Verifies that the async pool path also forwards the caller's
105-
/// already-advanced <see cref="TimeoutTimer"/> to the factory.
122+
/// Identifies which kind of caller-supplied <see cref="TimeoutTimer"/> a
123+
/// parameterized test should construct. Used because
124+
/// <see cref="InlineDataAttribute"/> cannot carry a live
125+
/// <see cref="TimeoutTimer"/> instance.
126+
/// </summary>
127+
public enum TimerKind
128+
{
129+
Expired,
130+
Infinite,
131+
}
132+
133+
/// <summary>
134+
/// Verifies the resolution matrix for the synchronous
135+
/// <c>WaitHandle.WaitAny</c> timeout:
136+
/// <list type="bullet">
137+
/// <item>switch ON → use the caller timer's remaining budget
138+
/// (expired → 0, infinite → <see cref="Timeout.Infinite"/>);</item>
139+
/// <item>switch OFF → ignore the caller timer and use
140+
/// <c>CreationTimeout</c>, treating <c>0</c> as
141+
/// <see cref="Timeout.Infinite"/> per legacy behavior.</item>
142+
/// </list>
106143
/// </summary>
107-
[Fact]
108-
public async Task GetConnection_Async_TimeoutTimerReflectsTimeAlreadyConsumed()
144+
[Theory]
145+
[InlineData(true, TimerKind.Expired, 5_000, 0u)]
146+
[InlineData(true, TimerKind.Infinite, 5_000, unchecked((uint)Timeout.Infinite))]
147+
[InlineData(false, TimerKind.Expired, 1_500, 1_500u)]
148+
[InlineData(false, TimerKind.Expired, 0, unchecked((uint)Timeout.Infinite))]
149+
public void ResolvePoolWaitTimeoutMs_ReturnsExpected(
150+
bool switchEnabled,
151+
TimerKind timerKind,
152+
int creationTimeoutMs,
153+
uint expected)
109154
{
110155
// Arrange
111-
var factory = new MockSqlConnectionFactory();
112-
var pool = CreatePool(factory);
113-
var owner = new SqlConnection("Timeout=30");
114-
var fakeTime = new FakeTimeProvider();
115-
TimeoutTimer timer = TimeoutTimer.StartNew(TimeSpan.FromSeconds(30), fakeTime);
116-
var tcs = new TaskCompletionSource<DbConnectionInternal>();
156+
using LocalAppContextSwitchesHelper switches = new()
157+
{
158+
UseOverallConnectTimeoutForPoolWait = switchEnabled,
159+
};
160+
TimeoutTimer timer = timerKind switch
161+
{
162+
TimerKind.Expired => TimeoutTimer.StartExpired(),
163+
TimerKind.Infinite => TimeoutTimer.StartNew(TimeSpan.Zero),
164+
_ => throw new ArgumentOutOfRangeException(nameof(timerKind)),
165+
};
117166

118-
// Act: 5s consumed before entering the pool, then an async request.
119-
fakeTime.Advance(TimeSpan.FromSeconds(5));
120-
pool.TryGetConnection(
121-
owner,
122-
taskCompletionSource: tcs,
167+
// Act
168+
uint result = WaitHandleDbConnectionPool.ResolvePoolWaitTimeoutMs(
123169
timer,
124-
out DbConnectionInternal? connection);
125-
126-
// Bound the await so a regression in the pool can't hang the suite.
127-
Task completed = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(30)));
128-
Assert.Same(tcs.Task, completed);
129-
DbConnectionInternal result = await tcs.Task;
170+
creationTimeoutMs);
130171

131-
// Assert: factory got the caller's timer with the reduced budget.
132-
Assert.NotNull(result);
133-
Assert.Same(timer, factory.CapturedTimeout);
134-
Assert.Equal(25_000, factory.CapturedTimeout!.MillisecondsRemainingInt);
172+
// Assert
173+
Assert.Equal(expected, result);
135174
}
136175

137176
/// <summary>
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using Microsoft.Data.ProviderBase;
7+
using Microsoft.Data.SqlClient.Connection;
8+
using Microsoft.Data.SqlClient.Tests.Common;
9+
using Xunit;
10+
11+
namespace Microsoft.Data.SqlClient.UnitTests;
12+
13+
/// <summary>
14+
/// Verifies how <see cref="SqlConnectionInternal"/> selects the
15+
/// <see cref="TimeoutTimer"/> that governs the login phase, driven by the
16+
/// <c>UseOverallConnectTimeoutForPoolWait</c> AppContext switch.
17+
///
18+
/// When the switch is enabled the connection must reuse the caller-supplied
19+
/// timer as-is so the remaining budget (already reflecting any time spent
20+
/// waiting for the pool) is honored during login. When disabled it must
21+
/// construct a fresh timer from <c>ConnectTimeout</c>, preserving legacy
22+
/// behavior. Asserting against the extracted
23+
/// <see cref="SqlConnectionInternal.ResolveLoginTimeout"/> helper keeps this
24+
/// branch coverage free of any real network connection.
25+
/// </summary>
26+
public class SqlConnectionInternalTimeoutTests
27+
{
28+
[Theory]
29+
[InlineData(true)]
30+
[InlineData(false)]
31+
public void ResolveLoginTimeout_HonorsSwitch(bool switchEnabled)
32+
{
33+
// Arrange
34+
using LocalAppContextSwitchesHelper switches = new()
35+
{
36+
UseOverallConnectTimeoutForPoolWait = switchEnabled,
37+
};
38+
const int ConnectTimeoutSeconds = 30;
39+
TimeoutTimer callerTimeout = TimeoutTimer.StartNew(TimeSpan.FromSeconds(ConnectTimeoutSeconds));
40+
41+
// Act
42+
TimeoutTimer resolved = SqlConnectionInternal.ResolveLoginTimeout(
43+
callerTimeout,
44+
ConnectTimeoutSeconds);
45+
46+
// Assert
47+
if (switchEnabled)
48+
{
49+
// Switch on: caller's timer must flow through unchanged so any
50+
// time already consumed counts against the overall budget.
51+
Assert.Same(callerTimeout, resolved);
52+
}
53+
else
54+
{
55+
// Switch off (legacy): a fresh timer must be started from
56+
// ConnectTimeout, independent of the caller's timer.
57+
Assert.NotSame(callerTimeout, resolved);
58+
Assert.Equal(TimeSpan.FromSeconds(ConnectTimeoutSeconds).Ticks, resolved.OriginalTicks);
59+
}
60+
}
61+
}

0 commit comments

Comments
 (0)