Skip to content
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.

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

Merge DbConnectionPool #2812

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

edwardneal
Copy link
Contributor

Contributes to #1261.

DbConnectionPool is almost identical between .NET Core and Framework. When combined with #2410, this should mean that everything in the ProviderBase folders besides DbConnectionPoolCounters can be merged.

Core differences here:

  • In DbConnectionPool.CleanupCallback, Semaphore.WaitOne used an overload with an exitContext parameter in Framework but not in Core. This parameter seems to be ignored in .NET Core, but I've included the change for the sake of .NET Framework.
  • DbConnectionPool.CreateCleanupTimer returned ADP.UnsafeCreateTimer in .NET Core but created a Timer instance directly in .NET Framework. I've adopted the .NET Core methodology - the comment for UnsafeCreateTimer indicates that it's used to prevent the current ExecutionContext from flowing into a global timer and not being GCed, which seems reasonable.

More generally, there are a lot of conditionally-compiled blocks to enable the differing metrics implementations. Hopefully these will cease to exist when the OTel metrics are implemented.

@DavoudEshtehari DavoudEshtehari added this to the 6.0-preview2 milestone Aug 23, 2024
@DavoudEshtehari DavoudEshtehari added the ➕ Code Health Changes related to source code improvements label Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Changes related to source code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants