Skip to content

Get/Return pooled connections #3404

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Get/Return pooled connections #3404

Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
87a1129
Add get flow and basic tests.
mdaigle Jun 6, 2025
4233f94
Add tests. Clean up diff.
mdaigle Jun 6, 2025
fb8c9d0
Add additional tests. Refactor to test error behavior.
mdaigle Jun 9, 2025
73a379b
Implement return flow and add return/reuse tests.
mdaigle Jun 9, 2025
7ac22f6
Clean up comments and todos.
mdaigle Jun 9, 2025
7c75930
Add test cases.
mdaigle Jun 9, 2025
6dde5b9
Enable init property accessor in netfx.
mdaigle Jun 12, 2025
9999bf7
Switch to auto properties.
mdaigle Jun 12, 2025
931ac03
Reorder class members.
mdaigle Jun 12, 2025
7d734f9
Address review comments.
mdaigle Jun 12, 2025
12a8121
Merge main
mdaigle Jun 12, 2025
ec27e63
Fix channels package resolution.
mdaigle Jun 12, 2025
c4d5808
Review changes.
mdaigle Jun 16, 2025
0451b50
Set CreateTime in constructor. This makes sense becuase CreateTime ca…
mdaigle Jun 20, 2025
3f79f99
Naming
mdaigle Jun 20, 2025
2d260bb
Fix potential connection leak when setting TaskCompletionSource result.
mdaigle Jun 20, 2025
20adc00
Fix package reference for central package management.
mdaigle Jun 24, 2025
a440e10
Fix formatting. Refactor get connection loop.
mdaigle Jun 24, 2025
456ea1f
Move concurrent data structures out to dedicated class.
mdaigle Jun 26, 2025
46dd516
Refactor data structure to remove generics, revert to previous logic.
mdaigle Jun 26, 2025
7613b2c
Cleanup
mdaigle Jun 27, 2025
f7b9465
Fix timeout exception handling for async path.
mdaigle Jun 27, 2025
4b79754
Merge main
mdaigle Jun 27, 2025
67d0487
Review comments. Fix merge conflict.
mdaigle Jun 30, 2025
0d9b606
Merge branch 'main' of github.com:dotnet/SqlClient into dev/mdaigle/g…
mdaigle Jun 30, 2025
7377e85
Change object reference to full type. Fix tab.
mdaigle Jun 30, 2025
901ba47
cleanup
mdaigle Jun 30, 2025
142f0ca
Add link to github issue for async pathways.
mdaigle Jun 30, 2025
5769604
Remove side effects from IsLiveConnection. Fix exception handling and…
mdaigle Jul 1, 2025
bc8b094
Minor review feedback. Make tests more reliable.
mdaigle Jul 7, 2025
92528d3
Fix compilation
mdaigle Jul 7, 2025
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 src/Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<PackageVersion Include="System.Memory" Version="4.5.5" />
<PackageVersion Include="System.Data.Common" Version="4.3.0" />
<PackageVersion Include="System.Text.Encodings.Web" Version="8.0.0" />
<PackageVersion Include="System.Threading.Channels" Version="8.0.0" />
</ItemGroup>
<!-- NetFx and NetCore project dependencies -->
<ItemGroup>
Expand Down
5 changes: 5 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
<!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. -->
<PackageReference Include="System.Text.Json" />
</ItemGroup>

<!-- netstandard dependencies -->
<ItemGroup Condition="'$(TargetGroup)' != 'netcoreapp'">
<PackageReference Include="System.Threading.Channels" />
</ItemGroup>

<Import Project="$(ToolsDir)targets\ResolveContract.targets" Condition="'$(OSGroup)' == 'AnyOS' AND '$(TargetGroup)' != 'netcoreapp'" />
<Import Project="$(ToolsDir)targets\NotSupported.targets" Condition="'$(OSGroup)' == 'AnyOS' AND '$(TargetGroup)' != 'netcoreapp'" />
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\ChannelDbConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\ChannelDbConnectionPool.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\ConnectionPoolSlots.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\ConnectionPoolSlots.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\DbConnectionPoolAuthenticationContext.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\DbConnectionPoolAuthenticationContext.cs</Link>
</Compile>
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<PackageReference Include="System.Security.Cryptography.Pkcs" />
<PackageReference Include="System.Text.Encodings.Web" />
<PackageReference Include="System.Text.Json" />
<PackageReference Include="System.Threading.Channels" />
</ItemGroup>
<Import Project="$(ToolsDir)targets\TrimDocsForIntelliSense.targets" />
</Project>
4 changes: 4 additions & 0 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\ChannelDbConnectionPool.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\ChannelDbConnectionPool.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\ConnectionPoolSlots.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\ConnectionPoolSlots.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ConnectionPool\DbConnectionPoolAuthenticationContext.cs">
<Link>Microsoft\Data\SqlClient\ConnectionPool\DbConnectionPoolAuthenticationContext.cs</Link>
</Compile>
Expand Down Expand Up @@ -974,6 +977,7 @@
<PackageReference Include="System.Security.Cryptography.Pkcs" />
<PackageReference Include="System.Text.Encodings.Web" />
<PackageReference Include="System.Text.Json" />
<PackageReference Include="System.Threading.Channels" />
</ItemGroup>
<Import Project="$(CommonSourceRoot)tools\targets\GenerateResourceStringsSource.targets" />
<Import Project="$(NetFxSource)tools\targets\GenerateThisAssemblyCs.targets" />
Expand Down
3 changes: 2 additions & 1 deletion 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 @@ -20,7 +20,8 @@
<Reference Include="System.Configuration.ConfigurationManager" />
<Reference Include="System.Transactions" />
<PackageReference Include="System.Data.Common" />
</ItemGroup>
<PackageReference Include="System.Threading.Channels" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' != 'net462'">
<!-- References that only apply to netcore -->
<PackageReference Include="System.Configuration.ConfigurationManager" />
Expand Down
16 changes: 8 additions & 8 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 @@ -46,11 +46,6 @@ internal abstract class DbConnectionInternal
/// </summary>
private bool _cannotBePooled;

/// <summary>
/// When the connection was created.
/// </summary>
private DateTime _createTime;

/// <summary>
/// [usage must be thread-safe] the transaction that we're enlisted in, either manually or automatically.
/// </summary>
Expand Down Expand Up @@ -93,10 +88,16 @@ internal DbConnectionInternal(ConnectionState state, bool hidePassword, bool all
AllowSetConnectionString = allowSetConnectionString;
ShouldHidePassword = hidePassword;
State = state;
CreateTime = DateTime.UtcNow;
}

#region Properties

/// <summary>
/// When the connection was created.
/// </summary>
internal DateTime CreateTime { get; init; }
Copy link
Contributor

@paulmedynski paulmedynski Jul 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

init means "Must be assigned during construction", but doesn't restrict that assignment to a constructor body. A caller could do this:

DbConnectionInternal conn = new(...) { CreateTime = DateTime.UnixEpoch; }

The initializer body runs after the constructor body, but still during construction. This isn't what we want.

I think all you need is:

internal DateTime CreateTime { get; }

That will make CrateTime immutable - you will be forced to assign it in all constructor bodies, and then prohibited from assigning to it anywhere else.

I suspect this applies to all of your use of init in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I didn't realize the init properties could be set in that way.


internal bool AllowSetConnectionString { get; }

internal bool CanBePooled => !IsConnectionDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out _);
Expand Down Expand Up @@ -531,7 +532,7 @@ internal void DeactivateConnection()
// If we're not already doomed, check the connection's lifetime and
// doom it if it's lifetime has elapsed.
DateTime now = DateTime.UtcNow;
if (now.Ticks - _createTime.Ticks > Pool.LoadBalanceTimeout.Ticks)
if (now.Ticks - CreateTime.Ticks > Pool.LoadBalanceTimeout.Ticks)
{
DoNotPoolThisConnection();
}
Expand Down Expand Up @@ -701,7 +702,6 @@ internal void MakeNonPooledObject(DbConnection owningObject)
/// <param name="connectionPool"></param>
internal void MakePooledConnection(IDbConnectionPool connectionPool)
{
_createTime = DateTime.UtcNow;
Pool = connectionPool;
}

Expand Down Expand Up @@ -756,7 +756,7 @@ internal virtual void PrepareForReplaceConnection()
// By default, there is no preparation required
}

internal void PrePush(object expectedOwner)
internal void PrePush(DbConnection expectedOwner)
{
// Called by IDbConnectionPool when we're about to be put into it's pool, we take this
// opportunity to ensure ownership and pool counts are legit.
Expand Down
Loading
Loading