From bf7a3251d80b2855605c6cf9b7c1c1e73712349a Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 20 Apr 2021 17:56:55 +1000 Subject: [PATCH] Ensure more ReaderWriterLockSlim are disposed or converted --- .../PureLiveModelFactory.cs | 27 ++++++++++++-- .../Logging/WebProfilerProvider.cs | 22 ++++-------- .../PublishedCache/NuCache/MemberCache.cs | 22 +++++++++++- .../NuCache/PublishedSnapshot.cs | 1 + .../PublishedContentTypeCache.cs | 23 +++++++++++- src/Umbraco.Web/Routing/SiteDomainHelper.cs | 36 ++++++++++++++----- 6 files changed, 101 insertions(+), 30 deletions(-) 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); + } } }