diff --git a/CHANGELOG.md b/CHANGELOG.md index 666008f163..c5e7b6c4c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Castle Core Changelog +## Unreleased + +Deprecations: +- The API surrounding `Lock` has been deprecated. This consists of the members listed below. Consider using the Base Class Library's `System.Threading.ReaderWriterLockSlim` instead. (@stakx, #391) + - `Castle.Core.Internal.Lock` (class) + - `Castle.Core.Internal.ILockHolder` (interface) + - `Castle.Core.Internal.IUpgradeableLockHolder` (interface) +- The proxy type cache in `ModuleScope` should no longer be accessed directly. For this reason, the members listed below have been deprecated. (@stakx, #391) + - `Castle.DynamicProxy.ModuleScope.Lock` (property) + - `Castle.DynamicProxy.ModuleScope.GetFromCache` (method) + - `Castle.DynamicProxy.ModuleScope.RegisterInCache` (method) + - `Castle.DynamicProxy.Generators.BaseProxyGenerator.AddToCache` (method) + - `Castle.DynamicProxy.Generators.BaseProxyGenerator.GetFromCache` (method) + - `Castle.DynamicProxy.Generators.CacheKey` (class) + - `Castle.DynamicProxy.Serialization.CacheMappingsAttribute.ApplyTo` (method) + - `Castle.DynamicProxy.Serialization.CacheMappingsAttribute.GetDeserializedMappings` (method) + ## 4.3.1 (2018-06-21) Enhancements: diff --git a/buildscripts/common.props b/buildscripts/common.props index 26cc063233..066ebb4646 100644 --- a/buildscripts/common.props +++ b/buildscripts/common.props @@ -2,6 +2,7 @@ $(NoWarn);CS1591;CS3014;CS3003;CS3001;CS3021 + $(NoWarn);CS0612;CS0618 git https://github.com/castleproject/Core 0.0.0 diff --git a/src/Castle.Core.Tests/Core.Tests/Internal/SlimReadWriteLockTestCase.cs b/src/Castle.Core.Tests/Core.Tests/Internal/SlimReadWriteLockTestCase.cs index b3fb92b116..79a381aeb5 100644 --- a/src/Castle.Core.Tests/Core.Tests/Internal/SlimReadWriteLockTestCase.cs +++ b/src/Castle.Core.Tests/Core.Tests/Internal/SlimReadWriteLockTestCase.cs @@ -14,11 +14,13 @@ namespace Castle.Core.Internal.Tests { + using System; using System.Threading; using NUnit.Framework; [TestFixture] + [Obsolete] public class SlimReadWriteLockTestCase { private SlimReadWriteLock @lock; diff --git a/src/Castle.Core/Components.DictionaryAdapter/DictionaryAdapterFactory.cs b/src/Castle.Core/Components.DictionaryAdapter/DictionaryAdapterFactory.cs index 3371cd1d82..387f265ff5 100644 --- a/src/Castle.Core/Components.DictionaryAdapter/DictionaryAdapterFactory.cs +++ b/src/Castle.Core/Components.DictionaryAdapter/DictionaryAdapterFactory.cs @@ -36,9 +36,8 @@ namespace Castle.Components.DictionaryAdapter /// public class DictionaryAdapterFactory : IDictionaryAdapterFactory { - private readonly Dictionary interfaceToMeta = new Dictionary(); - - private readonly Lock interfaceToMetaLock = Lock.Create(); + private readonly SynchronizedDictionary interfaceToMeta = + new SynchronizedDictionary(); #region IDictionaryAdapterFactory @@ -130,35 +129,21 @@ private DictionaryAdapterMeta InternalGetAdapterMeta(Type type, if (type.GetTypeInfo().IsInterface == false) throw new ArgumentException("Only interfaces can be adapted to a dictionary", "type"); - DictionaryAdapterMeta meta; - - using (interfaceToMetaLock.ForReading()) - { - if (interfaceToMeta.TryGetValue(type, out meta)) - return meta; - } - - using (var heldLock = interfaceToMetaLock.ForReadingUpgradeable()) + return interfaceToMeta.GetOrAdd(type, t => { - if (interfaceToMeta.TryGetValue(type, out meta)) - return meta; - - using (heldLock.Upgrade()) + if (descriptor == null && other != null) { - if (descriptor == null && other != null) - descriptor = other.CreateDescriptor(); + descriptor = other.CreateDescriptor(); + } #if FEATURE_LEGACY_REFLECTION_API - var appDomain = Thread.GetDomain(); - var typeBuilder = CreateTypeBuilder(type, appDomain); + var appDomain = Thread.GetDomain(); + var typeBuilder = CreateTypeBuilder(type, appDomain); #else - var typeBuilder = CreateTypeBuilder(type); + var typeBuilder = CreateTypeBuilder(type); #endif - meta = CreateAdapterMeta(type, typeBuilder, descriptor); - interfaceToMeta.Add(type, meta); - return meta; - } - } + return CreateAdapterMeta(type, typeBuilder, descriptor); + }); } private object InternalGetAdapter(Type type, IDictionary dictionary, PropertyDescriptor descriptor) diff --git a/src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Utilities/SingletonDispenser.cs b/src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Utilities/SingletonDispenser.cs index 3d4cefdfa7..6aee638efc 100644 --- a/src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Utilities/SingletonDispenser.cs +++ b/src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Utilities/SingletonDispenser.cs @@ -18,12 +18,11 @@ namespace Castle.Components.DictionaryAdapter.Xml using System; using System.Collections.Generic; using System.Threading; - using Castle.Core.Internal; public class SingletonDispenser where TItem : class { - private readonly Lock locker; + private readonly ReaderWriterLockSlim locker; private readonly Dictionary items; private readonly Func factory; @@ -32,7 +31,7 @@ public SingletonDispenser(Func factory) if (factory == null) throw Error.ArgumentNull("factory"); - this.locker = new SlimReadWriteLock(); + this.locker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); this.items = new Dictionary(); this.factory = factory; } @@ -53,22 +52,44 @@ private TItem GetOrCreate(TKey key) private bool TryGetExistingItem(TKey key, out object item) { - using (locker.ForReading()) + locker.EnterReadLock(); + try { if (items.TryGetValue(key, out item)) + { return true; + } + } + finally + { + locker.ExitReadLock(); } - using (var hold = locker.ForReadingUpgradeable()) + locker.EnterUpgradeableReadLock(); + try { if (items.TryGetValue(key, out item)) + { return true; - - using (hold.Upgrade()) - items[key] = item = new ManualResetEvent(false); + } + else + { + locker.EnterWriteLock(); + try + { + items[key] = item = new ManualResetEvent(false); + return false; + } + finally + { + locker.ExitWriteLock(); + } + } + } + finally + { + locker.ExitUpgradeableReadLock(); } - - return false; } private TItem WaitForCreate(TKey key, object item) @@ -77,8 +98,15 @@ private TItem WaitForCreate(TKey key, object item) handle.WaitOne(); - using (locker.ForReading()) - return (TItem) items[key]; + locker.EnterReadLock(); + try + { + return (TItem)items[key]; + } + finally + { + locker.ExitReadLock(); + } } private TItem Create(TKey key, object item) @@ -87,8 +115,15 @@ private TItem Create(TKey key, object item) var result = factory(key); - using (locker.ForWriting()) + locker.EnterWriteLock(); + try + { items[key] = result; + } + finally + { + locker.ExitWriteLock(); + } handle.Set(); return result; diff --git a/src/Castle.Core/Core/Internal/ILockHolder.cs b/src/Castle.Core/Core/Internal/ILockHolder.cs index 01c99d9ea8..0e54dd03ca 100644 --- a/src/Castle.Core/Core/Internal/ILockHolder.cs +++ b/src/Castle.Core/Core/Internal/ILockHolder.cs @@ -15,7 +15,10 @@ namespace Castle.Core.Internal { using System; + using System.ComponentModel; + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] public interface ILockHolder:IDisposable { bool LockAcquired { get; } diff --git a/src/Castle.Core/Core/Internal/IUpgradeableLockHolder.cs b/src/Castle.Core/Core/Internal/IUpgradeableLockHolder.cs index f64b779281..cc1f3c5699 100644 --- a/src/Castle.Core/Core/Internal/IUpgradeableLockHolder.cs +++ b/src/Castle.Core/Core/Internal/IUpgradeableLockHolder.cs @@ -14,6 +14,11 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; + + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] public interface IUpgradeableLockHolder : ILockHolder { ILockHolder Upgrade(); diff --git a/src/Castle.Core/Core/Internal/Lock.cs b/src/Castle.Core/Core/Internal/Lock.cs index 079420df80..a33b401619 100644 --- a/src/Castle.Core/Core/Internal/Lock.cs +++ b/src/Castle.Core/Core/Internal/Lock.cs @@ -14,6 +14,12 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; + using System.Threading; + + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] public abstract class Lock { public abstract IUpgradeableLockHolder ForReadingUpgradeable(); @@ -32,5 +38,10 @@ public static Lock Create() { return new SlimReadWriteLock(); } + + internal static Lock CreateFor(ReaderWriterLockSlim underlyingLock) + { + return new SlimReadWriteLock(underlyingLock); + } } } diff --git a/src/Castle.Core/Core/Internal/NoOpLock.cs b/src/Castle.Core/Core/Internal/NoOpLock.cs index db24141d95..c38dd1654e 100644 --- a/src/Castle.Core/Core/Internal/NoOpLock.cs +++ b/src/Castle.Core/Core/Internal/NoOpLock.cs @@ -14,6 +14,11 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; + + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class NoOpLock : ILockHolder { public static readonly ILockHolder Lock = new NoOpLock(); diff --git a/src/Castle.Core/Core/Internal/NoOpUpgradeableLock.cs b/src/Castle.Core/Core/Internal/NoOpUpgradeableLock.cs index c8bbd0f4dc..e350fcfbb2 100644 --- a/src/Castle.Core/Core/Internal/NoOpUpgradeableLock.cs +++ b/src/Castle.Core/Core/Internal/NoOpUpgradeableLock.cs @@ -14,6 +14,11 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; + + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class NoOpUpgradeableLock : IUpgradeableLockHolder { public static readonly IUpgradeableLockHolder Lock = new NoOpUpgradeableLock(); diff --git a/src/Castle.Core/Core/Internal/SlimReadLockHolder.cs b/src/Castle.Core/Core/Internal/SlimReadLockHolder.cs index 698f956cc4..c21471c163 100644 --- a/src/Castle.Core/Core/Internal/SlimReadLockHolder.cs +++ b/src/Castle.Core/Core/Internal/SlimReadLockHolder.cs @@ -14,8 +14,12 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; using System.Threading; + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class SlimReadLockHolder : ILockHolder { private readonly ReaderWriterLockSlim locker; diff --git a/src/Castle.Core/Core/Internal/SlimReadWriteLock.cs b/src/Castle.Core/Core/Internal/SlimReadWriteLock.cs index 3aa07c5651..ed239fd185 100644 --- a/src/Castle.Core/Core/Internal/SlimReadWriteLock.cs +++ b/src/Castle.Core/Core/Internal/SlimReadWriteLock.cs @@ -14,11 +14,25 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; using System.Threading; + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class SlimReadWriteLock : Lock { - private readonly ReaderWriterLockSlim locker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); + private readonly ReaderWriterLockSlim locker; + + public SlimReadWriteLock() + { + this.locker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); + } + + internal SlimReadWriteLock(ReaderWriterLockSlim underlyingLock) + { + this.locker = underlyingLock; + } public override IUpgradeableLockHolder ForReadingUpgradeable() { diff --git a/src/Castle.Core/Core/Internal/SlimUpgradeableReadLockHolder.cs b/src/Castle.Core/Core/Internal/SlimUpgradeableReadLockHolder.cs index 67ced743f3..593c95e690 100644 --- a/src/Castle.Core/Core/Internal/SlimUpgradeableReadLockHolder.cs +++ b/src/Castle.Core/Core/Internal/SlimUpgradeableReadLockHolder.cs @@ -14,8 +14,12 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; using System.Threading; + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class SlimUpgradeableReadLockHolder : IUpgradeableLockHolder { private readonly ReaderWriterLockSlim locker; diff --git a/src/Castle.Core/Core/Internal/SlimWriteLockHolder.cs b/src/Castle.Core/Core/Internal/SlimWriteLockHolder.cs index ae86ffe957..c46018fed1 100644 --- a/src/Castle.Core/Core/Internal/SlimWriteLockHolder.cs +++ b/src/Castle.Core/Core/Internal/SlimWriteLockHolder.cs @@ -14,8 +14,12 @@ namespace Castle.Core.Internal { + using System; + using System.ComponentModel; using System.Threading; + [Obsolete("Consider using `System.Threading.ReaderWriterLockSlim` instead of `Lock` and related types.")] // TODO: Remove this type. + [EditorBrowsable(EditorBrowsableState.Never)] internal class SlimWriteLockHolder : ILockHolder { private readonly ReaderWriterLockSlim locker; diff --git a/src/Castle.Core/Core/Internal/SynchronizedDictionary.cs b/src/Castle.Core/Core/Internal/SynchronizedDictionary.cs new file mode 100644 index 0000000000..59ab376cec --- /dev/null +++ b/src/Castle.Core/Core/Internal/SynchronizedDictionary.cs @@ -0,0 +1,128 @@ +// Copyright 2004-2018 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Castle.Core.Internal +{ + using System; + using System.Collections.Generic; + using System.Threading; + + internal sealed class SynchronizedDictionary : IDisposable + { + private Dictionary items; + private ReaderWriterLockSlim itemsLock; + + public SynchronizedDictionary() + { + items = new Dictionary(); + itemsLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); + } + + [Obsolete] // TODO: Remove this property along with the `ModuleScope.Lock` property. + public ReaderWriterLockSlim Lock => itemsLock; + + public void AddOrUpdateWithoutTakingLock(TKey key, TValue value) + { + items[key] = value; + } + + public void Dispose() + { + itemsLock.Dispose(); + } + + public TValue GetOrAdd(TKey key, Func valueFactory) + { + TValue value; + + itemsLock.EnterReadLock(); + try + { + if (items.TryGetValue(key, out value)) + { + return value; + } + } + finally + { + itemsLock.ExitReadLock(); + } + + itemsLock.EnterUpgradeableReadLock(); + try + { + if (items.TryGetValue(key, out value)) + { + return value; + } + else + { + itemsLock.EnterWriteLock(); + try + { + value = valueFactory.Invoke(key); + items.Add(key, value); + return value; + } + finally + { + itemsLock.ExitWriteLock(); + } + } + } + finally + { + itemsLock.ExitUpgradeableReadLock(); + } + } + + public TValue GetOrAddWithoutTakingLock(TKey key, Func valueFactory) + { + TValue value; + + if (items.TryGetValue(key, out value)) + { + return value; + } + else + { + value = valueFactory.Invoke(key); + items.Add(key, value); + return value; + } + } + + public void ForEach(Action action) + { + itemsLock.EnterReadLock(); + try + { + foreach (var item in items) + { + action.Invoke(item.Key, item.Value); + } + } + finally + { + itemsLock.ExitReadLock(); + } + } + + [Obsolete] // TODO: Remove this method along with the `ModuleScope.GetFromCache` method. + public bool TryGetValueWithoutTakingLock(TKey key, out TValue value) + { + return items.TryGetValue(key, out value); + } + } +} diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs index bcb0e75382..cd20725586 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs @@ -179,19 +179,10 @@ private Type GetDelegateType(MetaMethod method, ClassEmitter @class, ProxyGenera ToArray(), null); - var type = scope.GetFromCache(key); - if (type != null) - { - return type; - } - - type = new DelegateTypeGenerator(method, targetType) + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new DelegateTypeGenerator(method, targetType) .Generate(@class, options, namingScope) - .BuildType(); - - scope.RegisterInCache(key, type); - - return type; + .BuildType()); } private Type GetInvocationType(MetaMethod method, ClassEmitter @class, ProxyGenerationOptions options) diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs index df0dd08ef9..47d7819d92 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs @@ -126,19 +126,10 @@ private Type GetDelegateType(MetaMethod method, ClassEmitter @class, ProxyGenera ToArray(), null); - var type = scope.GetFromCache(key); - if (type != null) - { - return type; - } - - type = new DelegateTypeGenerator(method, targetType) + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new DelegateTypeGenerator(method, targetType) .Generate(@class, options, namingScope) - .BuildType(); - - scope.RegisterInCache(key, type); - - return type; + .BuildType()); } private Type GetInvocationType(MetaMethod method, ClassEmitter @class, ProxyGenerationOptions options) @@ -150,16 +141,7 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter @class, ProxyGene // no locking required as we're already within a lock - var invocation = scope.GetFromCache(key); - if (invocation != null) - { - return invocation; - } - invocation = BuildInvocationType(method, @class, options); - - scope.RegisterInCache(key, invocation); - - return invocation; + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => BuildInvocationType(method, @class, options)); } private MethodGenerator IndirectlyCalledMethodGenerator(MetaMethod method, ClassEmitter proxy, diff --git a/src/Castle.Core/DynamicProxy/Contributors/DelegateProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/DelegateProxyTargetContributor.cs index b6db660c5d..a14e8b206e 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/DelegateProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/DelegateProxyTargetContributor.cs @@ -57,23 +57,15 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter, ProxyGen var key = new CacheKey(method.Method, CompositionInvocationTypeGenerator.BaseType, null, null); // no locking required as we're already within a lock - var invocation = scope.GetFromCache(key); - if (invocation != null) - { - return invocation; - } - invocation = new CompositionInvocationTypeGenerator(method.Method.DeclaringType, - method, - method.Method, - false, - null) + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new CompositionInvocationTypeGenerator(method.Method.DeclaringType, + method, + method.Method, + false, + null) .Generate(emitter, options, namingScope) - .BuildType(); - - scope.RegisterInCache(key, invocation); - - return invocation; + .BuildType()); } } } \ No newline at end of file diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyTargetContributor.cs index b81be4bb47..4c333be11c 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyTargetContributor.cs @@ -92,22 +92,14 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter @class, ProxyGene // no locking required as we're already within a lock - var invocation = scope.GetFromCache(key); - if (invocation != null) - { - return invocation; - } - - invocation = new CompositionInvocationTypeGenerator(method.Method.DeclaringType, - method, - method.Method, - canChangeTarget, - null) - .Generate(@class, options, namingScope).BuildType(); - - scope.RegisterInCache(key, invocation); - - return invocation; + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new CompositionInvocationTypeGenerator(method.Method.DeclaringType, + method, + method.Method, + canChangeTarget, + null) + .Generate(@class, options, namingScope) + .BuildType()); } } } \ No newline at end of file diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs index c537169df9..317eda5821 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs @@ -77,23 +77,14 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter, ProxyGen // no locking required as we're already within a lock - var invocation = scope.GetFromCache(key); - if (invocation != null) - { - return invocation; - } - - invocation = new CompositionInvocationTypeGenerator(method.Method.DeclaringType, - method, - method.Method, - canChangeTarget, - null) + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new CompositionInvocationTypeGenerator(method.Method.DeclaringType, + method, + method.Method, + canChangeTarget, + null) .Generate(emitter, options, namingScope) - .BuildType(); - - scope.RegisterInCache(key, invocation); - - return invocation; + .BuildType()); } } } \ No newline at end of file diff --git a/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs index cbd7d39a05..8a6c49a463 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs @@ -132,23 +132,14 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter, ProxyGen // no locking required as we're already within a lock - var invocation = scope.GetFromCache(key); - if (invocation != null) - { - return invocation; - } - - invocation = new CompositionInvocationTypeGenerator(method.Method.DeclaringType, - method, - method.Method, - canChangeTarget, - null) + return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => + new CompositionInvocationTypeGenerator(method.Method.DeclaringType, + method, + method.Method, + canChangeTarget, + null) .Generate(emitter, options, namingScope) - .BuildType(); - - scope.RegisterInCache(key, invocation); - - return invocation; + .BuildType()); } } } \ No newline at end of file diff --git a/src/Castle.Core/DynamicProxy/Generators/BaseProxyGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/BaseProxyGenerator.cs index 1e76e725b9..22fdaa0807 100644 --- a/src/Castle.Core/DynamicProxy/Generators/BaseProxyGenerator.cs +++ b/src/Castle.Core/DynamicProxy/Generators/BaseProxyGenerator.cs @@ -16,6 +16,7 @@ namespace Castle.DynamicProxy.Generators { using System; using System.Collections.Generic; + using System.ComponentModel; using System.Diagnostics; using System.Linq; using System.Reflection; @@ -111,6 +112,8 @@ protected void AddMappingNoCheck(Type @interface, ITypeContributor implementer, mapping.Add(@interface, implementer); } + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Remove this method. + [EditorBrowsable(EditorBrowsableState.Never)] protected void AddToCache(CacheKey key, Type type) { scope.RegisterInCache(key, type); @@ -335,6 +338,8 @@ protected ConstructorEmitter GenerateStaticConstructor(ClassEmitter emitter) return emitter.CreateTypeConstructor(); } + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Remove this method. + [EditorBrowsable(EditorBrowsableState.Never)] protected Type GetFromCache(CacheKey key) { return scope.GetFromCache(key); @@ -382,42 +387,29 @@ protected void InitializeStaticFields(Type builtType) builtType.SetStaticField("proxyGenerationOptions", BindingFlags.NonPublic, ProxyGenerationOptions); } + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Redeclare this method as `private protected`. + [EditorBrowsable(EditorBrowsableState.Never)] protected Type ObtainProxyType(CacheKey cacheKey, Func factory) { - Type cacheType; - using (var locker = Scope.Lock.ForReading()) - { - cacheType = GetFromCache(cacheKey); - if (cacheType != null) - { - Logger.DebugFormat("Found cached proxy type {0} for target type {1}.", cacheType.FullName, targetType.FullName); - return cacheType; - } - } + bool notFoundInTypeCache = false; - // This is to avoid generating duplicate types under heavy multithreaded load. - using (var locker = Scope.Lock.ForWriting()) + var proxyType = Scope.TypeCache.GetOrAdd(cacheKey, _ => { - // Only one thread at a time may enter a write lock. - // See if an earlier lock holder populated the cache. - cacheType = GetFromCache(cacheKey); - if (cacheType != null) - { - Logger.DebugFormat("Found cached proxy type {0} for target type {1}.", cacheType.FullName, targetType.FullName); - return cacheType; - } - - // Log details about the cache miss + notFoundInTypeCache = true; Logger.DebugFormat("No cached proxy type was found for target type {0}.", targetType.FullName); + EnsureOptionsOverrideEqualsAndGetHashCode(ProxyGenerationOptions); var name = Scope.NamingScope.GetUniqueName("Castle.Proxies." + targetType.Name + "Proxy"); - var proxyType = factory.Invoke(name, Scope.NamingScope.SafeSubScope()); - - AddToCache(cacheKey, proxyType); + return factory.Invoke(name, Scope.NamingScope.SafeSubScope()); + }); - return proxyType; + if (!notFoundInTypeCache) + { + Logger.DebugFormat("Found cached proxy type {0} for target type {1}.", proxyType.FullName, targetType.FullName); } + + return proxyType; } private bool IsConstructorVisible(ConstructorInfo constructor) diff --git a/src/Castle.Core/DynamicProxy/Generators/CacheKey.cs b/src/Castle.Core/DynamicProxy/Generators/CacheKey.cs index 43c3950471..e91241f015 100644 --- a/src/Castle.Core/DynamicProxy/Generators/CacheKey.cs +++ b/src/Castle.Core/DynamicProxy/Generators/CacheKey.cs @@ -15,8 +15,11 @@ namespace Castle.DynamicProxy.Generators { using System; + using System.ComponentModel; using System.Reflection; + [Obsolete("Intended for internal use only.")] // TODO: Redeclare this type as `internal`. + [EditorBrowsable(EditorBrowsableState.Never)] #if FEATURE_SERIALIZATION [Serializable] #endif diff --git a/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs b/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs index b8d846d1bd..dc22c7e547 100644 --- a/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs +++ b/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs @@ -18,16 +18,15 @@ namespace Castle.DynamicProxy.Internal using System.Collections.Generic; using System.Diagnostics; using System.Reflection; + using System.Threading; using Castle.Core.Internal; using Castle.DynamicProxy.Generators; public static class InvocationHelper { - private static readonly Dictionary cache = - new Dictionary(); - - private static readonly Lock @lock = Lock.Create(); + private static readonly SynchronizedDictionary cache = + new SynchronizedDictionary(); public static MethodInfo GetMethodOnObject(object target, MethodInfo proxiedMethod) { @@ -48,39 +47,10 @@ public static MethodInfo GetMethodOnType(Type type, MethodInfo proxiedMethod) Debug.Assert(proxiedMethod.DeclaringType.IsAssignableFrom(type), "proxiedMethod.DeclaringType.IsAssignableFrom(type)"); - using (var locker = @lock.ForReading()) - { - var methodOnTarget = GetFromCache(proxiedMethod, type); - if (methodOnTarget != null) - { - return methodOnTarget; - } - } - - using (var locker = @lock.ForReadingUpgradeable()) - { - var methodOnTarget = GetFromCache(proxiedMethod, type); - if (methodOnTarget != null) - { - return methodOnTarget; - } - // Upgrade the lock to a write lock. - using (locker.Upgrade()) - { - methodOnTarget = ObtainMethod(proxiedMethod, type); - PutToCache(proxiedMethod, type, methodOnTarget); - } - return methodOnTarget; - } - } + var cacheKey = new CacheKey(proxiedMethod, type); - private static MethodInfo GetFromCache(MethodInfo methodInfo, Type type) - { - var key = new CacheKey(methodInfo, type); - MethodInfo method; - cache.TryGetValue(key, out method); - return method; + return cache.GetOrAdd(cacheKey, ck => ObtainMethod(proxiedMethod, type)); } private static MethodInfo ObtainMethod(MethodInfo proxiedMethod, Type type) @@ -127,12 +97,6 @@ private static MethodInfo ObtainMethod(MethodInfo proxiedMethod, Type type) return methodOnTarget.MakeGenericMethod(genericArguments); } - private static void PutToCache(MethodInfo methodInfo, Type type, MethodInfo value) - { - var key = new CacheKey(methodInfo, type); - cache.Add(key, value); - } - private struct CacheKey : IEquatable { public CacheKey(MethodInfo method, Type type) diff --git a/src/Castle.Core/DynamicProxy/ModuleScope.cs b/src/Castle.Core/DynamicProxy/ModuleScope.cs index 2dab5be652..8cadbb829c 100644 --- a/src/Castle.Core/DynamicProxy/ModuleScope.cs +++ b/src/Castle.Core/DynamicProxy/ModuleScope.cs @@ -16,10 +16,13 @@ namespace Castle.DynamicProxy { using System; using System.Collections.Generic; + using System.ComponentModel; + using System.Diagnostics; using System.IO; using System.Reflection; using System.Reflection.Emit; using System.Resources; + using System.Threading; using Castle.Core.Internal; using Castle.DynamicProxy.Generators; @@ -52,10 +55,11 @@ public class ModuleScope private readonly string weakModulePath; // Keeps track of generated types - private readonly Dictionary typeCache = new Dictionary(); + private readonly SynchronizedDictionary typeCache = new SynchronizedDictionary(); // Users of ModuleScope should use this lock when accessing the cache - private readonly Lock cacheLock = Lock.Create(); + [Obsolete] // TODO: Remove this field together with the `Lock` property. + private readonly Lock cacheLock; // Used to lock the module builder creation private readonly object moduleLocker = new object(); @@ -142,6 +146,8 @@ public ModuleScope(bool savePhysicalAssembly, bool disableSignedModule, INamingS this.strongModulePath = strongModulePath; this.weakAssemblyName = weakAssemblyName; this.weakModulePath = weakModulePath; + + this.cacheLock = Lock.CreateFor(typeCache.Lock); } public INamingScope NamingScope @@ -152,20 +158,26 @@ public INamingScope NamingScope /// /// Users of this should use this lock when accessing the cache. /// + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Remove this property. + [EditorBrowsable(EditorBrowsableState.Never)] public Lock Lock { get { return cacheLock; } } + internal SynchronizedDictionary TypeCache => typeCache; + /// /// Returns a type from this scope's type cache, or null if the key cannot be found. /// /// The key to be looked up in the cache. /// The type from this scope's type cache matching the key, or null if the key cannot be found + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Remove this method. + [EditorBrowsable(EditorBrowsableState.Never)] public Type GetFromCache(CacheKey key) { Type type; - typeCache.TryGetValue(key, out type); + typeCache.TryGetValueWithoutTakingLock(key, out type); return type; } @@ -174,9 +186,11 @@ public Type GetFromCache(CacheKey key) /// /// The key to be associated with the type. /// The type to be stored in the cache. + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Remove this method. + [EditorBrowsable(EditorBrowsableState.Never)] public void RegisterInCache(CacheKey key, Type type) { - typeCache[key] = type; + typeCache.AddOrUpdateWithoutTakingLock(key, type); } /// @@ -512,21 +526,17 @@ public string SaveAssembly(bool strongNamed) #if FEATURE_SERIALIZATION private void AddCacheMappings(AssemblyBuilder builder) { - Dictionary mappings; + var mappings = new Dictionary(); - using (Lock.ForReading()) + typeCache.ForEach((key, value) => { - mappings = new Dictionary(); - foreach (var cacheEntry in typeCache) + // NOTE: using == returns invalid results. + // we need to use Equals here for it to work properly + if (builder.Equals(value.Assembly)) { - // NOTE: using == returns invalid results. - // we need to use Equals here for it to work properly - if(builder.Equals(cacheEntry.Value.Assembly)) - { - mappings.Add(cacheEntry.Key, cacheEntry.Value.FullName); - } + mappings.Add(key, value.FullName); } - } + }); CacheMappingsAttribute.ApplyTo(builder, mappings); } @@ -565,7 +575,7 @@ public void LoadAssemblyIntoCache(Assembly assembly) if (loadedType != null) { - RegisterInCache(mapping.Key, loadedType); + typeCache.AddOrUpdateWithoutTakingLock(mapping.Key, loadedType); } } } diff --git a/src/Castle.Core/DynamicProxy/ProxyUtil.cs b/src/Castle.Core/DynamicProxy/ProxyUtil.cs index d849d9c092..efb86044d3 100644 --- a/src/Castle.Core/DynamicProxy/ProxyUtil.cs +++ b/src/Castle.Core/DynamicProxy/ProxyUtil.cs @@ -19,6 +19,7 @@ namespace Castle.DynamicProxy using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; + using System.Threading; #if FEATURE_REMOTING using System.Runtime.Remoting; @@ -28,8 +29,7 @@ namespace Castle.DynamicProxy public static class ProxyUtil { - private static readonly IDictionary internalsVisibleToDynamicProxy = new Dictionary(); - private static readonly Lock internalsVisibleToDynamicProxyLock = Lock.Create(); + private static readonly SynchronizedDictionary internalsVisibleToDynamicProxy = new SynchronizedDictionary(); public static object GetUnproxiedInstance(object instance) { @@ -131,30 +131,11 @@ public static bool IsAccessible(Type type) /// The assembly to inspect. internal static bool AreInternalsVisibleToDynamicProxy(Assembly asm) { - using (var locker = internalsVisibleToDynamicProxyLock.ForReading()) + return internalsVisibleToDynamicProxy.GetOrAdd(asm, a => { - if (internalsVisibleToDynamicProxy.ContainsKey(asm)) - { - return internalsVisibleToDynamicProxy[asm]; - } - } - - using (var locker = internalsVisibleToDynamicProxyLock.ForReadingUpgradeable()) - { - if (internalsVisibleToDynamicProxy.ContainsKey(asm)) - { - return internalsVisibleToDynamicProxy[asm]; - } - - // Upgrade the lock to a write lock. - using (locker.Upgrade()) - { - var internalsVisibleTo = asm.GetCustomAttributes(); - var found = internalsVisibleTo.Any(attr => attr.AssemblyName.Contains(ModuleScope.DEFAULT_ASSEMBLY_NAME)); - internalsVisibleToDynamicProxy.Add(asm, found); - return found; - } - } + var internalsVisibleTo = asm.GetCustomAttributes(); + return internalsVisibleTo.Any(attr => attr.AssemblyName.Contains(ModuleScope.DEFAULT_ASSEMBLY_NAME)); + }); } internal static bool IsAccessibleType(Type target) diff --git a/src/Castle.Core/DynamicProxy/Serialization/CacheMappingsAttribute.cs b/src/Castle.Core/DynamicProxy/Serialization/CacheMappingsAttribute.cs index 183fab6b28..1626ac35a7 100644 --- a/src/Castle.Core/DynamicProxy/Serialization/CacheMappingsAttribute.cs +++ b/src/Castle.Core/DynamicProxy/Serialization/CacheMappingsAttribute.cs @@ -18,6 +18,7 @@ namespace Castle.DynamicProxy.Serialization { using System; using System.Collections.Generic; + using System.ComponentModel; using System.IO; using System.Reflection; using System.Reflection.Emit; @@ -47,6 +48,8 @@ public byte[] SerializedCacheMappings get { return serializedCacheMappings; } } + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Redeclare this method as `internal`. + [EditorBrowsable(EditorBrowsableState.Never)] public Dictionary GetDeserializedMappings() { using (var stream = new MemoryStream(SerializedCacheMappings)) @@ -56,6 +59,8 @@ public Dictionary GetDeserializedMappings() } } + [Obsolete("Exposes a component that is intended for internal use only.")] // TODO: Redeclare this method as `internal`. + [EditorBrowsable(EditorBrowsableState.Never)] public static void ApplyTo(AssemblyBuilder assemblyBuilder, Dictionary mappings) { using (var stream = new MemoryStream())