Skip to content

Commit 7b13305

Browse files
Address PR #4295 Copilot review: document UseLegacyIdleTimeoutBehavior switch and tighten idle-timeout test coverage
- Delete redundant IdleTimeout_DefaultIsZero_DisablesExpiry from ChannelDbConnectionPoolTest. The 'default' framing was misleading (the builder-layer default is 300, not 0) and the zero-as-argument path is already covered by IdleTimeout_PoolGroupOptions_ConvertsSecondsToTimeSpan and IdleTimeout_Zero_DoesNotExpire. - Add IdleTimeout_LegacySwitch_SuppressesEviction to both ChannelDbConnectionPoolTest and WaitHandleDbConnectionPoolIdleTimeoutTest. Flips UseLegacyIdleTimeoutBehavior=true via LocalAppContextSwitchesHelper, back-dates ReturnedTime well past the 1-second timeout, and asserts the connection is still reused. Closes the coverage gap on switch gating in both pool implementations. - Document the gating in the public surfaces: extend SqlConnectionStringBuilder.xml <remarks> with a paragraph describing the Switch.Microsoft.Data.SqlClient.UseLegacyIdleTimeoutBehavior AppContext switch (defaults to true), and the three opt-out mechanisms (AppContext.SetSwitch, <RuntimeHostConfigurationOption>, runtimeconfig.json). Mirror the gating note in DbConnectionString_IdleTimeout in Strings.resx and the auto-generated Strings.Designer.cs accessor so the [ResDescription] surfaced in property grids matches.
1 parent 5b1bbe9 commit 7b13305

5 files changed

Lines changed: 56 additions & 18 deletions

File tree

doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,9 @@ The following example converts an existing connection string from using SQL Serv
999999
<para>
10001000
Idle timeout operates independently of <see cref="P:Microsoft.Data.SqlClient.SqlConnectionStringBuilder.LoadBalanceTimeout" />. Whichever threshold is exceeded first causes the connection to be discarded.
10011001
</para>
1002+
<para>
1003+
Enforcement of this property is gated by the <c>Switch.Microsoft.Data.SqlClient.UseLegacyIdleTimeoutBehavior</c> AppContext switch, which defaults to <see langword="true" /> for backward compatibility. While the switch is enabled, the pool preserves its historical idle-handling behavior and the value configured here is not actively enforced as a discard threshold. To opt in to the new behavior described above, set the switch to <see langword="false" />, for example via <c>AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseLegacyIdleTimeoutBehavior", false)</c> early in application startup, or via <c>&lt;RuntimeHostConfigurationOption&gt;</c> in your project file or <c>runtimeconfig.json</c>.
1004+
</para>
10021005
</remarks>
10031006
</IdleTimeout>
10041007
<MaxPoolSize>

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Microsoft.Data.SqlClient/src/Resources/Strings.resx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@
961961
<value>The minimum amount of time (in seconds) for this connection to live in the pool before being destroyed.</value>
962962
</data>
963963
<data name="DbConnectionString_IdleTimeout" xml:space="preserve">
964-
<value>The maximum amount of time (in seconds) a connection can sit unused (idle) in the pool before it is discarded. A value of 0 disables idle expiration.</value>
964+
<value>The maximum amount of time (in seconds) a connection can sit unused (idle) in the pool before it is discarded. A value of 0 disables idle expiration. Enforcement is gated by the Switch.Microsoft.Data.SqlClient.UseLegacyIdleTimeoutBehavior AppContext switch, which defaults to true for backward compatibility.</value>
965965
</data>
966966
<data name="DbConnectionString_MaxPoolSize" xml:space="preserve">
967967
<value>The maximum number of connections allowed in the pool.</value>

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -976,22 +976,6 @@ public void IdleTimeout_PoolGroupOptions_ConvertsSecondsToTimeSpan()
976976
Assert.Equal(TimeSpan.FromSeconds(30), poolGroupOptions.IdleTimeout);
977977
}
978978

979-
[Fact]
980-
public void IdleTimeout_DefaultIsZero_DisablesExpiry()
981-
{
982-
// Explicitly passing zero keeps idle expiry off.
983-
var poolGroupOptions = new DbConnectionPoolGroupOptions(
984-
poolByIdentity: false,
985-
minPoolSize: 0,
986-
maxPoolSize: 50,
987-
creationTimeout: 15,
988-
loadBalanceTimeout: 0,
989-
hasTransactionAffinity: true,
990-
idleTimeout: 0);
991-
992-
Assert.Equal(TimeSpan.Zero, poolGroupOptions.IdleTimeout);
993-
}
994-
995979
[Fact]
996980
public void IdleTimeout_StampedOnReturn()
997981
{
@@ -1102,6 +1086,34 @@ public void IdleTimeout_Set_KeepsFreshConnection()
11021086
Assert.Same(first, second);
11031087
}
11041088

1089+
[Fact]
1090+
public void IdleTimeout_LegacySwitch_SuppressesEviction()
1091+
{
1092+
using LocalAppContextSwitchesHelper switchesHelper = new();
1093+
switchesHelper.UseLegacyIdleTimeoutBehavior = true;
1094+
1095+
// Arrange - 1-second idle timeout, but legacy switch suppresses the new eviction path.
1096+
var pool = ConstructPoolWithIdleTimeout(idleTimeoutSeconds: 1);
1097+
SqlConnection owner = new();
1098+
pool.TryGetConnection(owner, taskCompletionSource: null,
1099+
TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)),
1100+
out DbConnectionInternal? first);
1101+
Assert.NotNull(first);
1102+
1103+
// Return + back-date well past the configured timeout.
1104+
pool.ReturnInternalConnection(first, owner);
1105+
BackdateReturnedTime(first, TimeSpan.FromMinutes(5));
1106+
1107+
// Act - request another connection.
1108+
SqlConnection owner2 = new();
1109+
pool.TryGetConnection(owner2, taskCompletionSource: null,
1110+
TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)),
1111+
out DbConnectionInternal? second);
1112+
1113+
// Assert - with the legacy switch on, the stale connection is still reused.
1114+
Assert.Same(first, second);
1115+
}
1116+
11051117
// Forcibly rewinds a connection's ReturnedTime by the given amount so tests don't have to sleep.
11061118
private static void BackdateReturnedTime(DbConnectionInternal connection, TimeSpan delta)
11071119
{

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,27 @@ public void IdleTimeout_Set_KeepsFreshConnection()
162162
// Assert - same instance reused, well within idle window
163163
Assert.Same(first, second);
164164
}
165+
166+
[Fact]
167+
public void IdleTimeout_LegacySwitch_SuppressesEviction()
168+
{
169+
using LocalAppContextSwitchesHelper switchesHelper = new();
170+
switchesHelper.UseLegacyIdleTimeoutBehavior = true;
171+
172+
// Arrange - 1-second idle timeout, but legacy switch suppresses the new eviction path.
173+
_pool = CreatePool(idleTimeoutSeconds: 1);
174+
SqlConnection owner = new();
175+
DbConnectionInternal first = GetConnection(owner);
176+
177+
// Return + back-date well past the configured timeout.
178+
_pool.ReturnInternalConnection(first, owner);
179+
BackdateReturnedTime(first, TimeSpan.FromMinutes(5));
180+
181+
// Act - request another connection.
182+
SqlConnection owner2 = new();
183+
DbConnectionInternal second = GetConnection(owner2);
184+
185+
// Assert - with the legacy switch on, the stale connection is still reused.
186+
Assert.Same(first, second);
187+
}
165188
}

0 commit comments

Comments
 (0)