From fdadd80414af2dc773678ac5cb178881a7fc9d5e Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 17 Jun 2014 18:42:00 +0200 Subject: [PATCH] U4-4931 - do not cache exceptions Conflicts: src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs --- .../Cache/DictionaryCacheProviderBase.cs | 24 ++++++++++++++----- .../Cache/HttpRequestCacheProvider.cs | 5 +++- .../Cache/HttpRuntimeCacheProvider.cs | 7 ++++-- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs b/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs index 52144ed17c..28beb19f4d 100644 --- a/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs +++ b/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs @@ -26,6 +26,18 @@ namespace Umbraco.Core.Cache return string.Format("{0}-{1}", CacheItemPrefix, key); } + protected object GetSafeLazyValue(Lazy lazy) + { + try + { + return lazy.Value; + } + catch + { + return null; + } + } + #region Clear public virtual void ClearAllCache() @@ -56,7 +68,7 @@ namespace Umbraco.Core.Cache { // entry.Value is Lazy and not null, its value may be null // remove null values as well, does not hurt - var value = ((Lazy) x.Value).Value; + var value = GetSafeLazyValue((Lazy) x.Value); // return exceptions as null return value == null || value.GetType().ToString().InvariantEquals(typeName); }) .ToArray()) @@ -75,7 +87,7 @@ namespace Umbraco.Core.Cache // entry.Value is Lazy and not null, its value may be null // remove null values as well, does not hurt // compare on exact type, don't use "is" - var value = ((Lazy)x.Value).Value; + var value = GetSafeLazyValue((Lazy)x.Value); // return exceptions as null return value == null || value.GetType() == typeOfT; }) .ToArray()) @@ -95,7 +107,7 @@ namespace Umbraco.Core.Cache // entry.Value is Lazy and not null, its value may be null // remove null values as well, does not hurt // compare on exact type, don't use "is" - var value = ((Lazy)x.Value).Value; + var value = GetSafeLazyValue((Lazy)x.Value); // return exceptions as null if (value == null) return true; return value.GetType() == typeOfT // run predicate on the 'public key' part only, ie without prefix @@ -140,7 +152,7 @@ namespace Umbraco.Core.Cache { return GetDictionaryEntries() .Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith)) - .Select(x => ((Lazy)x.Value).Value) + .Select(x => GetSafeLazyValue((Lazy)x.Value)) // return exceptions as null .Where(x => x != null) // backward compat, don't store null values in the cache .ToList(); } @@ -154,7 +166,7 @@ namespace Umbraco.Core.Cache { return GetDictionaryEntries() .Where(x => Regex.IsMatch(((string)x.Key).Substring(plen), regexString)) - .Select(x => ((Lazy)x.Value).Value) + .Select(x => GetSafeLazyValue((Lazy)x.Value)) // return exceptions as null .Where(x => x != null) // backward compat, don't store null values in the cache .ToList(); } @@ -166,7 +178,7 @@ namespace Umbraco.Core.Cache using (new ReadLock(Locker)) { var result = GetEntry(cacheKey) as Lazy; // null if key not found - return result == null ? null : result.Value; + return result == null ? null : GetSafeLazyValue(result); // return exceptions as null } } diff --git a/src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs b/src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs index 989fd3a69d..c40ae3843c 100644 --- a/src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs +++ b/src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs @@ -53,7 +53,7 @@ namespace Umbraco.Core.Cache using (var lck = new UpgradeableReadLock(Locker)) { result = _context().Items[cacheKey] as Lazy; // null if key not found - if (result == null || (result.IsValueCreated && result.Value == null)) + if (result == null || (result.IsValueCreated && GetSafeLazyValue(result) == null)) // get exceptions as null { lck.UpgradeToWriteLock(); @@ -62,6 +62,9 @@ namespace Umbraco.Core.Cache } } + // this may throw if getCacheItem throws, but this is the only place where + // it would throw as everywhere else we use GetLazySaveValue() to hide exceptions + // and pretend exceptions were never inserted into cache to begin with. return result.Value; } diff --git a/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs b/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs index 70e18fada4..79c8116b17 100644 --- a/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs +++ b/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs @@ -85,7 +85,7 @@ namespace Umbraco.Core.Cache using (var lck = new UpgradeableReadLock(Locker)) { result = _cache.Get(cacheKey) as Lazy; // null if key not found - if (result == null || (result.IsValueCreated && result.Value == null)) + if (result == null || (result.IsValueCreated && GetSafeLazyValue(result) == null)) // get exceptions as null { lck.UpgradeToWriteLock(); @@ -96,6 +96,9 @@ namespace Umbraco.Core.Cache } } + // this may throw if getCacheItem throws, but this is the only place where + // it would throw as everywhere else we use GetLazySaveValue() to hide exceptions + // and pretend exceptions were never inserted into cache to begin with. return result.Value; } @@ -129,7 +132,7 @@ namespace Umbraco.Core.Cache // and make sure we don't store a null value. var result = new Lazy(getCacheItem); - var value = result.Value; // force evaluation now + var value = result.Value; // force evaluation now - this may throw if cacheItem throws, and then nothing goes into cache if (value == null) return; // do not store null values (backward compat) cacheKey = GetCacheKey(cacheKey);