Deprecate Lock and ModuleScope's public type cache API

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:

buildscripts/common.props

@@ -2,6 +2,7 @@
 
 	<PropertyGroup>
 		<NoWarn>$(NoWarn);CS1591;CS3014;CS3003;CS3001;CS3021</NoWarn>
+		<NoWarn>$(NoWarn);CS0612;CS0618</NoWarn> <!-- TODO: Remove this line once `[Obsolete]` members have been dealt with. -->
 		<RepositoryType>git</RepositoryType>
 		<RepositoryUrl>https://github.com/castleproject/Core</RepositoryUrl>
 		<BuildVersion>0.0.0</BuildVersion>

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;

src/Castle.Core/Components.DictionaryAdapter/DictionaryAdapterFactory.cs

@@ -36,9 +36,8 @@ namespace Castle.Components.DictionaryAdapter
 	/// </summary>
 	public class DictionaryAdapterFactory : IDictionaryAdapterFactory
 	{
-		private readonly Dictionary<Type, DictionaryAdapterMeta> interfaceToMeta = new Dictionary<Type, DictionaryAdapterMeta>();
-
-		private readonly Lock interfaceToMetaLock = Lock.Create();
+		private readonly SynchronizedDictionary<Type, DictionaryAdapterMeta> interfaceToMeta =
+			new SynchronizedDictionary<Type, DictionaryAdapterMeta>();
 
 		#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)

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<TKey, TItem>
 		where TItem : class
 	{
-		private readonly Lock locker;
+		private readonly ReaderWriterLockSlim locker;
 		private readonly Dictionary<TKey, object> items;
 		private readonly Func<TKey, TItem> factory;
 
@@ -32,7 +31,7 @@ public SingletonDispenser(Func<TKey, TItem> factory)
 			if (factory == null)
 				throw Error.ArgumentNull("factory");
 
-			this.locker  = new SlimReadWriteLock();
+			this.locker  = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);
 			this.items   = new Dictionary<TKey, object>();
 			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;

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; }

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();

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);
+		}
 	}
 }

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();

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();

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;

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()
 		{

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;

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;