Avoid replacing realized service cache for singletons and promoted singletons (#52484)

- 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.

diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/DynamicServiceProviderEngine.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/DynamicServiceProviderEngine.cs
index f8153ba..fa41c2d 100644 (file)
--- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/DynamicServiceProviderEngine.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/DynamicServiceProviderEngine.cs
@@ -19,12 +19,27 @@ namespace Microsoft.Extensions.DependencyInjection.ServiceLookup
             int callCount = 0;
             return scope =>
             {
+                // We want to directly use the callsite value if it's set and the scope is the root scope.
+                // We've already called into the RuntimeResolver and pre-computed any singletons or root scope
+                // Avoid the compilation for singletons (or promoted singletons)
+                if (scope.IsRootScope && callSite.Value != null)
+                {
+                    return callSite.Value;
+                }
+
                 // Resolve the result before we increment the call count, this ensures that singletons
                 // won't cause any side effects during the compilation of the resolve function.
                 var result = RuntimeResolver.Resolve(callSite, scope);
 
                 if (Interlocked.Increment(ref callCount) == 2)
                 {
+                    // This second check is to avoid the race where we end up kicking off a background thread
+                    // if multiple calls to GetService race and resolve the values for singletons before the initial check above.
+                    if (scope.IsRootScope && callSite.Value != null)
+                    {
+                        return callSite.Value;
+                    }
+
                     // Don't capture the ExecutionContext when forking to build the compiled version of the
                     // resolve function
                     _ = ThreadPool.UnsafeQueueUserWorkItem(_ =>

diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs
index fefb711..d1464e5 100644 (file)
--- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs
@@ -43,6 +43,8 @@ namespace Microsoft.Extensions.DependencyInjection.ServiceLookup
 
         public IServiceProvider ServiceProvider => this;
 
+        public bool IsRootScope => this == Engine.Root;
+
         internal object CaptureDisposable(object service)
         {
             _captureDisposableCallback?.Invoke(service);

diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs
index 0ca582c..b0c09d3 100644 (file)
--- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs
@@ -1088,6 +1088,21 @@ namespace Microsoft.Extensions.DependencyInjection.Tests
             }
         }
 
+        [Fact]
+        public void ScopedServiceResolvedFromSingletonAfterCompilation()
+        {
+            ServiceProvider sp = new ServiceCollection()
+                                .AddScoped<A>()
+                                .BuildServiceProvider();
+
+            var singleton = sp.GetRequiredService<A>();
+            for (int i = 0; i < 10; i++)
+            {
+                Assert.Same(singleton, sp.GetRequiredService<A>());
+                Thread.Sleep(10); // Give the background thread time to compile
+            }
+        }
+
         private async Task<bool> ResolveUniqueServicesConcurrently()
         {
             var types = new Type[]