Skip to content

Connect timeout propagated through pool #4270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mdaigle wants to merge 32 commits into
base: main
Choose a base branch
from
Draft

Connect timeout propagated through pool #4270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d37911c
Clean up TimeoutTimer
mdaigle May 11, 2026
cdbac4a
Pass TimeoutTimer down from pool.
mdaigle May 11, 2026
8abc9d5
compilation fixes
mdaigle May 12, 2026
d8cb7f9
Lift TimeoutTimer creation up to SqlConnection.TryOpenInner
mdaigle May 12, 2026
c2658d3
Inject TimeProvider into TimeoutTimer for deterministic tests
mdaigle May 12, 2026
57708b3
Enable nullable.
mdaigle May 12, 2026
c8342a9
Remove low value tests
mdaigle May 12, 2026
634f350
Remove low value tests.
mdaigle May 12, 2026
3a7fa1e
Inline timer creation in tests.
mdaigle May 12, 2026
fc9e85f
test cleanup
mdaigle May 12, 2026
c9cf6ef
clarify cancellation token test
mdaigle May 12, 2026
d6a26e3
Add AAA comments and doc comments to tests.
mdaigle May 12, 2026
220e945
use auto property
mdaigle May 13, 2026
03ebdf0
Improve test
mdaigle May 13, 2026
ad0649d
Walk back some refactors to reduce changes.
mdaigle May 13, 2026
635ddcf
move dependencies and use latest versions.
mdaigle May 13, 2026
c18c227
Add missing dependency.
mdaigle May 13, 2026
7e241b1
Add timeprovider package to manual tests.
mdaigle May 14, 2026
fed27de
Avoid using ADP time methods.
mdaigle May 14, 2026
aad4c94
Fix build warning.
mdaigle May 14, 2026
21f5696
Add comments for future work.
mdaigle May 14, 2026
535e242
Address copilot comments and fix tests.
mdaigle May 14, 2026
16494ad
Clean up child timer creation.
mdaigle May 14, 2026
dc7bfa1
Improve error message checking.
mdaigle May 14, 2026
0ed523e
Add legacy pool unit tests for timeout propagation.
mdaigle May 14, 2026
df97c19
Make test more reliable and add comments
mdaigle May 20, 2026
0358280
Doc comment improvements.
mdaigle May 21, 2026
e0161df
Isolate funky TdsParser timeout logic.
mdaigle May 21, 2026
70db022
Shift logic inside PendingGetConnection
mdaigle May 21, 2026
d8d7127
Add feature switch to enable new timeout lifecycle.
mdaigle May 21, 2026
16d1133
Make tests more reliable. Mark one as flaky and add a condition for i…
mdaigle May 22, 2026
fcb6acf
Remove flaky test. Improve test coverage of milliseconds remaining.
mdaigle May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<!-- SqlClient Dependencies -->

<ItemGroup>
<PackageVersion Include="Microsoft.Bcl.TimeProvider" Version="10.0.8" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI" Version="6.0.2" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI.runtime" Version="6.0.2" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="8.16.0" />
Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@
<Reference Include="System.Transactions" />

<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Bcl.TimeProvider" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI" ExcludeAssets="compile" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
Expand All @@ -282,6 +283,7 @@
<!-- References for netcore -->
<ItemGroup Condition="'$(TargetFramework)' != 'net462'">
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Bcl.TimeProvider" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
Expand Down
21 changes: 13 additions & 8 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ protected internal override Task<DataTable> GetSchemaAsync(
internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry) =>
TryOpenConnectionInternal(outerConnection, connectionFactory, retry);
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout) =>
TryOpenConnectionInternal(outerConnection, connectionFactory, retry, timeout);

/// <inheritdoc/>
internal override void ResetConnection() => throw ADP.ClosedConnectionError();
Expand All @@ -78,7 +79,8 @@ protected DbConnectionBusy(ConnectionState state) : base(state, true, false)
internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry)
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout)
=> throw ADP.ConnectionAlreadyOpen(State);
}

Expand Down Expand Up @@ -119,13 +121,15 @@ internal override void CloseConnection(DbConnection owningObject, SqlConnectionF
internal override bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry) =>
TryOpenConnection(outerConnection, connectionFactory, retry);
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout) =>
TryOpenConnection(outerConnection, connectionFactory, retry, timeout);

internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry)
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout)
{
if (retry == null || !retry.Task.IsCompleted)
{
Expand Down Expand Up @@ -173,7 +177,8 @@ private DbConnectionClosedPreviouslyOpened() : base(ConnectionState.Closed, true
internal override bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry) =>
TryOpenConnection(outerConnection, connectionFactory, retry);
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout) =>
TryOpenConnection(outerConnection, connectionFactory, retry, timeout);
}
}
19 changes: 7 additions & 12 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,14 +686,6 @@ internal void MakePooledConnection(IDbConnectionPool connectionPool)
Pool = connectionPool;
}

internal virtual void OpenConnection(DbConnection outerConnection, SqlConnectionFactory connectionFactory)
Copy link
Copy Markdown
Contributor Author

@mdaigle mdaigle May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is unused. Not sure why it was still here.

{
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
Expand Down Expand Up @@ -800,15 +792,17 @@ internal void SetInStasis()
internal virtual bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry)
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout)
{
throw ADP.ConnectionAlreadyOpen(State);
}

internal virtual bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry)
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout)
{
throw ADP.MethodNotImplemented();
}
Expand Down Expand Up @@ -910,7 +904,8 @@ protected virtual void ReleaseAdditionalLocksForClose(bool lockToken)
protected bool TryOpenConnectionInternal(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry)
TaskCompletionSource<DbConnectionInternal> retry,
TimeoutTimer timeout)
{
// ?->Connecting: prevent set_ConnectionString during Open
if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this))
Expand All @@ -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;
}
Expand Down
Loading
Loading