From 744bd32903a6dfc4677c1a416a3a00a20b4be0cb Mon Sep 17 00:00:00 2001 From: avtc Date: Fri, 21 Dec 2018 20:11:30 +0200 Subject: [PATCH 1/4] Nolock solution for TryGetRegistration, IsRegistered, RegistrationsFor. Issues: #941, #906 --- .../Core/Registration/ComponentRegistry.cs | 47 ++++++++++- .../Registration/ServiceRegistrationInfo.cs | 82 +++++++++++++------ 2 files changed, 102 insertions(+), 27 deletions(-) diff --git a/src/Autofac/Core/Registration/ComponentRegistry.cs b/src/Autofac/Core/Registration/ComponentRegistry.cs index fe594aeaa..04522aca7 100644 --- a/src/Autofac/Core/Registration/ComponentRegistry.cs +++ b/src/Autofac/Core/Registration/ComponentRegistry.cs @@ -65,7 +65,7 @@ public class ComponentRegistry : Disposable, IComponentRegistry /// /// Keeps track of the status of registered services. /// - private readonly Dictionary _serviceInfo = new Dictionary(); + private readonly ConcurrentDictionary _serviceInfo = new ConcurrentDictionary(); private readonly ConcurrentDictionary> _decorators = new ConcurrentDictionary>(); @@ -118,13 +118,31 @@ public bool TryGetRegistration(Service service, out IComponentRegistration regis { if (service == null) throw new ArgumentNullException(nameof(service)); + var info = GetInitializedServiceInfoOrDefault(service); + if (info != null && info.TryGetRegistration(out registration)) + { + return true; + } + lock (_synchRoot) { - var info = GetInitializedServiceInfo(service); + info = GetInitializedServiceInfo(service); return info.TryGetRegistration(out registration); } } + private IComponentRegistration GetIfRegisteredAndInitializedOrDefault(Service service) + { + var info = GetInitializedServiceInfoOrDefault(service); + if (info != null && info.IsInitialized) + { + if (info.TryGetRegistration(out var registration)) + return registration; + } + + return null; + } + /// /// Determines whether the specified service is registered. /// @@ -134,6 +152,10 @@ public bool IsRegistered(Service service) { if (service == null) throw new ArgumentNullException(nameof(service)); + var info = GetInitializedServiceInfoOrDefault(service); + if (info != null && info.IsRegistered) + return true; + lock (_synchRoot) { return GetInitializedServiceInfo(service).IsRegistered; @@ -233,9 +255,15 @@ public IEnumerable RegistrationsFor(Service service) { if (service == null) throw new ArgumentNullException(nameof(service)); + var info = GetInitializedServiceInfoOrDefault(service); + if (info != null) + { + return info.Implementations.ToArray(); + } + lock (_synchRoot) { - var info = GetInitializedServiceInfo(service); + info = GetInitializedServiceInfo(service); return info.Implementations.ToArray(); } } @@ -373,10 +401,21 @@ private ServiceRegistrationInfo GetServiceInfo(Service service) return existing; var info = new ServiceRegistrationInfo(service); - _serviceInfo.Add(service, info); + _serviceInfo.TryAdd(service, info); return info; } + /// + /// Returns ServiceInfo only in case it already exists and initialized for nolock resolution. + /// + private ServiceRegistrationInfo GetInitializedServiceInfoOrDefault(Service service) + { + ServiceRegistrationInfo existing; + if (_serviceInfo.TryGetValue(service, out existing) && existing.IsInitialized) + return existing; + return null; + } + private EventHandler GetRegistered() { if (Properties.TryGetValue(MetadataKeys.RegisteredPropertyKey, out var registered)) diff --git a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs index 0a31ceec7..0a91a8b83 100644 --- a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs +++ b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs @@ -27,6 +27,7 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Threading; namespace Autofac.Core.Registration { @@ -41,21 +42,27 @@ internal class ServiceRegistrationInfo /// /// List of implicit default service implementations. Overriding default implementations are appended to the end, /// so the enumeration should begin from the end too, and the most default implementation comes last. + /// Used from multithreaded environment, use Interlocked/Volatile for read and write new List each time. /// - private readonly List _defaultImplementations = new List(); + private List _defaultImplementations = new List(); /// /// List of service implementations coming from sources. Sources have priority over preserve-default implementations. /// Implementations from sources are enumerated in preserve-default order, so the most default implementation comes first. + /// Used from multithreaded environment, use Interlocked/Volatile for read and write new List each time. /// private List _sourceImplementations = null; /// /// List of explicit service implementations specified with the PreserveExistingDefaults option. /// Enumerated in preserve-defaults order, so the most default implementation comes first. + /// Used from multithreaded environment, use Interlocked/Volatile for read and write new List each time. /// private List _preserveDefaultImplementations = null; + /// + /// Used from multithreaded environment, use Interlocked/Volatile for read and write each time. + /// [SuppressMessage("Microsoft.ApiDesignGuidelines", "CA2213", Justification = "The creator of the compponent registration is responsible for disposal.")] private IComponentRegistration _defaultImplementation; @@ -73,12 +80,25 @@ public ServiceRegistrationInfo(Service service) _service = service; } + private volatile bool _isInitialized; + /// /// Gets a value indicating whether the first time a service is requested, initialization (e.g. reading from sources) /// happens. This value will then be set to true. Calling many methods on this type before /// initialisation is an error. /// - public bool IsInitialized { get; private set; } + public bool IsInitialized + { + get + { + return _isInitialized; + } + + private set + { + _isInitialized = value; + } + } /// /// Gets the known implementations. The first implementation is a default one. @@ -88,15 +108,17 @@ public IEnumerable Implementations get { RequiresInitialization(); - var resultingCollection = Enumerable.Reverse(_defaultImplementations); - if (_sourceImplementations != null) + var resultingCollection = Enumerable.Reverse(Volatile.Read(ref _defaultImplementations)); + var sourceImplementations = Volatile.Read(ref _sourceImplementations); + if (sourceImplementations != null) { - resultingCollection = resultingCollection.Concat(_sourceImplementations); + resultingCollection = resultingCollection.Concat(sourceImplementations); } - if (_preserveDefaultImplementations != null) + var preserveDefaultImplementations = Volatile.Read(ref _preserveDefaultImplementations); + if (preserveDefaultImplementations != null) { - resultingCollection = resultingCollection.Concat(_preserveDefaultImplementations); + resultingCollection = resultingCollection.Concat(preserveDefaultImplementations); } return resultingCollection; @@ -122,9 +144,9 @@ public bool IsRegistered } private bool Any => - _defaultImplementations.Count > 0 || - _sourceImplementations != null || - _preserveDefaultImplementations != null; + Volatile.Read(ref _defaultImplementations).Count > 0 || + Volatile.Read(ref _sourceImplementations) != null || + Volatile.Read(ref _preserveDefaultImplementations) != null; public void AddImplementation(IComponentRegistration registration, bool preserveDefaults, bool originatedFromSource) { @@ -132,21 +154,29 @@ public void AddImplementation(IComponentRegistration registration, bool preserve { if (originatedFromSource) { - if (_sourceImplementations == null) + // called inside of lock, just making List _sourceImplementations thread safe + if (Volatile.Read(ref _sourceImplementations) == null) { - _sourceImplementations = new List(); + Volatile.Write(ref _sourceImplementations, new List()); } - _sourceImplementations.Add(registration); + var newSourceImplementations = new List(Volatile.Read(ref _sourceImplementations)); + newSourceImplementations.Add(registration); + + Volatile.Write(ref _sourceImplementations, newSourceImplementations); } else { - if (_preserveDefaultImplementations == null) + // called inside of lock, just making List _preserveDefaultImplementations thread safe + if (Volatile.Read(ref _preserveDefaultImplementations) == null) { - _preserveDefaultImplementations = new List(); + Volatile.Write(ref _preserveDefaultImplementations, new List()); } - _preserveDefaultImplementations.Add(registration); + var newpreserveDefaultImplementations = new List(Volatile.Read(ref _preserveDefaultImplementations)); + newpreserveDefaultImplementations.Add(registration); + + Volatile.Write(ref _preserveDefaultImplementations, newpreserveDefaultImplementations); } } else @@ -154,20 +184,26 @@ public void AddImplementation(IComponentRegistration registration, bool preserve if (originatedFromSource) throw new ArgumentOutOfRangeException(nameof(originatedFromSource)); - _defaultImplementations.Add(registration); + var newDefaultImplementations = new List(Volatile.Read(ref _defaultImplementations)); + newDefaultImplementations.Add(registration); + + Volatile.Write(ref _defaultImplementations, newDefaultImplementations); } - _defaultImplementation = null; + Volatile.Write(ref _defaultImplementation, null); } public bool TryGetRegistration(out IComponentRegistration registration) { RequiresInitialization(); - registration = _defaultImplementation ?? (_defaultImplementation = - _defaultImplementations.LastOrDefault() ?? - _sourceImplementations?.First() ?? - _preserveDefaultImplementations?.First()); + registration = Interlocked.CompareExchange( + ref _defaultImplementation, + Volatile.Read(ref _defaultImplementations).LastOrDefault() ?? Volatile.Read(ref _sourceImplementations)?.First() ?? Volatile.Read(ref _preserveDefaultImplementations)?.First(), + null); + + if (registration == null) + registration = Volatile.Read(ref _defaultImplementation); return registration != null; } @@ -236,7 +272,7 @@ public bool ShouldRecalculateAdaptersOn(IComponentRegistration registration) // - Have already been initialized // - Were created via a registration source (because we might be adding an equivalent explicit registration such as Func) // - Don't contain any registrations (because a registration source was added when no adaptee was present) - return IsInitialized && (_sourceImplementations != null || !Any); + return IsInitialized && (Volatile.Read(ref _sourceImplementations) != null || !Any); } } } \ No newline at end of file From f1bfdf855b95ace36e6370e98cd237af7583369d Mon Sep 17 00:00:00 2001 From: avtc Date: Fri, 21 Dec 2018 20:30:02 +0200 Subject: [PATCH 2/4] Remove recently added excessive method --- .../Core/Registration/ComponentRegistry.cs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/Autofac/Core/Registration/ComponentRegistry.cs b/src/Autofac/Core/Registration/ComponentRegistry.cs index 04522aca7..4c4c4c1fa 100644 --- a/src/Autofac/Core/Registration/ComponentRegistry.cs +++ b/src/Autofac/Core/Registration/ComponentRegistry.cs @@ -118,6 +118,7 @@ public bool TryGetRegistration(Service service, out IComponentRegistration regis { if (service == null) throw new ArgumentNullException(nameof(service)); + // get without lock if already registered var info = GetInitializedServiceInfoOrDefault(service); if (info != null && info.TryGetRegistration(out registration)) { @@ -131,18 +132,6 @@ public bool TryGetRegistration(Service service, out IComponentRegistration regis } } - private IComponentRegistration GetIfRegisteredAndInitializedOrDefault(Service service) - { - var info = GetInitializedServiceInfoOrDefault(service); - if (info != null && info.IsInitialized) - { - if (info.TryGetRegistration(out var registration)) - return registration; - } - - return null; - } - /// /// Determines whether the specified service is registered. /// @@ -152,9 +141,12 @@ public bool IsRegistered(Service service) { if (service == null) throw new ArgumentNullException(nameof(service)); + // get without lock if already registered var info = GetInitializedServiceInfoOrDefault(service); if (info != null && info.IsRegistered) + { return true; + } lock (_synchRoot) { @@ -255,6 +247,7 @@ public IEnumerable RegistrationsFor(Service service) { if (service == null) throw new ArgumentNullException(nameof(service)); + // get without lock if already registered var info = GetInitializedServiceInfoOrDefault(service); if (info != null) { From 86787132ba244bea28f13b02545998c047e500b2 Mon Sep 17 00:00:00 2001 From: avtc Date: Fri, 21 Dec 2018 20:36:38 +0200 Subject: [PATCH 3/4] Fix proper case --- src/Autofac/Core/Registration/ServiceRegistrationInfo.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs index 0a91a8b83..c939e4cb6 100644 --- a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs +++ b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs @@ -173,10 +173,10 @@ public void AddImplementation(IComponentRegistration registration, bool preserve Volatile.Write(ref _preserveDefaultImplementations, new List()); } - var newpreserveDefaultImplementations = new List(Volatile.Read(ref _preserveDefaultImplementations)); - newpreserveDefaultImplementations.Add(registration); + var newPreserveDefaultImplementations = new List(Volatile.Read(ref _preserveDefaultImplementations)); + newPreserveDefaultImplementations.Add(registration); - Volatile.Write(ref _preserveDefaultImplementations, newpreserveDefaultImplementations); + Volatile.Write(ref _preserveDefaultImplementations, newPreserveDefaultImplementations); } } else From 5c99efdeecb1110b79915aecc74a9cf35f423133 Mon Sep 17 00:00:00 2001 From: avtc Date: Sun, 23 Dec 2018 21:36:09 +0200 Subject: [PATCH 4/4] Better way of converting coalesce --- .../Registration/ServiceRegistrationInfo.cs | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs index c939e4cb6..7d390d8e2 100644 --- a/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs +++ b/src/Autofac/Core/Registration/ServiceRegistrationInfo.cs @@ -197,15 +197,35 @@ public bool TryGetRegistration(out IComponentRegistration registration) { RequiresInitialization(); - registration = Interlocked.CompareExchange( - ref _defaultImplementation, - Volatile.Read(ref _defaultImplementations).LastOrDefault() ?? Volatile.Read(ref _sourceImplementations)?.First() ?? Volatile.Read(ref _preserveDefaultImplementations)?.First(), - null); + registration = Volatile.Read(ref _defaultImplementation); + if (registration != null) + return true; - if (registration == null) - registration = Volatile.Read(ref _defaultImplementation); + var defaultImplementations = Volatile.Read(ref _defaultImplementations); + if (defaultImplementations.Count > 0) + { + registration = defaultImplementations[defaultImplementations.Count - 1]; + Volatile.Write(ref _defaultImplementation, registration); + return true; + } + + var sourceImplementations = Volatile.Read(ref _sourceImplementations); + if (sourceImplementations != null && sourceImplementations.Count > 0) + { + registration = sourceImplementations[0]; + Volatile.Write(ref _defaultImplementation, registration); + return true; + } + + var preserveDefaultImplementations = Volatile.Read(ref _preserveDefaultImplementations); + if (preserveDefaultImplementations != null && preserveDefaultImplementations.Count > 0) + { + registration = preserveDefaultImplementations[0]; + Volatile.Write(ref _defaultImplementation, registration); + return true; + } - return registration != null; + return false; } public void Include(IRegistrationSource source)