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

Removes ability to have recursive locks in SnapDictionary, changes logic to require locking around the methods just like ContentStore, updates tests

Browse Source
This commit is contained in:
Shannon
2020-01-03 15:04:39 +11:00
parent 8e3b3c8326
commit 243e76b3cc
4 changed files with 204 additions and 194 deletions
Download Patch File Download Diff File Expand all files Collapse all files

104
src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs
View File

@@ -39,7 +39,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
private long _liveGen, _floorGen;
private bool _nextGen, _collectAuto;
private Task _collectTask;
private volatile int _wlocked;
private List<KeyValuePair<int, ContentNodeKit>> _wchanges;
// TODO: collection trigger (ok for now)
@@ -83,7 +82,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
private class WriteLockInfo
{
public bool Taken;
public bool Count;
}
// a scope contextual that represents a locked writer to the dictionary
@@ -111,7 +109,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
// TODO: GetScopedWriter? should the dict have a ref onto the scope provider?
public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider)
{
_logger.Debug<ContentStore>("GetScopedWriteLock");
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped));
}
@@ -123,6 +120,9 @@ namespace Umbraco.Web.PublishedCache.NuCache
private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
if (Monitor.IsEntered(_wlocko))
throw new InvalidOperationException("Recursive locks not allowed");
Monitor.Enter(_wlocko, ref lockInfo.Taken);
lock(_rlocko)
@@ -131,9 +131,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
try { }
finally
{
_wlocked++;
lockInfo.Count = true;
if (_nextGen == false || (forceGen && _wlocked == 1))
if (_nextGen == false || (forceGen))
{
// because we are changing things, a new generation
// is created, which will trigger a new snapshot
@@ -179,12 +177,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
_localDb.Commit();
}
// TODO: Shouldn't this be in a finally block?
// TODO: Shouldn't this be decremented after we exit??
// TODO: Shouldn't the locked flag never exceed 1?
if (lockInfo.Count)
_wlocked--;
// TODO: Shouldn't this be in a finally block?
if (lockInfo.Taken)
Monitor.Exit(_wlocko);
@@ -220,22 +212,18 @@ namespace Umbraco.Web.PublishedCache.NuCache
public void ReleaseLocalDb()
{
_logger.Info<ContentStore>("Releasing DB...");
var lockInfo = new WriteLockInfo();
try
{
try
{
// Trying to lock could throw exceptions so always make sure to clean up.
_logger.Info<ContentStore>("Trying to lock before releasing DB (lock count = {LockCount}) ...", _wlocked);
// Trying to lock could throw exceptions so always make sure to clean up.
Lock(lockInfo);
}
finally
{
try
{
_logger.Info<ContentStore>("Disposing local DB...");
_localDb?.Dispose();
}
catch (Exception ex)
@@ -275,57 +263,61 @@ namespace Umbraco.Web.PublishedCache.NuCache
#region Content types
public void NewContentTypes(IEnumerable<IPublishedContentType> types)
/// <summary>
/// Sets data for new content types
/// </summary>
/// <param name="types"></param>
/// <remarks>
/// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock
/// otherwise an exception will occur.
/// </remarks>
/// <exception cref="InvalidOperationException">
/// Thrown if this method is not called within a write lock
/// </exception>
public void NewContentTypesLocked(IEnumerable<IPublishedContentType> types)
{
var lockInfo = new WriteLockInfo();
try
{
_logger.Debug<ContentStore>("NewContentTypes");
Lock(lockInfo);
EnsureLocked();
foreach (var type in types)
{
SetValueLocked(_contentTypesById, type.Id, type);
SetValueLocked(_contentTypesByAlias, type.Alias, type);
}
}
finally
foreach (var type in types)
{
Release(lockInfo);
SetValueLocked(_contentTypesById, type.Id, type);
SetValueLocked(_contentTypesByAlias, type.Alias, type);
}
}
public void UpdateContentTypes(IEnumerable<IPublishedContentType> types)
/// <summary>
/// Sets data for updated content types
/// </summary>
/// <param name="types"></param>
/// <remarks>
/// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock
/// otherwise an exception will occur.
/// </remarks>
/// <exception cref="InvalidOperationException">
/// Thrown if this method is not called within a write lock
/// </exception>
public void UpdateContentTypesLocked(IEnumerable<IPublishedContentType> types)
{
//nothing to do if this is empty, no need to lock/allocate/iterate/etc...
if (!types.Any()) return;
var lockInfo = new WriteLockInfo();
try
EnsureLocked();
var index = types.ToDictionary(x => x.Id, x => x);
foreach (var type in index.Values)
{
_logger.Debug<ContentStore>("UpdateContentTypes");
Lock(lockInfo);
var index = types.ToDictionary(x => x.Id, x => x);
foreach (var type in index.Values)
{
SetValueLocked(_contentTypesById, type.Id, type);
SetValueLocked(_contentTypesByAlias, type.Alias, type);
}
foreach (var link in _contentNodes.Values)
{
var node = link.Value;
if (node == null) continue;
var contentTypeId = node.ContentType.Id;
if (index.TryGetValue(contentTypeId, out var contentType) == false) continue;
SetValueLocked(_contentNodes, node.Id, new ContentNode(node, contentType));
}
SetValueLocked(_contentTypesById, type.Id, type);
SetValueLocked(_contentTypesByAlias, type.Alias, type);
}
finally
foreach (var link in _contentNodes.Values)
{
Release(lockInfo);
var node = link.Value;
if (node == null) continue;
var contentTypeId = node.ContentType.Id;
if (index.TryGetValue(contentTypeId, out var contentType) == false) continue;
SetValueLocked(_contentNodes, node.Id, new ContentNode(node, contentType));
}
}
@@ -1256,7 +1248,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
// else we need to try to create a new gen ref
// whether we are wlocked or not, noone can rlock while we do,
// so _liveGen and _nextGen are safe
if (_wlocked > 0) // volatile, cannot ++ but could --
if (Monitor.IsEntered(_wlocko))
{
// write-locked, cannot use latest gen (at least 1) so use previous
var snapGen = _nextGen ? _liveGen - 1 : _liveGen;

14
src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs
View File

@@ -622,7 +622,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
.Where(x => x.RootContentId.HasValue && x.LanguageIsoCode.IsNullOrWhiteSpace() == false)
.Select(x => new Domain(x.Id, x.DomainName, x.RootContentId.Value, CultureInfo.GetCultureInfo(x.LanguageIsoCode), x.IsWildcard)))
{
_domainStore.Set(domain.Id, domain);
_domainStore.SetLocked(domain.Id, domain);
}
}
@@ -980,7 +980,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
break;
case DomainChangeTypes.Remove:
_domainStore.Clear(payload.Id);
_domainStore.ClearLocked(payload.Id);
break;
case DomainChangeTypes.Refresh:
var domain = _serviceContext.DomainService.GetById(payload.Id);
@@ -988,7 +988,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
if (domain.RootContentId.HasValue == false) continue; // anomaly
if (domain.LanguageIsoCode.IsNullOrWhiteSpace()) continue; // anomaly
var culture = CultureInfo.GetCultureInfo(domain.LanguageIsoCode);
_domainStore.Set(domain.Id, new Domain(domain.Id, domain.DomainName, domain.RootContentId.Value, culture, domain.IsWildcard));
_domainStore.SetLocked(domain.Id, new Domain(domain.Id, domain.DomainName, domain.RootContentId.Value, culture, domain.IsWildcard));
break;
}
}
@@ -1075,9 +1075,9 @@ namespace Umbraco.Web.PublishedCache.NuCache
_contentStore.UpdateContentTypes(removedIds, typesA, kits);
if (!otherIds.IsCollectionEmpty())
_contentStore.UpdateContentTypes(CreateContentTypes(PublishedItemType.Content, otherIds.ToArray()));
_contentStore.UpdateContentTypesLocked(CreateContentTypes(PublishedItemType.Content, otherIds.ToArray()));
if (!newIds.IsCollectionEmpty())
_contentStore.NewContentTypes(CreateContentTypes(PublishedItemType.Content, newIds.ToArray()));
_contentStore.NewContentTypesLocked(CreateContentTypes(PublishedItemType.Content, newIds.ToArray()));
scope.Complete();
}
}
@@ -1106,9 +1106,9 @@ namespace Umbraco.Web.PublishedCache.NuCache
_mediaStore.UpdateContentTypes(removedIds, typesA, kits);
if (!otherIds.IsCollectionEmpty())
_mediaStore.UpdateContentTypes(CreateContentTypes(PublishedItemType.Media, otherIds.ToArray()).ToArray());
_mediaStore.UpdateContentTypesLocked(CreateContentTypes(PublishedItemType.Media, otherIds.ToArray()).ToArray());
if (!newIds.IsCollectionEmpty())
_mediaStore.NewContentTypes(CreateContentTypes(PublishedItemType.Media, newIds.ToArray()).ToArray());
_mediaStore.NewContentTypesLocked(CreateContentTypes(PublishedItemType.Media, newIds.ToArray()).ToArray());
scope.Complete();
}
}

154
src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs
View File

@@ -35,7 +35,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
private long _liveGen, _floorGen;
private bool _nextGen, _collectAuto;
private Task _collectTask;
private volatile int _wlocked;
// minGenDelta to be adjusted
// we may want to throttle collects even if delta is reached
@@ -84,7 +83,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
private class WriteLockInfo
{
public bool Taken;
public bool Count;
}
// a scope contextual that represents a locked writer to the dictionary
@@ -92,8 +90,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
{
private readonly WriteLockInfo _lockinfo = new WriteLockInfo();
private readonly SnapDictionary<TKey, TValue> _dictionary;
private int _released;
public ScopedWriteLock(SnapDictionary<TKey, TValue> dictionary, bool scoped)
{
_dictionary = dictionary;
@@ -102,8 +99,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Release(bool completed)
{
if (Interlocked.CompareExchange(ref _released, 1, 0) != 0)
return;
_dictionary.Release(_lockinfo, completed);
}
}
@@ -117,8 +112,17 @@ namespace Umbraco.Web.PublishedCache.NuCache
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped));
}
private void EnsureLocked()
{
if (!Monitor.IsEntered(_wlocko))
throw new InvalidOperationException("Write lock must be acquried.");
}
private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
if (Monitor.IsEntered(_wlocko))
throw new InvalidOperationException("Recursive locks not allowed");
Monitor.Enter(_wlocko, ref lockInfo.Taken);
lock(_rlocko)
@@ -132,11 +136,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
try { }
finally
{
// increment the lock count, and register that this lock is counting
_wlocked++;
lockInfo.Count = true;
if (_nextGen == false || (forceGen && _wlocked == 1))
if (_nextGen == false || (forceGen))
{
// because we are changing things, a new generation
// is created, which will trigger a new snapshot
@@ -181,8 +181,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
// decrement the lock count, if counting, then exit the lock
if (lockInfo.Count) _wlocked--;
// TODO: Shouldn't this be in a finally block?
Monitor.Exit(_wlocko);
}
@@ -198,75 +197,59 @@ namespace Umbraco.Web.PublishedCache.NuCache
return link;
}
public void Set(TKey key, TValue value)
public void SetLocked(TKey key, TValue value)
{
var lockInfo = new WriteLockInfo();
try
{
Lock(lockInfo);
EnsureLocked();
// this is safe only because we're write-locked
var link = GetHead(key);
if (link != null)
// this is safe only because we're write-locked
var link = GetHead(key);
if (link != null)
{
// already in the dict
if (link.Gen != _liveGen)
{
// already in the dict
if (link.Gen != _liveGen)
{
// for an older gen - if value is different then insert a new
// link for the new gen, with the new value
if (link.Value != value)
_items.TryUpdate(key, new LinkedNode<TValue>(value, _liveGen, link), link);
}
else
{
// for the live gen - we can fix the live gen - and remove it
// if value is null and there's no next gen
if (value == null && link.Next == null)
_items.TryRemove(key, out link);
else
link.Value = value;
}
// for an older gen - if value is different then insert a new
// link for the new gen, with the new value
if (link.Value != value)
_items.TryUpdate(key, new LinkedNode<TValue>(value, _liveGen, link), link);
}
else
{
_items.TryAdd(key, new LinkedNode<TValue>(value, _liveGen));
}
}
finally
{
Release(lockInfo);
}
}
public void Clear(TKey key)
{
Set(key, null);
}
public void Clear()
{
var lockInfo = new WriteLockInfo();
try
{
Lock(lockInfo);
// this is safe only because we're write-locked
foreach (var kvp in _items.Where(x => x.Value != null))
{
if (kvp.Value.Gen < _liveGen)
{
var link = new LinkedNode<TValue>(null, _liveGen, kvp.Value);
_items.TryUpdate(kvp.Key, link, kvp.Value);
}
// for the live gen - we can fix the live gen - and remove it
// if value is null and there's no next gen
if (value == null && link.Next == null)
_items.TryRemove(key, out link);
else
{
kvp.Value.Value = null;
}
link.Value = value;
}
}
finally
else
{
Release(lockInfo);
_items.TryAdd(key, new LinkedNode<TValue>(value, _liveGen));
}
}
public void ClearLocked(TKey key)
{
SetLocked(key, null);
}
public void ClearLocked()
{
EnsureLocked();
// this is safe only because we're write-locked
foreach (var kvp in _items.Where(x => x.Value != null))
{
if (kvp.Value.Gen < _liveGen)
{
var link = new LinkedNode<TValue>(null, _liveGen, kvp.Value);
_items.TryUpdate(kvp.Key, link, kvp.Value);
}
else
{
kvp.Value.Value = null;
}
}
}
@@ -336,7 +319,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
// else we need to try to create a new gen object
// whether we are wlocked or not, noone can rlock while we do,
// so _liveGen and _nextGen are safe
if (_wlocked > 0) // volatile, cannot ++ but could --
if (Monitor.IsEntered(_wlocko))
{
// write-locked, cannot use latest gen (at least 1) so use previous
var snapGen = _nextGen ? _liveGen - 1 : _liveGen;
@@ -468,17 +451,18 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
public /*async*/ Task PendingCollect()
{
Task task;
lock (_rlocko)
{
task = _collectTask;
}
return task ?? Task.CompletedTask;
//if (task != null)
// await task;
}
// TODO: This is never used? Should it be? Maybe move to TestHelper below?
//public /*async*/ Task PendingCollect()
//{
// Task task;
// lock (_rlocko)
// {
// task = _collectTask;
// }
// return task ?? Task.CompletedTask;
// //if (task != null)
// // await task;
//}
public long GenCount => _genObjs.Count;
@@ -503,7 +487,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public long LiveGen => _dict._liveGen;
public long FloorGen => _dict._floorGen;
public bool NextGen => _dict._nextGen;
public int WLocked => _dict._wlocked;
public bool IsLocked => Monitor.IsEntered(_dict._wlocko);
public bool CollectAuto
{