Changes readlocks to normal locks, no need to have extra objects allocated and complex code.

This commit is contained in:
Shannon
2020-01-03 12:39:56 +11:00
parent 3d8e9a78e3
commit e6b333a3ec
2 changed files with 32 additions and 98 deletions

View File

@@ -20,6 +20,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
// this class is an extended version of SnapDictionary
// most of the snapshots management code, etc is an exact copy
// SnapDictionary has unit tests to ensure it all works correctly
// For locking information, see SnapDictionary
private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor;
private readonly IVariationContextAccessor _variationContextAccessor;
@@ -79,11 +80,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
private readonly string _instanceId = Guid.NewGuid().ToString("N");
private class ReadLockInfo
{
public bool Taken;
}
private class WriteLockInfo
{
public bool Taken;
@@ -121,15 +117,10 @@ namespace Umbraco.Web.PublishedCache.NuCache
private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
// TODO: We are in a deadlock here somehow??
Monitor.Enter(_wlocko, ref lockInfo.Taken);
var rtaken = false;
try
lock(_rlocko)
{
Monitor.Enter(_rlocko, ref rtaken);
// see SnapDictionary
try { }
finally
@@ -146,26 +137,16 @@ namespace Umbraco.Web.PublishedCache.NuCache
_nextGen = true;
}
}
}
finally
{
if (rtaken) Monitor.Exit(_rlocko);
}
}
private void Lock(ReadLockInfo lockInfo)
{
Monitor.Enter(_rlocko, ref lockInfo.Taken);
}
}
private void Release(WriteLockInfo lockInfo, bool commit = true)
{
if (commit == false)
{
var rtaken = false;
try
lock(_rlocko)
{
Monitor.Enter(_rlocko, ref rtaken);
// see SnapDictionary
try { }
finally
{
@@ -173,10 +154,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
_liveGen -= 1;
}
}
finally
{
if (rtaken) Monitor.Exit(_rlocko);
}
Rollback(_contentNodes);
RollbackRoot();
@@ -207,11 +184,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
Monitor.Exit(_wlocko);
}
private void Release(ReadLockInfo lockInfo)
{
if (lockInfo.Taken) Monitor.Exit(_rlocko);
}
private void RollbackRoot()
{
if (_root.Gen <= _liveGen) return;
@@ -1248,13 +1220,8 @@ namespace Umbraco.Web.PublishedCache.NuCache
public Snapshot CreateSnapshot()
{
var lockInfo = new ReadLockInfo();
try
lock(_rlocko)
{
// TODO: This would be much simpler with just a lock(_rlocko) { }
// in this case I see no reason why we are using this syntax?!
Lock(lockInfo);
// if no next generation is required, and we already have one,
// use it and create a new snapshot
if (_nextGen == false && _genObj != null)
@@ -1306,10 +1273,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
return snapshot;
}
finally
{
Release(lockInfo);
}
}
public Snapshot LiveSnapshot => new Snapshot(this, _liveGen
@@ -1427,18 +1390,17 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
// TODO: This is never used? Should it be?
public async Task WaitForPendingCollect()
{
Task task;
lock (_rlocko)
{
task = _collectTask;
}
if (task != null)
await task;
}
// TODO: This is never used? Should it be? Maybe move to TestHelper below?
//public async Task WaitForPendingCollect()
//{
// Task task;
// lock (_rlocko)
// {
// task = _collectTask;
// }
// if (task != null)
// await task;
//}
public long GenCount => _genObjs.Count;

View File

@@ -22,6 +22,11 @@ namespace Umbraco.Web.PublishedCache.NuCache
// This class is optimized for many readers, few writers
// Readers are lock-free
// NOTE - we used to lock _rlocko the long hand way with Monitor.Enter(_rlocko, ref lockTaken) but this has
// been replaced with a normal c# lock because that's exactly how the normal c# lock works,
// see https://blogs.msdn.microsoft.com/ericlippert/2009/03/06/locks-and-exceptions-do-not-mix/
// for the readlock, there's no reason here to use the long hand way.
private readonly ConcurrentDictionary<TKey, LinkedNode<TValue>> _items;
private readonly ConcurrentQueue<GenObj> _genObjs;
private GenObj _genObj;
@@ -71,16 +76,11 @@ namespace Umbraco.Web.PublishedCache.NuCache
// are all ignored - Release is private and meant to be invoked with 'commit' being false only
// only on the outermost lock (by SnapDictionaryWriter)
// using (...) {} for locking is prone to nasty leaks in case of weird exceptions
// such as thread-abort or out-of-memory, but let's not worry about it now
// side note - using (...) {} for locking is prone to nasty leaks in case of weird exceptions
// such as thread-abort or out-of-memory, which is why we've moved away from the old using wrapper we had on locking.
private readonly string _instanceId = Guid.NewGuid().ToString("N");
private class ReadLockInfo
{
public bool Taken;
}
private class WriteLockInfo
{
public bool Taken;
@@ -121,18 +121,16 @@ namespace Umbraco.Web.PublishedCache.NuCache
{
Monitor.Enter(_wlocko, ref lockInfo.Taken);
var rtaken = false;
try
lock(_rlocko)
{
Monitor.Enter(_rlocko, ref rtaken);
// assume everything in finally runs atomically
// http://stackoverflow.com/questions/18501678/can-this-unexpected-behavior-of-prepareconstrainedregions-and-thread-abort-be-ex
// http://joeduffyblog.com/2005/03/18/atomicity-and-asynchronous-exception-failures/
// http://joeduffyblog.com/2007/02/07/introducing-the-new-readerwriterlockslim-in-orcas/
// http://chabster.blogspot.fr/2013/12/readerwriterlockslim-fails-on-dual.html
//RuntimeHelpers.PrepareConstrainedRegions();
try { } finally
try { }
finally
{
// increment the lock count, and register that this lock is counting
_wlocked++;
@@ -149,15 +147,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
}
finally
{
if (rtaken) Monitor.Exit(_rlocko);
}
}
private void Lock(ReadLockInfo lockInfo)
{
Monitor.Enter(_rlocko, ref lockInfo.Taken);
}
private void Release(WriteLockInfo lockInfo, bool commit = true)
@@ -168,22 +157,17 @@ namespace Umbraco.Web.PublishedCache.NuCache
if (commit == false)
{
var rtaken = false;
try
lock(_rlocko)
{
Monitor.Enter(_rlocko, ref rtaken);
try { } finally
try { }
finally
{
// forget about the temp. liveGen
_nextGen = false;
_liveGen -= 1;
}
}
finally
{
if (rtaken) Monitor.Exit(_rlocko);
}
foreach (var item in _items)
{
var link = item.Value;
@@ -202,11 +186,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
Monitor.Exit(_wlocko);
}
private void Release(ReadLockInfo lockInfo)
{
if (lockInfo.Taken) Monitor.Exit(_rlocko);
}
#endregion
#region Set, Clear, Get, Has
@@ -347,11 +326,8 @@ namespace Umbraco.Web.PublishedCache.NuCache
public Snapshot CreateSnapshot()
{
var lockInfo = new ReadLockInfo();
try
lock(_rlocko)
{
Lock(lockInfo);
// if no next generation is required, and we already have a gen object,
// use it to create a new snapshot
if (_nextGen == false && _genObj != null)
@@ -398,10 +374,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
return snapshot;
}
finally
{
Release(lockInfo);
}
}
public Task CollectAsync()