Skip to content

Conversation

@davidfowl
Copy link
Member

  • Scoped services resolved from the root container are singletons. This fixes a regression that caused the dynamically compiled callsite for scoped service to create new instances for scoped services resolved from root providers.
  • Added a test and added an optimized path that does not compile singletons and promoted singletons using the background thread.

Fixes dotnet/efcore#24824

…gltons

- Scoped services resolved from the root container are singletons. This fixes a regression that caused the dynamically compiled callsite for scoped service to create new instances for scoped services resolved from root providers.
- Added a test and added an optimized path that does not compile singletons and promoted singletons using the background thread.
@ghost
Copy link

ghost commented May 8, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Scoped services resolved from the root container are singletons. This fixes a regression that caused the dynamically compiled callsite for scoped service to create new instances for scoped services resolved from root providers.
  • Added a test and added an optimized path that does not compile singletons and promoted singletons using the background thread.

Fixes dotnet/efcore#24824

Author: davidfowl
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

@davidfowl davidfowl requested a review from maryamariyan May 8, 2021 00:03
for (int i = 0; i < 10; i++)
{
Assert.Same(singleton, sp.GetRequiredService<A>());
Thread.Sleep(10); // Give the background thread time to compile
Copy link
Member Author

Choose a reason for hiding this comment

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

😨

}

[Fact]
public void ScopedServiceResolvedFromSingletonAfterCompilation()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also want to test a use case where a scoped service is registered in a scoped service provider and is depending on a singleton service?

Copy link
Member Author

Choose a reason for hiding this comment

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

With the same loop? I want to say I'm sure we have that scenario covered, but never say never 😄

@davidfowl davidfowl merged commit 4bc509e into main May 8, 2021
@davidfowl
Copy link
Member Author

Merging to unblock upstream

@eerhardt eerhardt deleted the davidfowl/scoped-singletons branch May 10, 2021 14:26
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants