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

U4-6597 - fix issue with caches that would cache exceptions

Browse Source
This commit is contained in:
Stephan
2015-05-19 09:16:05 +02:00
parent 16e06b6d4d
commit dbaf7c0190
7 changed files with 164 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -12,7 +12,7 @@ namespace Umbraco.Core.Cache
protected const string CacheItemPrefix = "umbrtmche";
// an object that represent a value that has not been created yet
protected readonly object ValueNotCreated = new object();
protected internal static readonly object ValueNotCreated = new object();
// manupulate the underlying cache entries
// these *must* be called from within the appropriate locks
@@ -30,19 +30,49 @@ namespace Umbraco.Core.Cache
return string.Format("{0}-{1}", CacheItemPrefix, key);
}
protected object GetSafeLazyValue(Lazy<object> lazy, bool onlyIfValueIsCreated = false)
protected internal static Lazy<object> GetSafeLazy(Func<object> getCacheItem)
{
try
// try to generate the value and if it fails,
// wrap in an ExceptionHolder - would be much simpler
// to just use lazy.IsValueFaulted alas that field is
// internal
return new Lazy<object>(() =>
{
// if onlyIfValueIsCreated, do not trigger value creation
// must return something, though, to differenciate from null values
if (onlyIfValueIsCreated && lazy.IsValueCreated == false) return ValueNotCreated;
return lazy.Value;
}
catch
try
{
return getCacheItem();
}
catch (Exception e)
{
return new ExceptionHolder(e);
}
});
}
protected internal static object GetSafeLazyValue(Lazy<object> lazy, bool onlyIfValueIsCreated = false)
{
// if onlyIfValueIsCreated, do not trigger value creation
// must return something, though, to differenciate from null values
if (onlyIfValueIsCreated && lazy.IsValueCreated == false) return ValueNotCreated;
// if execution has thrown then lazy.IsValueCreated is false
// and lazy.IsValueFaulted is true (but internal) so we use our
// own exception holder (see Lazy<T> source code) to return null
if (lazy.Value is ExceptionHolder) return null;
// we have a value and execution has not thrown so returning
// here does not throw
return lazy.Value;
}
internal class ExceptionHolder
{
public ExceptionHolder(Exception e)
{
return null;
Exception = e;
}
public Exception Exception { get; private set; }
}
#region Clear

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

@@ -105,15 +105,22 @@ namespace Umbraco.Core.Cache
if (result == null || GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
{
result = new Lazy<object>(getCacheItem);
result = GetSafeLazy(getCacheItem);
ContextItems[cacheKey] = result;
}
}
// 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;
// using GetSafeLazy and GetSafeLazyValue ensures that we don't cache
// exceptions (but try again and again) and silently eat them - however at
// some point we have to report them - so need to re-throw here
// this does not throw anymore
//return result.Value;
var value = result.Value; // will not throw (safe lazy)
var eh = value as ExceptionHolder;
if (eh != null) throw eh.Exception; // throw once!
return value;
}
#endregion

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

@@ -129,7 +129,7 @@ namespace Umbraco.Core.Cache
if (result == null || GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
{
result = new Lazy<object>(getCacheItem);
result = GetSafeLazy(getCacheItem);
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);
@@ -138,10 +138,17 @@ 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;
// using GetSafeLazy and GetSafeLazyValue ensures that we don't cache
// exceptions (but try again and again) and silently eat them - however at
// some point we have to report them - so need to re-throw here
// this does not throw anymore
//return result.Value;
value = result.Value; // will not throw (safe lazy)
var eh = value as ExceptionHolder;
if (eh != null) throw eh.Exception; // throw once!
return value;
}
public object GetCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)
@@ -173,7 +180,7 @@ namespace Umbraco.Core.Cache
// NOTE - here also we must insert a Lazy<object> but we can evaluate it right now
// and make sure we don't store a null value.
var result = new Lazy<object>(getCacheItem);
var result = GetSafeLazy(getCacheItem);
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)

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

@@ -19,29 +19,11 @@ namespace Umbraco.Core.Cache
private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
internal ObjectCache MemoryCache;
// an object that represent a value that has not been created yet
protected readonly object ValueNotCreated = new object();
public ObjectCacheRuntimeCacheProvider()
{
MemoryCache = new MemoryCache("in-memory");
}
protected object GetSafeLazyValue(Lazy<object> lazy, bool onlyIfValueIsCreated = false)
{
try
{
// if onlyIfValueIsCreated, do not trigger value creation
// must return something, though, to differenciate from null values
if (onlyIfValueIsCreated && lazy.IsValueCreated == false) return ValueNotCreated;
return lazy.Value;
}
catch
{
return null;
}
}
#region Clear
public virtual void ClearAllCache()
@@ -72,7 +54,7 @@ namespace Umbraco.Core.Cache
// x.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 = DictionaryCacheProviderBase.GetSafeLazyValue((Lazy<object>)x.Value, true);
return value == null || value.GetType().ToString().InvariantEquals(typeName);
})
.Select(x => x.Key)
@@ -92,7 +74,7 @@ namespace Umbraco.Core.Cache
// x.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 = DictionaryCacheProviderBase.GetSafeLazyValue((Lazy<object>)x.Value, true);
return value == null || value.GetType() == typeOfT;
})
.Select(x => x.Key)
@@ -112,7 +94,7 @@ namespace Umbraco.Core.Cache
// x.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 = DictionaryCacheProviderBase.GetSafeLazyValue((Lazy<object>)x.Value, true);
if (value == null) return true;
return value.GetType() == typeOfT
&& predicate(x.Key, (T) value);
@@ -161,7 +143,7 @@ namespace Umbraco.Core.Cache
.ToArray(); // evaluate while locked
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.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
.ToList();
}
@@ -176,7 +158,7 @@ namespace Umbraco.Core.Cache
.ToArray(); // evaluate while locked
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.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
.ToList();
}
@@ -188,7 +170,7 @@ namespace Umbraco.Core.Cache
{
result = MemoryCache.Get(cacheKey) as Lazy<object>; // null if key not found
}
return result == null ? null : GetSafeLazyValue(result); // return exceptions as null
return result == null ? null : DictionaryCacheProviderBase.GetSafeLazyValue(result); // return exceptions as null
}
public object GetCacheItem(string cacheKey, Func<object> getCacheItem)
@@ -205,9 +187,9 @@ namespace Umbraco.Core.Cache
using (var lck = new UpgradeableReadLock(_locker))
{
result = MemoryCache.Get(cacheKey) as Lazy<object>;
if (result == null || GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
if (result == null || DictionaryCacheProviderBase.GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
{
result = new Lazy<object>(getCacheItem);
result = DictionaryCacheProviderBase.GetSafeLazy(getCacheItem);
var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles);
lck.UpgradeToWriteLock();
@@ -215,7 +197,12 @@ namespace Umbraco.Core.Cache
}
}
return result.Value;
//return result.Value;
var value = result.Value; // will not throw (safe lazy)
var eh = value as DictionaryCacheProviderBase.ExceptionHolder;
if (eh != null) throw eh.Exception; // throw once!
return value;
}
#endregion
@@ -227,7 +214,7 @@ namespace Umbraco.Core.Cache
// NOTE - here also we must insert a Lazy<object> but we can evaluate it right now
// and make sure we don't store a null value.
var result = new Lazy<object>(getCacheItem);
var result = DictionaryCacheProviderBase.GetSafeLazy(getCacheItem);
var value = result.Value; // force evaluation now
if (value == null) return; // do not store null values (backward compat)

56
src/Umbraco.Tests/Cache/CacheProviderTests.cs
View File

@@ -1,4 +1,5 @@
using System.Linq;
using System;
using System.Linq;
using System.Web.UI;
using NUnit.Framework;
using Umbraco.Core.Cache;
@@ -23,6 +24,59 @@ namespace Umbraco.Tests.Cache
Provider.ClearAllCache();
}
[Test]
public void Does_Not_Cache_Exceptions()
{
var counter = 0;
object result;
try
{
result = Provider.GetCacheItem("Blah", () =>
{
counter++;
throw new Exception("Do not cache this");
});
}
catch (Exception){}
try
{
result = Provider.GetCacheItem("Blah", () =>
{
counter++;
throw new Exception("Do not cache this");
});
}
catch (Exception){}
Assert.Greater(counter, 1);
}
[Test]
public void Ensures_Delegate_Result_Is_Cached_Once()
{
var counter = 0;
object result;
result = Provider.GetCacheItem("Blah", () =>
{
counter++;
return "";
});
result = Provider.GetCacheItem("Blah", () =>
{
counter++;
return "";
});
Assert.AreEqual(counter, 1);
}
[Test]
public void Can_Get_By_Search()
{

27
src/Umbraco.Tests/Cache/HttpRuntimeCacheProviderTests.cs
View File

@@ -1,4 +1,5 @@
using System.Web;
using System;
using System.Web;
using NUnit.Framework;
using Umbraco.Core.Cache;
@@ -29,5 +30,29 @@ namespace Umbraco.Tests.Cache
{
get { return _provider; }
}
[Test]
public void DoesNotCacheExceptions()
{
string value;
Assert.Throws<Exception>(() => { value = (string)_provider.GetCacheItem("key", () => GetValue(1)); });
Assert.Throws<Exception>(() => { value = (string)_provider.GetCacheItem("key", () => GetValue(2)); });
// does not throw
value = (string)_provider.GetCacheItem("key", () => GetValue(3));
Assert.AreEqual("succ3", value);
// cache
value = (string)_provider.GetCacheItem("key", () => GetValue(4));
Assert.AreEqual("succ3", value);
}
private static string GetValue(int i)
{
Console.WriteLine("get" + i);
if (i < 3)
throw new Exception("fail");
return "succ" + i;
}
}
}

29
src/Umbraco.Web.UI/config/umbracoSettings.config
View File

@@ -118,33 +118,10 @@
<useDomainPrefixes>false</useDomainPrefixes>
<!-- this will add a trailing slash (/) to urls when in directory url mode -->
<addTrailingSlash>true</addTrailingSlash>
<urlReplacing removeDoubleDashes="true">
<urlReplacing removeDoubleDashes="true" toAscii="true">
<char org=" ">-</char>
<char org="&quot;"></char>
<char org="'"></char>
<char org="%"></char>
<char org="."></char>
<char org=";"></char>
<char org="/"></char>
<char org="\"></char>
<char org=":"></char>
<char org="#"></char>
<char org="+">plus</char>
<char org="*">star</char>
<char org="&amp;"></char>
<char org="?"></char>
<char org="æ">ae</char>
<char org="ø">oe</char>
<char org="å">aa</char>
<char org="ä">ae</char>
<char org="ö">oe</char>
<char org="ü">ue</char>
<char org="ß">ss</char>
<char org="Ä">ae</char>
<char org="Ö">oe</char>
<char org="|">-</char>
<char org="&lt;"></char>
<char org="&gt;"></char>
<char org="ö">oxxx</char>
<!--<char org="ü">ue</char>-->
</urlReplacing>
</requestHandler>