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
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
@Abde1rahman1
Copy link
Author

@AndriySvyryd , Done. I've addressed all comments: updated the XML documentation with the concurrency warning, removed the redundant extension method as requested, and enhanced the test to verify functional behavior by asserting that a new service provider instance is created after clearing the cache

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