diff --git a/src/Umbraco.Core/Cache/AppCaches.cs b/src/Umbraco.Core/Cache/AppCaches.cs index fbfc4c8c82..05453663f5 100644 --- a/src/Umbraco.Core/Cache/AppCaches.cs +++ b/src/Umbraco.Core/Cache/AppCaches.cs @@ -6,8 +6,10 @@ namespace Umbraco.Core.Cache /// /// Represents the application caches. /// - public class AppCaches + public class AppCaches : IDisposable { + private bool _disposedValue; + /// /// Initializes a new instance of the for use in a web application. /// @@ -82,5 +84,26 @@ namespace Umbraco.Core.Cache /// search through all keys on a global scale. /// public IsolatedCaches IsolatedCaches { get; } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + RuntimeCache.DisposeIfDisposable(); + RequestCache.DisposeIfDisposable(); + IsolatedCaches.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/Cache/AppPolicedCacheDictionary.cs b/src/Umbraco.Core/Cache/AppPolicedCacheDictionary.cs index 5c60dededa..4a6c032e4f 100644 --- a/src/Umbraco.Core/Cache/AppPolicedCacheDictionary.cs +++ b/src/Umbraco.Core/Cache/AppPolicedCacheDictionary.cs @@ -7,7 +7,7 @@ namespace Umbraco.Core.Cache /// Provides a base class for implementing a dictionary of . /// /// The type of the dictionary key. - public abstract class AppPolicedCacheDictionary + public abstract class AppPolicedCacheDictionary : IDisposable { private readonly ConcurrentDictionary _caches = new ConcurrentDictionary(); @@ -24,6 +24,7 @@ namespace Umbraco.Core.Cache /// Gets the internal cache factory, for tests only! /// internal readonly Func CacheFactory; + private bool _disposedValue; /// /// Gets or creates a cache. @@ -70,5 +71,27 @@ namespace Umbraco.Core.Cache foreach (var cache in _caches.Values) cache.Clear(); } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + foreach(var value in _caches.Values) + { + value.DisposeIfDisposable(); + } + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Cache/DeepCloneAppCache.cs b/src/Umbraco.Core/Cache/DeepCloneAppCache.cs index eff06e2aad..10a83d7423 100644 --- a/src/Umbraco.Core/Cache/DeepCloneAppCache.cs +++ b/src/Umbraco.Core/Cache/DeepCloneAppCache.cs @@ -12,8 +12,10 @@ namespace Umbraco.Core.Cache /// instance, and ensuring that all inserts and returns are deep cloned copies of the cache item, /// when the item is deep-cloneable. /// - internal class DeepCloneAppCache : IAppPolicyCache + internal class DeepCloneAppCache : IAppPolicyCache, IDisposable { + private bool _disposedValue; + /// /// Initializes a new instance of the class. /// @@ -153,5 +155,24 @@ namespace Umbraco.Core.Cache return input; } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + InnerCache.DisposeIfDisposable(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs index 5c4f76f51d..e2f6017608 100644 --- a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs +++ b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs @@ -13,9 +13,10 @@ namespace Umbraco.Core.Cache /// /// Implements on top of a . /// - public class ObjectCacheAppCache : IAppPolicyCache + public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion); + private bool _disposedValue; /// /// Initializes a new instance of the . @@ -109,19 +110,34 @@ namespace Umbraco.Core.Cache Lazy result; - using (var lck = new UpgradeableReadLock(_locker)) + try { + _locker.EnterUpgradeableReadLock(); + result = MemoryCache.Get(key) as Lazy; if (result == null || FastDictionaryAppCacheBase.GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null { result = FastDictionaryAppCacheBase.GetSafeLazy(factory); var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles); - lck.UpgradeToWriteLock(); - //NOTE: This does an add or update - MemoryCache.Set(key, result, policy); + try + { + _locker.EnterWriteLock(); + //NOTE: This does an add or update + MemoryCache.Set(key, result, policy); + } + finally + { + if (_locker.IsWriteLockHeld) + _locker.ExitWriteLock(); + } } } + finally + { + if (_locker.IsUpgradeableReadLockHeld) + _locker.ExitUpgradeableReadLock(); + } //return result.Value; @@ -360,5 +376,23 @@ namespace Umbraco.Core.Cache } return policy; } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _locker.Dispose(); + } + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/Cache/WebCachingAppCache.cs b/src/Umbraco.Core/Cache/WebCachingAppCache.cs index bec596b129..671cd2d9c4 100644 --- a/src/Umbraco.Core/Cache/WebCachingAppCache.cs +++ b/src/Umbraco.Core/Cache/WebCachingAppCache.cs @@ -11,7 +11,7 @@ namespace Umbraco.Core.Cache /// Implements on top of a . /// /// The underlying cache is expected to be HttpRuntime.Cache. - internal class WebCachingAppCache : FastDictionaryAppCacheBase, IAppPolicyCache + internal class WebCachingAppCache : FastDictionaryAppCacheBase, IAppPolicyCache, IDisposable { // locker object that supports upgradeable read locking // does not need to support recursion if we implement the cache correctly and ensure @@ -19,6 +19,7 @@ namespace Umbraco.Core.Cache private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); private readonly System.Web.Caching.Cache _cache; + private bool _disposedValue; /// /// Initializes a new instance of the class. @@ -138,8 +139,10 @@ namespace Umbraco.Core.Cache var value = result == null ? null : GetSafeLazyValue(result); if (value != null) return value; - using (var lck = new UpgradeableReadLock(_locker)) + try { + _locker.EnterUpgradeableReadLock(); + result = _cache.Get(key) as Lazy; // null if key not found // cannot create value within the lock, so if result.IsValueCreated is false, just @@ -152,15 +155,28 @@ 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); - lck.UpgradeToWriteLock(); + try + { + _locker.EnterWriteLock(); - // create a cache dependency if one is needed. - var dependency = dependentFiles != null && dependentFiles.Length > 0 ? new CacheDependency(dependentFiles) : null; + // create a cache dependency if one is needed. + var dependency = dependentFiles != null && dependentFiles.Length > 0 ? new CacheDependency(dependentFiles) : null; - //NOTE: 'Insert' on System.Web.Caching.Cache actually does an add or update! - _cache.Insert(key, result, dependency, absolute, sliding, priority, removedCallback); + //NOTE: 'Insert' on System.Web.Caching.Cache actually does an add or update! + _cache.Insert(key, result, dependency, absolute, sliding, priority, removedCallback); + } + finally + { + if (_locker.IsWriteLockHeld) + _locker.ExitWriteLock(); + } } } + finally + { + if (_locker.IsUpgradeableReadLockHeld) + _locker.ExitUpgradeableReadLock(); + } // using GetSafeLazy and GetSafeLazyValue ensures that we don't cache // exceptions (but try again and again) and silently eat them - however at @@ -204,5 +220,24 @@ namespace Umbraco.Core.Cache _locker.ExitWriteLock(); } } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _locker.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/Collections/ConcurrentHashSet.cs b/src/Umbraco.Core/Collections/ConcurrentHashSet.cs index c4dba51acd..0424c4dae1 100644 --- a/src/Umbraco.Core/Collections/ConcurrentHashSet.cs +++ b/src/Umbraco.Core/Collections/ConcurrentHashSet.cs @@ -1,8 +1,8 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; -using System.Threading; namespace Umbraco.Core.Collections { @@ -14,9 +14,10 @@ namespace Umbraco.Core.Collections [Serializable] public class ConcurrentHashSet : ICollection { - private readonly HashSet _innerSet = new HashSet(); - private readonly ReaderWriterLockSlim _instanceLocker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); - + // Internally we just use a ConcurrentDictionary with the same value for the Value (since we don't actually care about the value) + private static readonly byte _emptyValue = 0x0; + private readonly ConcurrentDictionary _innerSet = new ConcurrentDictionary(); + /// /// Returns an enumerator that iterates through the collection. /// @@ -26,7 +27,7 @@ namespace Umbraco.Core.Collections /// 1 public IEnumerator GetEnumerator() { - return GetThreadSafeClone().GetEnumerator(); + return _innerSet.Keys.GetEnumerator(); } /// @@ -50,16 +51,7 @@ namespace Umbraco.Core.Collections /// The object to remove from the .The is read-only. public bool Remove(T item) { - try - { - _instanceLocker.EnterWriteLock(); - return _innerSet.Remove(item); - } - finally - { - if (_instanceLocker.IsWriteLockHeld) - _instanceLocker.ExitWriteLock(); - } + return _innerSet.TryRemove(item, out _); } @@ -74,17 +66,7 @@ namespace Umbraco.Core.Collections { get { - try - { - _instanceLocker.EnterReadLock(); - return _innerSet.Count; - } - finally - { - if (_instanceLocker.IsReadLockHeld) - _instanceLocker.ExitReadLock(); - } - + return _innerSet.Count; } } @@ -99,19 +81,10 @@ namespace Umbraco.Core.Collections /// /// Adds an item to the . /// - /// The object to add to the .The is read-only. + /// The object to add to the . public void Add(T item) { - try - { - _instanceLocker.EnterWriteLock(); - _innerSet.Add(item); - } - finally - { - if (_instanceLocker.IsWriteLockHeld) - _instanceLocker.ExitWriteLock(); - } + _innerSet.TryAdd(item, _emptyValue); } /// @@ -121,21 +94,7 @@ namespace Umbraco.Core.Collections /// public bool TryAdd(T item) { - if (Contains(item)) return false; - try - { - _instanceLocker.EnterWriteLock(); - - //double check - if (_innerSet.Contains(item)) return false; - _innerSet.Add(item); - return true; - } - finally - { - if (_instanceLocker.IsWriteLockHeld) - _instanceLocker.ExitWriteLock(); - } + return _innerSet.TryAdd(item, _emptyValue); } /// @@ -144,16 +103,7 @@ namespace Umbraco.Core.Collections /// The is read-only. public void Clear() { - try - { - _instanceLocker.EnterWriteLock(); - _innerSet.Clear(); - } - finally - { - if (_instanceLocker.IsWriteLockHeld) - _instanceLocker.ExitWriteLock(); - } + _innerSet.Clear(); } /// @@ -165,16 +115,7 @@ namespace Umbraco.Core.Collections /// The object to locate in the . public bool Contains(T item) { - try - { - _instanceLocker.EnterReadLock(); - return _innerSet.Contains(item); - } - finally - { - if (_instanceLocker.IsReadLockHeld) - _instanceLocker.ExitReadLock(); - } + return _innerSet.ContainsKey(item); } /// @@ -183,24 +124,8 @@ namespace Umbraco.Core.Collections /// The one-dimensional that is the destination of the elements copied from the . The array must have zero-based indexing.The zero-based index in at which copying begins. is a null reference (Nothing in Visual Basic). is less than zero. is equal to or greater than the length of the -or- The number of elements in the source is greater than the available space from to the end of the destination . public void CopyTo(T[] array, int index) { - var clone = GetThreadSafeClone(); - clone.CopyTo(array, index); - } - - private HashSet GetThreadSafeClone() - { - HashSet clone = null; - try - { - _instanceLocker.EnterReadLock(); - clone = new HashSet(_innerSet, _innerSet.Comparer); - } - finally - { - if (_instanceLocker.IsReadLockHeld) - _instanceLocker.ExitReadLock(); - } - return clone; + var keys = _innerSet.Keys; + keys.CopyTo(array, index); } /// @@ -209,8 +134,8 @@ namespace Umbraco.Core.Collections /// The one-dimensional that is the destination of the elements copied from . The must have zero-based indexing. The zero-based index in at which copying begins. is null. is less than zero. is multidimensional.-or- The number of elements in the source is greater than the available space from to the end of the destination . The type of the source cannot be cast automatically to the type of the destination . 2 public void CopyTo(Array array, int index) { - var clone = GetThreadSafeClone(); - Array.Copy(clone.ToArray(), 0, array, index, clone.Count); + var keys = _innerSet.Keys; + Array.Copy(keys.ToArray(), 0, array, index, keys.Count); } } } diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 973147b3fb..e2af5835f2 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -4,7 +4,6 @@ using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Linq; using System.Runtime.Serialization; -using System.Threading; namespace Umbraco.Core.Models { @@ -17,8 +16,6 @@ namespace Umbraco.Core.Models // TODO: Change this to ObservableDictionary so we can reduce the INotifyCollectionChanged implementation details public class PropertyGroupCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable { - private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); - internal PropertyGroupCollection() { } @@ -70,47 +67,37 @@ namespace Umbraco.Core.Models internal new void Add(PropertyGroup item) { - try + //Note this is done to ensure existing groups can be renamed + if (item.HasIdentity && item.Id > 0) { - _addLocker.EnterWriteLock(); - - //Note this is done to ensure existing groups can be renamed - if (item.HasIdentity && item.Id > 0) + var exists = Contains(item.Id); + if (exists) { - var exists = Contains(item.Id); + var keyExists = Contains(item.Name); + if (keyExists) + throw new Exception($"Naming conflict: Changing the name of PropertyGroup '{item.Name}' would result in duplicates"); + + //collection events will be raised in SetItem + SetItem(IndexOfKey(item.Id), item); + return; + } + } + else + { + var key = GetKeyForItem(item); + if (key != null) + { + var exists = Contains(key); if (exists) { - var keyExists = Contains(item.Name); - if (keyExists) - throw new Exception($"Naming conflict: Changing the name of PropertyGroup '{item.Name}' would result in duplicates"); - //collection events will be raised in SetItem - SetItem(IndexOfKey(item.Id), item); + SetItem(IndexOfKey(key), item); return; } } - else - { - var key = GetKeyForItem(item); - if (key != null) - { - var exists = Contains(key); - if (exists) - { - //collection events will be raised in SetItem - SetItem(IndexOfKey(key), item); - return; - } - } - } - //collection events will be raised in InsertItem - base.Add(item); - } - finally - { - if (_addLocker.IsWriteLockHeld) - _addLocker.ExitWriteLock(); } + //collection events will be raised in InsertItem + base.Add(item); } /// diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index cc9d00fa5d..b7a735e5dc 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -4,7 +4,6 @@ using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Linq; using System.Runtime.Serialization; -using System.Threading; namespace Umbraco.Core.Models { @@ -16,10 +15,6 @@ namespace Umbraco.Core.Models // TODO: Change this to ObservableDictionary so we can reduce the INotifyCollectionChanged implementation details public class PropertyTypeCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable { - [IgnoreDataMember] - private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); - - internal PropertyTypeCollection(bool supportsPublishing) { SupportsPublishing = supportsPublishing; @@ -87,36 +82,28 @@ namespace Umbraco.Core.Models item.SupportsPublishing = SupportsPublishing; // TODO: this is not pretty and should be refactored - try - { - _addLocker.EnterWriteLock(); - var key = GetKeyForItem(item); - if (key != null) - { - var exists = Contains(key); - if (exists) - { - //collection events will be raised in SetItem - SetItem(IndexOfKey(key), item); - return; - } - } - //check if the item's sort order is already in use - if (this.Any(x => x.SortOrder == item.SortOrder)) - { - //make it the next iteration - item.SortOrder = this.Max(x => x.SortOrder) + 1; - } - - //collection events will be raised in InsertItem - base.Add(item); - } - finally + var key = GetKeyForItem(item); + if (key != null) { - if (_addLocker.IsWriteLockHeld) - _addLocker.ExitWriteLock(); + var exists = Contains(key); + if (exists) + { + //collection events will be raised in SetItem + SetItem(IndexOfKey(key), item); + return; + } } + + //check if the item's sort order is already in use + if (this.Any(x => x.SortOrder == item.SortOrder)) + { + //make it the next iteration + item.SortOrder = this.Max(x => x.SortOrder) + 1; + } + + //collection events will be raised in InsertItem + base.Add(item); } /// diff --git a/src/Umbraco.Core/ReadLock.cs b/src/Umbraco.Core/ReadLock.cs index 9d3ef22168..f830d08bfa 100644 --- a/src/Umbraco.Core/ReadLock.cs +++ b/src/Umbraco.Core/ReadLock.cs @@ -1,20 +1,9 @@ using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading; namespace Umbraco.Core { - /// - /// Provides a convenience methodology for implementing locked access to resources. - /// - /// - /// Intended as an infrastructure class. - /// This is a very inefficient 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! - /// + [Obsolete("Use ReaderWriterLockSlim directly. This will be removed in future versions.")] public class ReadLock : IDisposable { private readonly ReaderWriterLockSlim _rwLock; diff --git a/src/Umbraco.Core/Services/IdkMap.cs b/src/Umbraco.Core/Services/IdkMap.cs index f352def49e..b1caa1ea59 100644 --- a/src/Umbraco.Core/Services/IdkMap.cs +++ b/src/Umbraco.Core/Services/IdkMap.cs @@ -7,7 +7,7 @@ using Umbraco.Core.Scoping; namespace Umbraco.Core.Services { - public class IdkMap + public class IdkMap : IDisposable { private readonly IScopeProvider _scopeProvider; private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(); @@ -46,6 +46,7 @@ namespace Umbraco.Core.Services private readonly ConcurrentDictionary id2key, Func key2id)> _dictionary = new ConcurrentDictionary id2key, Func key2id)>(); + private bool _disposedValue; internal void SetMapper(UmbracoObjectTypes umbracoObjectType, Func id2key, Func key2id) { @@ -106,8 +107,8 @@ namespace Umbraco.Core.Services } finally { - if (_locker.IsReadLockHeld) - _locker.ExitReadLock(); + if (_locker.IsWriteLockHeld) + _locker.ExitWriteLock(); } } @@ -125,8 +126,8 @@ namespace Umbraco.Core.Services } finally { - if (_locker.IsReadLockHeld) - _locker.ExitReadLock(); + if (_locker.IsWriteLockHeld) + _locker.ExitWriteLock(); } } #endif @@ -369,5 +370,23 @@ namespace Umbraco.Core.Services public T Id { get; } } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _locker.Dispose(); + } + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/UpgradeableReadLock.cs b/src/Umbraco.Core/UpgradeableReadLock.cs index e3717fdf9a..656470cd8a 100644 --- a/src/Umbraco.Core/UpgradeableReadLock.cs +++ b/src/Umbraco.Core/UpgradeableReadLock.cs @@ -1,17 +1,9 @@ using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading; namespace Umbraco.Core { - /// - /// Provides a convenience methodology for implementing locked access to resources. - /// - /// - /// Intended as an infrastructure class. - /// + [Obsolete("Use ReaderWriterLockSlim directly. This will be removed in future versions.")] public class UpgradeableReadLock : IDisposable { private readonly ReaderWriterLockSlim _rwLock; diff --git a/src/Umbraco.Core/WriteLock.cs b/src/Umbraco.Core/WriteLock.cs index dfa170218b..1b698dc59c 100644 --- a/src/Umbraco.Core/WriteLock.cs +++ b/src/Umbraco.Core/WriteLock.cs @@ -1,20 +1,9 @@ using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading; namespace Umbraco.Core { - /// - /// Provides a convenience methodology for implementing locked access to resources. - /// - /// - /// Intended as an infrastructure class. - /// This is a very inefficient 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! - /// + [Obsolete("Use ReaderWriterLockSlim directly. This will be removed in future versions.")] public class WriteLock : IDisposable { private readonly ReaderWriterLockSlim _rwLock; diff --git a/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs b/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs index 8ef99383a4..5b5f54cdc8 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs @@ -21,7 +21,7 @@ using File = System.IO.File; namespace Umbraco.ModelsBuilder.Embedded { - internal class PureLiveModelFactory : ILivePublishedModelFactory2, IRegisteredObject + internal class PureLiveModelFactory : ILivePublishedModelFactory2, IRegisteredObject, IDisposable { private Assembly _modelsAssembly; private Infos _infos = new Infos { ModelInfos = null, ModelTypeMap = new Dictionary() }; @@ -33,6 +33,7 @@ namespace Umbraco.ModelsBuilder.Embedded private int _ver, _skipver; private readonly int _debugLevel; private BuildManager _theBuildManager; + private bool _disposedValue; private readonly Lazy _umbracoServices; // fixme: this is because of circular refs :( private UmbracoServices UmbracoServices => _umbracoServices.Value; @@ -677,11 +678,31 @@ namespace Umbraco.ModelsBuilder.Embedded public void Stop(bool immediate) { - _watcher.EnableRaisingEvents = false; - _watcher.Dispose(); + Dispose(); HostingEnvironment.UnregisterObject(this); } + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _watcher.EnableRaisingEvents = false; + _watcher.Dispose(); + _locker.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } + #endregion } } diff --git a/src/Umbraco.Web/Logging/WebProfilerProvider.cs b/src/Umbraco.Web/Logging/WebProfilerProvider.cs index f38a606745..8dad5f5b96 100755 --- a/src/Umbraco.Web/Logging/WebProfilerProvider.cs +++ b/src/Umbraco.Web/Logging/WebProfilerProvider.cs @@ -16,7 +16,7 @@ namespace Umbraco.Web.Logging /// internal class WebProfilerProvider : AspNetRequestProvider { - private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(); + private readonly object _locker = new object(); private MiniProfiler _startupProfiler; private int _first; private volatile BootPhase _bootPhase; @@ -39,8 +39,7 @@ namespace Umbraco.Web.Logging public void BeginBootRequest() { - _locker.EnterWriteLock(); - try + lock (_locker) { if (_bootPhase != BootPhase.Boot) throw new InvalidOperationException("Invalid boot phase."); @@ -48,28 +47,19 @@ namespace Umbraco.Web.Logging // assign the profiler to be the current MiniProfiler for the request // is's already active, starting and all - HttpContext.Current.Items[":mini-profiler:"] = _startupProfiler; - } - finally - { - _locker.ExitWriteLock(); + HttpContext.Current.Items[":mini-profiler:"] = _startupProfiler; } } public void EndBootRequest() { - _locker.EnterWriteLock(); - try + lock (_locker) { if (_bootPhase != BootPhase.BootRequest) - throw new InvalidOperationException("Invalid boot phase."); + throw new InvalidOperationException("Invalid boot phase."); _bootPhase = BootPhase.Booted; - _startupProfiler = null; - } - finally - { - _locker.ExitWriteLock(); + _startupProfiler = null; } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/MemberCache.cs b/src/Umbraco.Web/PublishedCache/NuCache/MemberCache.cs index 2e196f629e..f311a7f302 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/MemberCache.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/MemberCache.cs @@ -13,7 +13,7 @@ using Umbraco.Web.PublishedCache.NuCache.Navigable; namespace Umbraco.Web.PublishedCache.NuCache { - internal class MemberCache : IPublishedMemberCache, INavigableData + internal class MemberCache : IPublishedMemberCache, INavigableData, IDisposable { private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; @@ -23,6 +23,7 @@ namespace Umbraco.Web.PublishedCache.NuCache private readonly IMemberService _memberService; private readonly PublishedContentTypeCache _contentTypeCache; private readonly bool _previewDefault; + private bool _disposedValue; public MemberCache(bool previewDefault, IAppCache snapshotCache, IMemberService memberService, PublishedContentTypeCache contentTypeCache, IPublishedSnapshotAccessor publishedSnapshotAccessor, IVariationContextAccessor variationContextAccessor, IEntityXmlSerializer entitySerializer) @@ -158,6 +159,25 @@ namespace Umbraco.Web.PublishedCache.NuCache return _contentTypeCache.Get(PublishedItemType.Member, alias); } + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _contentTypeCache.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } + #endregion } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs index 3f5c1aa4d5..85bad38ac7 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs @@ -32,6 +32,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { ContentCache.Dispose(); MediaCache.Dispose(); + MemberCache.Dispose(); } } diff --git a/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs b/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs index 0f6e9af6bd..c9db02f091 100644 --- a/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs +++ b/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs @@ -13,7 +13,7 @@ namespace Umbraco.Web.PublishedCache /// Represents a content type cache. /// /// This cache is not snapshotted, so it refreshes any time things change. - public class PublishedContentTypeCache + public class PublishedContentTypeCache : IDisposable { // NOTE: These are not concurrent dictionaries because all access is done within a lock private readonly Dictionary _typesByAlias = new Dictionary(); @@ -320,6 +320,8 @@ namespace Umbraco.Web.PublishedCache // for unit tests - changing the callback must reset the cache obviously // TODO: Why does this even exist? For testing you'd pass in a mocked service to get by id private Func _getPublishedContentTypeById; + private bool _disposedValue; + internal Func GetPublishedContentTypeById { get => _getPublishedContentTypeById; @@ -367,5 +369,24 @@ namespace Umbraco.Web.PublishedCache { return GetAliasKey(contentType.ItemType, contentType.Alias); } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _lock.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Web/Routing/SiteDomainHelper.cs b/src/Umbraco.Web/Routing/SiteDomainHelper.cs index 6173dfb43c..26937b2899 100644 --- a/src/Umbraco.Web/Routing/SiteDomainHelper.cs +++ b/src/Umbraco.Web/Routing/SiteDomainHelper.cs @@ -4,13 +4,14 @@ using System.Linq; using System.Threading; using System.Text.RegularExpressions; using Umbraco.Core; +using System.ComponentModel; namespace Umbraco.Web.Routing { /// /// Provides utilities to handle site domains. /// - public class SiteDomainHelper : ISiteDomainHelper + public class SiteDomainHelper : ISiteDomainHelper, IDisposable { #region Configure @@ -18,6 +19,7 @@ namespace Umbraco.Web.Routing private static Dictionary _sites; private static Dictionary> _bindings; private static Dictionary> _qualifiedSites; + private bool _disposedValue; // these are for unit tests *only* // ReSharper disable ConvertToAutoPropertyWithPrivateSetter @@ -30,16 +32,10 @@ namespace Umbraco.Web.Routing private const string DomainValidationSource = @"^(((?i:http[s]?://)?([-\w]+(\.[-\w]+)*)(:\d+)?(/)?))$"; private static readonly Regex DomainValidation = new Regex(DomainValidationSource, RegexOptions.IgnoreCase | RegexOptions.Compiled); - /// - /// Returns a disposable object that represents safe write access to config. - /// - /// Should be used in a using(SiteDomainHelper.ConfigWriteLock) { ... } mode. + [EditorBrowsable(EditorBrowsableState.Never)] protected static IDisposable ConfigWriteLock => new WriteLock(ConfigLock); - /// - /// Returns a disposable object that represents safe read access to config. - /// - /// Should be used in a using(SiteDomainHelper.ConfigWriteLock) { ... } mode. + [EditorBrowsable(EditorBrowsableState.Never)] protected static IDisposable ConfigReadLock => new ReadLock(ConfigLock); /// @@ -313,6 +309,28 @@ namespace Umbraco.Web.Routing return ret; } + #endregion + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + // This is pretty nasty disposing a static on an instance but it's because this whole class + // is pretty fubar. I'm sure we've fixed this all up in netcore now? We need to remove all statics. + ConfigLock.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } }