Fix race conditions with resolving singletons in DI (#47310)

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteRuntimeResolver.cs

@@ -7,6 +7,7 @@
 using System.Reflection;
 using System.Runtime.ExceptionServices;
 using System.Threading;
+using System.Diagnostics;
 
 namespace Microsoft.Extensions.DependencyInjection.ServiceLookup
 {
@@ -62,11 +63,8 @@ protected override object VisitRootCache(ServiceCallSite callSite, RuntimeResolv
             var lockType = RuntimeResolverLock.Root;
             bool lockTaken = false;
 
-            if ((context.AcquiredLocks & lockType) == 0)
-            {
-                // using more granular locking (per singleton) for the root
-                Monitor.Enter(callSite, ref lockTaken);
-            }
+            // using more granular locking (per singleton) for the root
+            Monitor.Enter(callSite, ref lockTaken);
             try
             {
                 return ResolveService(callSite, context, lockType, serviceProviderEngine: context.Scope.Engine.Root);
@@ -92,7 +90,7 @@ protected override object VisitScopeCache(ServiceCallSite callSite, RuntimeResol
         private object VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
         {
             bool lockTaken = false;
-            Dictionary<ServiceCacheKey, object> resolvedServices = serviceProviderEngine.ResolvedServices;
+            IDictionary<ServiceCacheKey, object> resolvedServices = serviceProviderEngine.ResolvedServices;
 
             // Taking locks only once allows us to fork resolution process
             // on another thread without causing the deadlock because we
@@ -118,19 +116,27 @@ private object VisitCache(ServiceCallSite callSite, RuntimeResolverContext conte
 
         private object ResolveService(ServiceCallSite callSite, RuntimeResolverContext context, RuntimeResolverLock lockType, ServiceProviderEngineScope serviceProviderEngine)
         {
-            Dictionary<ServiceCacheKey, object> resolvedServices = serviceProviderEngine.ResolvedServices;
-            if (!resolvedServices.TryGetValue(callSite.Cache.Key, out object resolved))
-            {
-                resolved = VisitCallSiteMain(callSite, new RuntimeResolverContext
-                {
-                    Scope = serviceProviderEngine,
-                    AcquiredLocks = context.AcquiredLocks | lockType
-                });
+            IDictionary<ServiceCacheKey, object> resolvedServices = serviceProviderEngine.ResolvedServices;
 
-                serviceProviderEngine.CaptureDisposable(resolved);
-                resolvedServices.Add(callSite.Cache.Key, resolved);
+            // Note: This method has already taken lock by the caller for resolution and access synchronization.
+            // For root: uses a concurrent dictionary and takes a per singleton lock for resolution.
+            // For scoped: takes a dictionary as both a resolution lock and a dictionary access lock.
+            Debug.Assert(
+                (lockType == RuntimeResolverLock.Root && resolvedServices is ConcurrentDictionary<ServiceCacheKey, object>) ||
+                (lockType == RuntimeResolverLock.Scope && Monitor.IsEntered(resolvedServices)));
+
+            if (resolvedServices.TryGetValue(callSite.Cache.Key, out object resolved))
+            {
+                return resolved;
             }
 
+            resolved = VisitCallSiteMain(callSite, new RuntimeResolverContext
+            {
+                Scope = serviceProviderEngine,
+                AcquiredLocks = context.AcquiredLocks | lockType
+            });
+            serviceProviderEngine.CaptureDisposable(resolved);
+            resolvedServices.Add(callSite.Cache.Key, resolved);
             return resolved;
         }
 

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngine.cs

@@ -19,7 +19,7 @@ internal abstract class ServiceProviderEngine : IServiceProviderEngine, IService
         protected ServiceProviderEngine(IEnumerable<ServiceDescriptor> serviceDescriptors)
         {
             _createServiceAccessor = CreateServiceAccessor;
-            Root = new ServiceProviderEngineScope(this);
+            Root = new ServiceProviderEngineScope(this, isRoot: true);
             RuntimeResolver = new CallSiteRuntimeResolver();
             CallSiteFactory = new CallSiteFactory(serviceDescriptors);
             CallSiteFactory.Add(typeof(IServiceProvider), new ServiceProviderCallSite());

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading.Tasks;
@@ -19,12 +20,15 @@ internal class ServiceProviderEngineScope : IServiceScope, IServiceProvider, IAs
         private bool _disposed;
         private readonly object _disposelock = new object();
 
-        public ServiceProviderEngineScope(ServiceProviderEngine engine)
+        public ServiceProviderEngineScope(ServiceProviderEngine engine, bool isRoot = false)
         {
             Engine = engine;
+
+            // To reduce lock contention for singletons upon resolve we use a concurrent dictionary.
+            ResolvedServices = isRoot ? new ConcurrentDictionary<ServiceCacheKey, object>() : new Dictionary<ServiceCacheKey, object>();
         }
 
-        internal Dictionary<ServiceCacheKey, object> ResolvedServices { get; } = new Dictionary<ServiceCacheKey, object>();
+        internal IDictionary<ServiceCacheKey, object> ResolvedServices { get; }
 
         public ServiceProviderEngine Engine { get; }