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

U4-4931 - lock contention & potential application lockdown

Browse Source 
Conflicts:
	src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs
	src/umbraco.cms/Actions/Action.cs
This commit is contained in:
Stephan
2014-05-16 16:48:31 +02:00
parent 60d6ae44b5
commit 8729156fbf
4 changed files with 64 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -81,25 +81,36 @@ namespace Umbraco.Core.Cache
{
cacheKey = GetCacheKey(cacheKey);
// NOTE - because we don't know what getCacheItem does, how long it will take and whether it will hang,
// getCacheItem should run OUTSIDE of the global application lock else we run into lock contention and
// nasty performance issues.
// So.... we insert a Lazy<object> in the cache while holding the global application lock, and then rely
// on the Lazy lock to ensure that getCacheItem runs once and everybody waits on it, while the global
// application lock has been released.
// Note that this means we'll end up storing null values in the cache, whereas in the past we made sure
// not to store them. There's code below, commented out, to make sure we do re-compute the value if it
// was previously computed as null - effectively reproducing the past behavior - but I'm not quite sure
// it is a good idea.
Lazy<object> result;
using (var lck = new UpgradeableReadLock(Locker))
{
var result = DictionaryCache.Get(cacheKey);
if (result == null)
result = DictionaryCache.Get(cacheKey) as Lazy<object>;
if (result == null /* || (result.IsValueCreated && result.Value == null) */)
{
lck.UpgradeToWriteLock();
result = getCacheItem();
if (result != null)
{
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);
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
}
result = new Lazy<object>(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);
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
}
return result;
}
return result.Value;
}
public object GetCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)
@@ -124,8 +135,11 @@ namespace Umbraco.Core.Cache
/// <param name="dependency"></param>
internal void InsertCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout = null, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, CacheDependency dependency = null)
{
var result = getCacheItem();
if (result == null) return;
// 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. Though I'm not sure it is a good idea.
var result = new Lazy<object>(getCacheItem);
//if (result.Value == null) return;
cacheKey = GetCacheKey(cacheKey);

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

@@ -141,32 +141,35 @@ namespace Umbraco.Core.Cache
CacheItemRemovedCallback removedCallback = null,
string[] dependentFiles = null)
{
// see notes in HttpRuntimeCacheProvider
Lazy<object> result;
using (var lck = new UpgradeableReadLock(Locker))
{
var result = MemoryCache.Get(cacheKey);
if (result == null)
result = MemoryCache.Get(cacheKey) as Lazy<object>;
if (result == null /* || (result.IsValueCreated && result.Value == null) */)
{
lck.UpgradeToWriteLock();
result = getCacheItem();
if (result != null)
{
var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles);
MemoryCache.Set(cacheKey, result, policy);
}
result = new Lazy<object>(getCacheItem);
var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles);
MemoryCache.Set(cacheKey, result, policy);
}
return result;
}
return result.Value;
}
public void InsertCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout = null, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)
{
object result = getCacheItem();
if (result != null)
{
var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles);
MemoryCache.Set(cacheKey, result, policy);
}
// see notes in HttpRuntimeCacheProvider
var result = new Lazy<object>(getCacheItem);
//if (result.Value == null) return;
var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles);
MemoryCache.Set(cacheKey, result, policy);
}
private static CacheItemPolicy GetPolicy(TimeSpan? timeout = null, bool isSliding = false, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)

5
src/Umbraco.Core/ObjectResolution/Resolution.cs
View File

@@ -63,6 +63,11 @@ namespace Umbraco.Core.ObjectResolution
}
}
// NOTE - the ugly code below exists only because of umbraco.BusinessLogic.Actions.Action.ReRegisterActionsAndHandlers
// which wants to re-register actions and handlers instead of properly restarting the application. Don't even think
// about using it for anything else. Also, while the backdoor is open, the resolution system is locked so nothing
// can work properly => deadlocks. Therefore, open the backdoor, do resolution changes EXCLUSIVELY, and close the door!
/// <summary>
/// Returns a disposable object that reprents dirty access to temporarily unfrozen resolution configuration.
/// </summary>

23
src/umbraco.cms/Actions/Action.cs
View File

@@ -33,7 +33,6 @@ namespace umbraco.BusinessLogic.Actions
[Obsolete("Actions and ActionHandlers are obsolete and should no longer be used")]
public class Action
{
private static readonly Dictionary<string, string> ActionJs = new Dictionary<string, string>();
private static readonly object Lock = new object();
@@ -49,21 +48,25 @@ namespace umbraco.BusinessLogic.Actions
/// </summary>
/// <remarks>
/// TODO: this shouldn't be needed... we should restart the app pool when a package is installed!
/// </remarks>
/// </remarks>
public static void ReRegisterActionsAndHandlers()
{
lock (Lock)
{
lock (Lock)
{
// NOTE use the DirtyBackdoor to change the resolution configuration EXCLUSIVELY
// ie do NOT do ANYTHING else while holding the backdoor, because while it is open
// the whole resolution system is locked => nothing can work properly => deadlocks
var newResolver = new ActionsResolver(
() => TypeFinder.FindClassesOfType<IAction>(PluginManager.Current.AssembliesToScan));
using (Umbraco.Core.ObjectResolution.Resolution.DirtyBackdoorToConfiguration)
{
//TODO: Based on the above, this is a big hack as types should all be cleared on package install!
ActionsResolver.Reset(false); // and do NOT reset the whole resolution!
//TODO: Based on the above, this is a big hack as types should all be cleared on package install!
ActionsResolver.Current = new ActionsResolver(
() => TypeFinder.FindClassesOfType<IAction>(PluginManager.Current.AssembliesToScan));
ActionsResolver.Current = newResolver;
}
}
}
}
/// <summary>