yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor some locks, get rid of warnings

Browse Source
This commit is contained in:
Stephan
2018-05-03 14:50:21 +02:00
parent 41144bb7f7
commit d04a573888
11 changed files with 260 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs
View File

@@ -24,8 +24,13 @@ namespace Umbraco.Core.Cache
protected abstract object GetEntry(string key);
// read-write lock the underlying cache
protected abstract IDisposable ReadLock { get; }
protected abstract IDisposable WriteLock { get; }
//protected abstract IDisposable ReadLock { get; }
//protected abstract IDisposable WriteLock { get; }
protected abstract void EnterReadLock();
protected abstract void ExitReadLock();
protected abstract void EnterWriteLock();
protected abstract void ExitWriteLock();
protected string GetCacheKey(string key)
{
@@ -88,21 +93,31 @@ namespace Umbraco.Core.Cache
public virtual void ClearAllCache()
{
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.ToArray())
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheItem(string key)
{
var cacheKey = GetCacheKey(key);
using (WriteLock)
try
{
EnterWriteLock();
RemoveEntry(cacheKey);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes(string typeName)
@@ -110,15 +125,16 @@ namespace Umbraco.Core.Cache
var type = TypeFinder.GetTypeByName(typeName);
if (type == null) return;
var isInterface = type.IsInterface;
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.Where(x =>
{
// entry.Value is Lazy<object> and not null, its value may be null
// remove null values as well, does not hurt
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
var value = GetSafeLazyValue((Lazy<object>) x.Value, true);
// if T is an interface remove anything that implements that interface
// otherwise remove exact types (not inherited types)
@@ -127,14 +143,19 @@ namespace Umbraco.Core.Cache
.ToArray())
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes<T>()
{
var typeOfT = typeof(T);
var isInterface = typeOfT.IsInterface;
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.Where(x =>
{
@@ -142,7 +163,7 @@ namespace Umbraco.Core.Cache
// remove null values as well, does not hurt
// compare on exact type, don't use "is"
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
var value = GetSafeLazyValue((Lazy<object>) x.Value, true);
// if T is an interface remove anything that implements that interface
// otherwise remove exact types (not inherited types)
@@ -151,6 +172,10 @@ namespace Umbraco.Core.Cache
.ToArray())
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes<T>(Func<string, T, bool> predicate)
@@ -158,8 +183,9 @@ namespace Umbraco.Core.Cache
var typeOfT = typeof(T);
var isInterface = typeOfT.IsInterface;
var plen = CacheItemPrefix.Length + 1;
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.Where(x =>
{
@@ -167,7 +193,7 @@ namespace Umbraco.Core.Cache
// remove null values as well, does not hurt
// compare on exact type, don't use "is"
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
var value = GetSafeLazyValue((Lazy<object>) x.Value, true);
if (value == null) return true;
// if T is an interface remove anything that implements that interface
@@ -178,30 +204,44 @@ namespace Umbraco.Core.Cache
}))
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheByKeySearch(string keyStartsWith)
{
var plen = CacheItemPrefix.Length + 1;
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith))
.ToArray())
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
public virtual void ClearCacheByKeyExpression(string regexString)
{
var plen = CacheItemPrefix.Length + 1;
using (WriteLock)
try
{
EnterWriteLock();
foreach (var entry in GetDictionaryEntries()
.Where(x => Regex.IsMatch(((string)x.Key).Substring(plen), regexString))
.ToArray())
RemoveEntry((string) entry.Key);
}
finally
{
ExitWriteLock();
}
}
#endregion
@@ -212,12 +252,18 @@ namespace Umbraco.Core.Cache
{
var plen = CacheItemPrefix.Length + 1;
IEnumerable<DictionaryEntry> entries;
using (ReadLock)
try
{
EnterReadLock();
entries = GetDictionaryEntries()
.Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith))
.ToArray(); // evaluate while locked
}
finally
{
ExitReadLock();
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null); // backward compat, don't store null values in the cache
@@ -228,12 +274,17 @@ namespace Umbraco.Core.Cache
const string prefix = CacheItemPrefix + "-";
var plen = prefix.Length;
IEnumerable<DictionaryEntry> entries;
using (ReadLock)
try
{
EnterReadLock();
entries = GetDictionaryEntries()
.Where(x => Regex.IsMatch(((string)x.Key).Substring(plen), regexString))
.ToArray(); // evaluate while locked
}
finally
{
ExitReadLock();
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null); // backward compat, don't store null values in the cache
@@ -243,10 +294,15 @@ namespace Umbraco.Core.Cache
{
cacheKey = GetCacheKey(cacheKey);
Lazy<object> result;
using (ReadLock)
try
{
EnterReadLock();
result = GetEntry(cacheKey) as Lazy<object>; // null if key not found
}
finally
{
ExitReadLock();
}
return result == null ? null : GetSafeLazyValue(result); // return exceptions as null
}

35
src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs
View File

@@ -79,24 +79,26 @@ namespace Umbraco.Core.Cache
#region Lock
protected override IDisposable ReadLock
private bool _entered;
protected override void EnterReadLock() => EnterWriteLock();
protected override void EnterWriteLock()
{
// there's no difference between ReadLock and WriteLock here
get { return WriteLock; }
if (HasContextItems)
{
System.Threading.Monitor.Enter(ContextItems.SyncRoot, ref _entered);
}
}
protected override IDisposable WriteLock
{
// NOTE
// could think about just overriding base.Locker to return a different
// object but then we'd create a ReaderWriterLockSlim per request,
// which is less efficient than just using a basic monitor lock.
protected override void ExitReadLock() => ExitWriteLock();
get
protected override void ExitWriteLock()
{
if (_entered)
{
return HasContextItems
? (IDisposable) new MonitorLock(ContextItems.SyncRoot)
: new NoopLocker();
_entered = false;
System.Threading.Monitor.Exit(ContextItems.SyncRoot);
}
}
@@ -113,8 +115,9 @@ namespace Umbraco.Core.Cache
Lazy<object> result;
using (WriteLock)
try
{
EnterWriteLock();
result = ContextItems[cacheKey] as Lazy<object>; // null if key not found
// cannot create value within the lock, so if result.IsValueCreated is false, just
@@ -127,6 +130,10 @@ namespace Umbraco.Core.Cache
ContextItems[cacheKey] = result;
}
}
finally
{
ExitWriteLock();
}
// using GetSafeLazy and GetSafeLazyValue ensures that we don't cache
// exceptions (but try again and again) and silently eat them - however at

36
src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs
View File

@@ -50,14 +50,26 @@ namespace Umbraco.Core.Cache
#region Lock
protected override IDisposable ReadLock
protected override void EnterReadLock()
{
get { return new ReadLock(_locker); }
_locker.EnterReadLock();
}
protected override IDisposable WriteLock
protected override void EnterWriteLock()
{
get { return new WriteLock(_locker); }
_locker.EnterWriteLock();;
}
protected override void ExitReadLock()
{
if (_locker.IsReadLockHeld)
_locker.ExitReadLock();
}
protected override void ExitWriteLock()
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
#endregion
@@ -118,10 +130,16 @@ namespace Umbraco.Core.Cache
// reads. We first try with a normal ReadLock for maximum concurrency and take the penalty of
// having to re-lock in case there's no value. Would need to benchmark to figure out whether
// it's worth it, though...
using (new ReadLock(_locker))
try
{
_locker.EnterReadLock();
result = _cache.Get(cacheKey) as Lazy<object>; // null if key not found
}
finally
{
if (_locker.IsReadLockHeld)
_locker.ExitReadLock();
}
var value = result == null ? null : GetSafeLazyValue(result);
if (value != null) return value;
@@ -195,11 +213,17 @@ namespace Umbraco.Core.Cache
var absolute = isSliding ? System.Web.Caching.Cache.NoAbsoluteExpiration : (timeout == null ? System.Web.Caching.Cache.NoAbsoluteExpiration : DateTime.Now.Add(timeout.Value));
var sliding = isSliding == false ? System.Web.Caching.Cache.NoSlidingExpiration : (timeout ?? System.Web.Caching.Cache.NoSlidingExpiration);
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
//NOTE: 'Insert' on System.Web.Caching.Cache actually does an add or update!
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public void InsertCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout = null, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)

80
src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs
View File

@@ -37,20 +37,32 @@ namespace Umbraco.Core.Cache
public virtual void ClearAllCache()
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
MemoryCache.DisposeIfDisposable();
MemoryCache = new MemoryCache("in-memory");
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheItem(string key)
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
if (MemoryCache[key] == null) return;
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes(string typeName)
@@ -58,8 +70,9 @@ namespace Umbraco.Core.Cache
var type = TypeFinder.GetTypeByName(typeName);
if (type == null) return;
var isInterface = type.IsInterface;
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
foreach (var key in MemoryCache
.Where(x =>
{
@@ -76,12 +89,18 @@ namespace Umbraco.Core.Cache
.ToArray()) // ToArray required to remove
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes<T>()
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
var typeOfT = typeof (T);
var isInterface = typeOfT.IsInterface;
foreach (var key in MemoryCache
@@ -101,12 +120,18 @@ namespace Umbraco.Core.Cache
.ToArray()) // ToArray required to remove
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheObjectTypes<T>(Func<string, T, bool> predicate)
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
var typeOfT = typeof(T);
var isInterface = typeOfT.IsInterface;
foreach (var key in MemoryCache
@@ -127,30 +152,47 @@ namespace Umbraco.Core.Cache
.ToArray()) // ToArray required to remove
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheByKeySearch(string keyStartsWith)
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
foreach (var key in MemoryCache
.Where(x => x.Key.InvariantStartsWith(keyStartsWith))
.Select(x => x.Key)
.ToArray()) // ToArray required to remove
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
public virtual void ClearCacheByKeyExpression(string regexString)
{
using (new WriteLock(_locker))
try
{
_locker.EnterWriteLock();
foreach (var key in MemoryCache
.Where(x => Regex.IsMatch(x.Key, regexString))
.Select(x => x.Key)
.ToArray()) // ToArray required to remove
MemoryCache.Remove(key);
}
finally
{
if (_locker.IsWriteLockHeld)
_locker.ExitWriteLock();
}
}
#endregion
@@ -160,12 +202,18 @@ namespace Umbraco.Core.Cache
public IEnumerable<object> GetCacheItemsByKeySearch(string keyStartsWith)
{
KeyValuePair<string, object>[] entries;
using (new ReadLock(_locker))
try
{
_locker.EnterReadLock();
entries = MemoryCache
.Where(x => x.Key.InvariantStartsWith(keyStartsWith))
.ToArray(); // evaluate while locked
}
finally
{
if (_locker.IsReadLockHeld)
_locker.ExitReadLock();
}
return entries
.Select(x => DictionaryCacheProviderBase.GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null) // backward compat, don't store null values in the cache
@@ -175,12 +223,18 @@ namespace Umbraco.Core.Cache
public IEnumerable<object> GetCacheItemsByKeyExpression(string regexString)
{
KeyValuePair<string, object>[] entries;
using (new ReadLock(_locker))
try
{
_locker.EnterReadLock();
entries = MemoryCache
.Where(x => Regex.IsMatch(x.Key, regexString))
.ToArray(); // evaluate while locked
}
finally
{
if (_locker.IsReadLockHeld)
_locker.ExitReadLock();
}
return entries
.Select(x => DictionaryCacheProviderBase.GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null) // backward compat, don't store null values in the cache
@@ -190,10 +244,16 @@ namespace Umbraco.Core.Cache
public object GetCacheItem(string cacheKey)
{
Lazy<object> result;
using (new ReadLock(_locker))
try
{
_locker.EnterReadLock();
result = MemoryCache.Get(cacheKey) as Lazy<object>; // null if key not found
}
finally
{
if (_locker.IsReadLockHeld)
_locker.ExitReadLock();
}
return result == null ? null : DictionaryCacheProviderBase.GetSafeLazyValue(result); // return exceptions as null
}

41
src/Umbraco.Core/Collections/ConcurrentHashSet.cs
View File

@@ -50,10 +50,16 @@ namespace Umbraco.Core.Collections
/// <param name="item">The object to remove from the <see cref="T:System.Collections.Generic.ICollection`1"/>.</param><exception cref="T:System.NotSupportedException">The <see cref="T:System.Collections.Generic.ICollection`1"/> is read-only.</exception>
public bool Remove(T item)
{
using (new WriteLock(_instanceLocker))
try
{
_instanceLocker.EnterWriteLock();
return _innerSet.Remove(item);
}
finally
{
if (_instanceLocker.IsWriteLockHeld)
_instanceLocker.ExitWriteLock();
}
}
@@ -86,10 +92,16 @@ namespace Umbraco.Core.Collections
/// <param name="item">The object to add to the <see cref="T:System.Collections.Generic.ICollection`1"/>.</param><exception cref="T:System.NotSupportedException">The <see cref="T:System.Collections.Generic.ICollection`1"/> is read-only.</exception>
public void Add(T item)
{
using (new WriteLock(_instanceLocker))
try
{
_instanceLocker.EnterWriteLock();
_innerSet.Add(item);
}
finally
{
if (_instanceLocker.IsWriteLockHeld)
_instanceLocker.ExitWriteLock();
}
}
/// <summary>
@@ -101,13 +113,20 @@ namespace Umbraco.Core.Collections
{
var clone = GetThreadSafeClone();
if (clone.Contains(item)) return false;
using (new WriteLock(_instanceLocker))
try
{
_instanceLocker.EnterWriteLock();
//double check
if (_innerSet.Contains(item)) return false;
_innerSet.Add(item);
return true;
}
finally
{
if (_instanceLocker.IsWriteLockHeld)
_instanceLocker.ExitWriteLock();
}
}
/// <summary>
@@ -116,10 +135,16 @@ namespace Umbraco.Core.Collections
/// <exception cref="T:System.NotSupportedException">The <see cref="T:System.Collections.Generic.ICollection`1"/> is read-only. </exception>
public void Clear()
{
using (new WriteLock(_instanceLocker))
try
{
_instanceLocker.EnterWriteLock();
_innerSet.Clear();
}
finally
{
if (_instanceLocker.IsWriteLockHeld)
_instanceLocker.ExitWriteLock();
}
}
/// <summary>
@@ -147,10 +172,16 @@ namespace Umbraco.Core.Collections
private HashSet<T> GetThreadSafeClone()
{
HashSet<T> clone = null;
using (new WriteLock(_instanceLocker))
try
{
_instanceLocker.EnterWriteLock();
clone = new HashSet<T>(_innerSet, _innerSet.Comparer);
}
finally
{
if (_instanceLocker.IsWriteLockHeld)
_instanceLocker.ExitWriteLock();
}
return clone;
}

9
src/Umbraco.Core/Models/PropertyGroupCollection.cs
View File

@@ -67,8 +67,10 @@ namespace Umbraco.Core.Models
internal new void Add(PropertyGroup item)
{
using (new WriteLock(_addLocker))
try
{
_addLocker.EnterWriteLock();
//Note this is done to ensure existig groups can be renamed
if (item.HasIdentity && item.Id > 0)
{
@@ -102,6 +104,11 @@ namespace Umbraco.Core.Models
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item));
}
finally
{
if (_addLocker.IsWriteLockHeld)
_addLocker.ExitWriteLock();
}
}
/// <summary>

8
src/Umbraco.Core/Models/PropertyTypeCollection.cs
View File

@@ -80,8 +80,9 @@ namespace Umbraco.Core.Models
item.IsPublishing = IsPublishing;
// fixme redo this entirely!!!
using (new WriteLock(_addLocker))
try
{
_addLocker.EnterWriteLock();
var key = GetKeyForItem(item);
if (key != null)
{
@@ -105,6 +106,11 @@ namespace Umbraco.Core.Models
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item));
}
finally
{
if (_addLocker.IsWriteLockHeld)
_addLocker.ExitWriteLock();
}
}
/// <summary>

9
src/Umbraco.Core/Persistence/UmbracoDatabaseFactory.cs
View File

@@ -126,8 +126,10 @@ namespace Umbraco.Core.Persistence
/// <inheritdoc />
public void Configure(string connectionString, string providerName)
{
using (new WriteLock(_lock))
try
{
_lock.EnterWriteLock();
_logger.Debug<UmbracoDatabaseFactory>("Configuring.");
if (Configured) throw new InvalidOperationException("Already configured.");
@@ -173,6 +175,11 @@ namespace Umbraco.Core.Persistence
_logger.Debug<UmbracoDatabaseFactory>("Configured.");
Configured = true;
}
finally
{
if (_lock.IsWriteLockHeld)
_lock.ExitWriteLock();
}
}
/// <inheritdoc />

6
src/Umbraco.Core/ReadLock.cs
View File

@@ -10,9 +10,11 @@ namespace Umbraco.Core
/// Provides a convenience methodology for implementing locked access to resources.
/// </summary>
/// <remarks>
/// Intended as an infrastructure class.
/// <para>Intended as an infrastructure class.</para>
/// <para>This is a very unefficient way to lock as it allocates one object each time we lock,
/// so it's OK to use this class for things that happen once, where it is convenient, but not
/// for performance-critical code!</para>
/// </remarks>
[Obsolete("stop using, allocates")]
public class ReadLock : IDisposable
{
private readonly ReaderWriterLockSlim _rwLock;

2
src/Umbraco.Core/Umbraco.Core.csproj
View File

@@ -394,6 +394,7 @@
<Compile Include="PropertyEditors\ValueConverters\ImageCropperValueTypeConverter.cs" />
<Compile Include="PropertyEditors\ValueListConfiguration.cs" />
<Compile Include="PropertyEditors\VoidEditor.cs" />
<Compile Include="ReadLock.cs" />
<Compile Include="ReflectionUtilities-Unused.cs" />
<Compile Include="Runtime\CoreRuntime.cs" />
<Compile Include="Runtime\CoreRuntimeComponent.cs" />
@@ -1264,7 +1265,6 @@
<Compile Include="PropertyEditors\ValueConverters\UploadPropertyConverter.cs" />
<Compile Include="PropertyEditors\ValueConverters\YesNoValueConverter.cs" />
<Compile Include="Publishing\ScheduledPublisher.cs" />
<Compile Include="ReadLock.cs" />
<Compile Include="ReflectionUtilities.cs" />
<Compile Include="RenderingEngine.cs" />
<Compile Include="RuntimeLevel.cs" />

6
src/Umbraco.Core/WriteLock.cs
View File

@@ -10,9 +10,11 @@ namespace Umbraco.Core
/// Provides a convenience methodology for implementing locked access to resources.
/// </summary>
/// <remarks>
/// Intended as an infrastructure class.
/// <para>Intended as an infrastructure class.</para>
/// <para>This is a very unefficient way to lock as it allocates one object each time we lock,
/// so it's OK to use this class for things that happen once, where it is convenient, but not
/// for performance-critical code!</para>
/// </remarks>
[Obsolete("stop using, allocates")]
public class WriteLock : IDisposable
{
private readonly ReaderWriterLockSlim _rwLock;