Skip to content

Commit 0cf4d4a

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 0cf4d4a

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
@@ -990,6 +990,9 @@ The following example converts an existing connection string from using SQL Serv
990990
<para>
991991
This property corresponds to the "Connection Idle Timeout" key within the connection string.
992992
</para>
993+
<para>
994+
In versions where the AppContext switch <c>Switch.Microsoft.Data.SqlClient.UseLegacyIdleTimeoutBehavior</c> is enabled (the default), the driver preserves historical pooling behavior and does not enforce this setting. Set the switch to <see langword="false" /> to enable idle-timeout enforcement.
995+
</para>
993996
<para>
994997
The driver makes a best effort to discard connections that have remained idle in the pool for longer than this value. The exact point in the connection lifecycle at which the check occurs is an implementation detail and may change over time. This protects callers from receiving connections that may have been silently closed by firewalls, load balancers, or server-side inactivity thresholds.
995998
</para>

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 when legacy idle-timeout behavior is disabled. A value of 0 disables idle expiration.</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)