⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@Abde1rahman1
Copy link

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:

Summary

This PR implements a "pubternal" way to clear the internal service provider cache, as requested in #27189. This allows developers to resolve memory leaks and state corruption issues, especially in heavy testing scenarios where many internal service providers are created.

Changes

  • Added Clear() to ServiceProviderCache to allow clearing the internal _configurations dictionary.

  • Added a static ClearServiceProviderCache() method to EntityFrameworkMetrics in the Infrastructure.Internal namespace.

  • Added a unit test in ServiceProviderCacheTest to ensure the cache is effectively cleared.

  • Tests for the changes have been added (for bug fixes / features)

  • Code follows the same patterns and style as existing code in this repo

@Abde1rahman1 Abde1rahman1 requested a review from a team as a code owner January 12, 2026 13:21
@AndriySvyryd AndriySvyryd self-assigned this Jan 12, 2026
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static void ClearServiceProviderCache() => ServiceProviderCache.Instance.Clear();
Copy link
Member

Choose a reason for hiding this comment

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

Remove this and the extension method.
ServiceProviderCache.Instance.Clear() can be called directly

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a mechanism to clear the internal EF Core service provider cache to address memory leaks in testing scenarios. The implementation adds a Clear() method to ServiceProviderCache and exposes it through a static method on EntityFrameworkMetrics and an extension method on DatabaseFacade.

Changes:

  • Added Clear() method to ServiceProviderCache to clear the internal configurations dictionary
  • Added static ClearServiceProviderCache() entry point (intended but incomplete)
  • Added DatabaseFacade extension method to provide a public API surface
  • Added unit test to verify cache clearing functionality

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/EFCore/Internal/ServiceProviderCache.cs Adds virtual Clear() method to clear the internal _configurations dictionary
src/EFCore/Infrastructure/Internal/EntityFrameworkMetrics.cs Adds using statement for ServiceProviderCache namespace (but missing the actual static method implementation)
src/EFCore/Infrastructure/Internal/InfrastructureExtensions.cs Adds DatabaseFacade extension method to call EntityFrameworkMetrics.ClearServiceProviderCache()
test/EFCore.Tests/ServiceProviderCacheTest.cs Adds test that verifies cache clearing using reflection to check internal state

Comment on lines +51 to +55
/// <summary>
/// Clears the internal Entity Framework service provider cache.
/// </summary>
public static void ClearServiceProviderCache(this DatabaseFacade databaseFacade)
=> EntityFrameworkMetrics.ClearServiceProviderCache();
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

The ClearServiceProviderCache extension method is missing the required internal API XML documentation comment. According to EF Core coding guidelines, all members in Internal namespaces must include the standard internal API documentation comment that warns users about using internal APIs.

Copilot generated this review using guidance from repository custom instructions.
@roji roji force-pushed the main branch 2 times, most recently from 249ae47 to 6b86657 Compare January 13, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants