From 117dd09f8c089a22ff2e9911477e5e8dbcf3be15 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 4 Dec 2015 11:11:32 -0800 Subject: [PATCH] Improving monitors usage for output cache --- .../Filters/OutputCacheFilter.cs | 68 ++++++++++++------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.OutputCache/Filters/OutputCacheFilter.cs b/src/Orchard.Web/Modules/Orchard.OutputCache/Filters/OutputCacheFilter.cs index f273a1b57..56be73b56 100644 --- a/src/Orchard.Web/Modules/Orchard.OutputCache/Filters/OutputCacheFilter.cs +++ b/src/Orchard.Web/Modules/Orchard.OutputCache/Filters/OutputCacheFilter.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Specialized; using System.Globalization; @@ -24,11 +23,10 @@ using Orchard.UI.Admin; using Orchard.Utility.Extensions; namespace Orchard.OutputCache.Filters { - public class OutputCacheFilter : FilterProvider, IActionFilter, IResultFilter { + public class OutputCacheFilter : FilterProvider, IActionFilter, IResultFilter, IDisposable { private static string _refreshKey = "__r"; private static long _epoch = new DateTime(2014, DateTimeKind.Utc).Ticks; - private static readonly ConcurrentDictionary _cacheKeyLocks = new ConcurrentDictionary(); // Dependencies. private readonly ICacheManager _cacheManager; @@ -41,6 +39,8 @@ namespace Orchard.OutputCache.Filters { private readonly ICacheService _cacheService; private readonly ISignals _signals; private readonly ShellSettings _shellSettings; + private bool _isDisposed = false; + public ILogger Logger { get; set; } public OutputCacheFilter( @@ -96,15 +96,11 @@ namespace Orchard.OutputCache.Filters { return; // Computing the cache key after we know that the request is cacheable means that we are only performing this calculation on requests that require it - _cacheKey = ComputeCacheKey(filterContext, GetCacheKeyParameters(filterContext)); + _cacheKey = String.Intern(ComputeCacheKey(filterContext, GetCacheKeyParameters(filterContext))); _invariantCacheKey = ComputeCacheKey(filterContext, null); Logger.Debug("Cache key '{0}' was created.", _cacheKey); - // The cache key lock for a given cache key is used to synchronize requests to - // ensure only a single request is regenerating the item. - var cacheKeyLock = _cacheKeyLocks.GetOrAdd(_cacheKey, x => new object()); - try { // Is there a cached item, and are we allowed to serve it? @@ -118,7 +114,7 @@ namespace Orchard.OutputCache.Filters { if (cacheItem.IsInGracePeriod(_now)) { // Render the content unless another request is already doing so. - if (Monitor.TryEnter(cacheKeyLock)) { + if (Monitor.TryEnter(_cacheKey)) { Logger.Debug("Item '{0}' is in grace period and not currently being rendered; rendering item...", _cacheKey); BeginRenderItem(filterContext); return; @@ -135,7 +131,7 @@ namespace Orchard.OutputCache.Filters { // No cached item found, or client doesn't want it; acquire the cache key // lock to render the item. Logger.Debug("Item '{0}' was not found in cache or client refuses it. Acquiring cache key lock...", _cacheKey); - if (Monitor.TryEnter(cacheKeyLock, TimeSpan.FromSeconds(20))) { + if (Monitor.TryEnter(_cacheKey)) { Logger.Debug("Cache key lock for item '{0}' was acquired.", _cacheKey); // Item might now have been rendered and cached by another request; if so serve it from cache. @@ -143,7 +139,7 @@ namespace Orchard.OutputCache.Filters { cacheItem = GetCacheItem(_cacheKey); if (cacheItem != null) { Logger.Debug("Item '{0}' was now found; releasing cache key lock and serving from cache.", _cacheKey); - Monitor.Exit(cacheKeyLock); + Monitor.Exit(_cacheKey); ServeCachedItem(filterContext, cacheItem); return; } @@ -159,8 +155,7 @@ namespace Orchard.OutputCache.Filters { catch { // Remember to release the cache key lock in the event of an exception! Logger.Debug("Exception occurred for item '{0}'; releasing any acquired lock.", _cacheKey); - if (Monitor.IsEntered(cacheKeyLock)) - Monitor.Exit(cacheKeyLock); + ReleaseCacheKeyLock(); throw; } } @@ -177,11 +172,11 @@ namespace Orchard.OutputCache.Filters { var captureHandlerIsAttached = false; try { - + // This filter is not reentrant (multiple executions within the same request are // not supported) so child actions are ignored completely. if (filterContext.IsChildAction || !_isCachingRequest) - return; + return; Logger.Debug("Item '{0}' was rendered.", _cacheKey); @@ -358,7 +353,7 @@ namespace Orchard.OutputCache.Filters { protected virtual IDictionary GetCacheKeyParameters(ActionExecutingContext filterContext) { var result = new Dictionary(); - + // Vary by action parameters. foreach (var p in filterContext.ActionParameters) result.Add("PARAM:" + p.Key, p.Value); @@ -381,7 +376,7 @@ namespace Orchard.OutputCache.Filters { result["HEADER:" + varyByRequestHeader] = requestHeaders[varyByRequestHeader]; } - + // Vary by request culture if configured. if (CacheSettings.VaryByCulture) { result["culture"] = _workContext.CurrentCulture.ToLowerInvariant(); @@ -472,7 +467,7 @@ namespace Orchard.OutputCache.Filters { var response = filterContext.HttpContext.Response; // Fix for missing charset in response headers - response.Charset = response.Charset; + response.Charset = response.Charset; // Adds some caching information to the output if requested. if (CacheSettings.DebugMode) { @@ -538,12 +533,10 @@ namespace Orchard.OutputCache.Filters { } private void ReleaseCacheKeyLock() { - if (_cacheKey != null) { - object cacheKeyLock; - if (_cacheKeyLocks.TryGetValue(_cacheKey, out cacheKeyLock) && Monitor.IsEntered(cacheKeyLock)) { - Logger.Debug("Releasing cache key lock for item '{0}'.", _cacheKey); - Monitor.Exit(cacheKeyLock); - } + if (_cacheKey != null && Monitor.IsEntered(_cacheKey)) { + Logger.Debug("Releasing cache key lock for item '{0}'.", _cacheKey); + Monitor.Exit(_cacheKey); + _cacheKey = null; } } @@ -597,12 +590,37 @@ namespace Orchard.OutputCache.Filters { var cacheItem = _cacheStorageProvider.GetCacheItem(key); return cacheItem; } - catch(Exception e) { + catch (Exception e) { Logger.Error(e, "An unexpected error occured while reading a cache entry"); } return null; } + + public void Dispose() { + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) { + if (!_isDisposed) { + if (disposing) { + // Free other state (managed objects). + } + + if (_cacheKey != null && Monitor.IsEntered(_cacheKey)) { + Monitor.Exit(_cacheKey); + } + + _isDisposed = true; + } + } + + ~OutputCacheFilter() { + // Ensure locks are released even after an unexpected exception + Dispose(false); + } + } public class ViewDataContainer : IViewDataContainer {