Skip to content

Commit

Permalink
NuCache: better fixing, cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan committed Mar 26, 2019
1 parent 851c844 commit dfe6f20
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 84 deletions.
7 changes: 1 addition & 6 deletions src/Umbraco.Core/Scoping/ScopeContextualBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Umbraco.Core.Scoping
/// </remarks>
public abstract class ScopeContextualBase : IDisposable
{
private bool _using, _scoped;
private bool _scoped;

/// <summary>
/// Gets a contextual object.
Expand All @@ -38,9 +38,6 @@ public static T Get<T>(IScopeProvider scopeProvider, string key, Func<bool, T> c
() => ctor(true),
(completed, item) => { item.Release(completed); });

// the object can be 'used' only once at a time
if (w._using) throw new InvalidOperationException("panic: used.");
w._using = true;
w._scoped = true;

return w;
Expand All @@ -52,8 +49,6 @@ public static T Get<T>(IScopeProvider scopeProvider, string key, Func<bool, T> c
/// </remarks>
public void Dispose()
{
_using = false;

if (_scoped == false)
Release(true);
}
Expand Down
67 changes: 15 additions & 52 deletions src/Umbraco.Tests/Cache/SnapDictionaryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ public void WriteLockingFirstSnapshot()
Assert.AreEqual(1, d.Test.LiveGen);
Assert.IsTrue(d.Test.NextGen);

using (d.GetWriter(GetScopeProvider()))
using (d.GetScopedWriteLock(GetScopeProvider()))
{
var s1 = d.CreateSnapshot();

Expand Down Expand Up @@ -685,7 +685,7 @@ public void WriteLocking()
Assert.IsFalse(d.Test.NextGen);
Assert.AreEqual("uno", s2.Get(1));

using (d.GetWriter(GetScopeProvider()))
using (d.GetScopedWriteLock(GetScopeProvider()))
{
// gen 3
Assert.AreEqual(2, d.Test.GetValues(1).Length);
Expand Down Expand Up @@ -724,13 +724,13 @@ public void NestedWriteLocking1()

var scopeProvider = GetScopeProvider();

using (var w1 = d.GetWriter(scopeProvider))
using (var w1 = d.GetScopedWriteLock(scopeProvider))
{
Assert.AreEqual(1, t.LiveGen);
Assert.AreEqual(1, t.WLocked);
Assert.IsTrue(t.NextGen);

using (var w2 = d.GetWriter(scopeProvider))
using (var w2 = d.GetScopedWriteLock(scopeProvider))
{
Assert.AreEqual(1, t.LiveGen);
Assert.AreEqual(2, t.WLocked);
Expand Down Expand Up @@ -770,9 +770,9 @@ public void NestedWriteLocking2()
var scopeContext = new ScopeContext();
var scopeProvider = GetScopeProvider(scopeContext);

using (var w1 = d.GetWriter(scopeProvider))
using (var w1 = d.GetScopedWriteLock(scopeProvider))
{
using (var w2 = d.GetWriter(scopeProvider))
using (var w2 = d.GetScopedWriteLock(scopeProvider))
{
Assert.AreSame(w1, w2);

Expand All @@ -794,13 +794,13 @@ public void NestedWriteLocking3()
var scopeProvider1 = GetScopeProvider();
var scopeProvider2 = GetScopeProvider(scopeContext);

using (var w1 = d.GetWriter(scopeProvider1))
using (var w1 = d.GetScopedWriteLock(scopeProvider1))
{
Assert.AreEqual(1, t.LiveGen);
Assert.AreEqual(1, t.WLocked);
Assert.IsTrue(t.NextGen);

using (var w2 = d.GetWriter(scopeProvider2))
using (var w2 = d.GetScopedWriteLock(scopeProvider2))
{
Assert.AreEqual(1, t.LiveGen);
Assert.AreEqual(2, t.WLocked);
Expand Down Expand Up @@ -850,7 +850,7 @@ public void WriteLocking2()

var scopeProvider = GetScopeProvider();

using (d.GetWriter(scopeProvider))
using (d.GetScopedWriteLock(scopeProvider))
{
// gen 3
Assert.AreEqual(2, d.Test.GetValues(1).Length);
Expand Down Expand Up @@ -895,7 +895,7 @@ public void WriteLocking3()

var scopeProvider = GetScopeProvider();

using (d.GetWriter(scopeProvider))
using (d.GetScopedWriteLock(scopeProvider))
{
// creating a snapshot in a write-lock does NOT return the "current" content
// it uses the previous snapshot, so new snapshot created only on release
Expand Down Expand Up @@ -935,7 +935,7 @@ public void ScopeLocking1()
var scopeContext = new ScopeContext();
var scopeProvider = GetScopeProvider(scopeContext);

using (d.GetWriter(scopeProvider))
using (d.GetScopedWriteLock(scopeProvider))
{
// creating a snapshot in a write-lock does NOT return the "current" content
// it uses the previous snapshot, so new snapshot created only on release
Expand Down Expand Up @@ -985,7 +985,7 @@ public void ScopeLocking2()
var scopeContext = new ScopeContext();
var scopeProvider = GetScopeProvider(scopeContext);

using (d.GetWriter(scopeProvider))
using (d.GetScopedWriteLock(scopeProvider))
{
// creating a snapshot in a write-lock does NOT return the "current" content
// it uses the previous snapshot, so new snapshot created only on release
Expand Down Expand Up @@ -1015,45 +1015,8 @@ public void ScopeLocking2()
Assert.AreEqual(2, s4.Gen);
Assert.AreEqual("uno", s4.Get(1));

// fixme - remove debugging code
/*
Exception caught = null;
var genFlip = 0;
var lckFlip = 0;
var thread = new System.Threading.Thread(() =>
{
try
{
for (var i = 0; i < 20; i++)
{
if (t.LiveGen == 2 && genFlip == 0) genFlip = i; // flips at 1
if (t.WLocked == 0 && lckFlip == 0) lckFlip = i; // flips at 10 ie 5s, as expected
d.CreateSnapshot();
System.Threading.Thread.Sleep(500);
}
}
catch (Exception e)
{
caught = e;
}
});
thread.Start();
*/

scopeContext.ScopeExit(false);

// fixme - remove debugging code
/*
thread.Join();
Assert.IsNull(caught); // but then how can it be not null?
Console.WriteLine(genFlip);
Console.WriteLine(lckFlip);
Assert.AreEqual(1, genFlip);
Assert.AreEqual(10, lckFlip);
*/

// now things have changed
Assert.AreEqual(2, t.LiveGen);
Assert.IsFalse(t.NextGen);
Expand Down Expand Up @@ -1104,13 +1067,13 @@ public void DontPanic()
var d = new SnapDictionary<int, string>();
d.Test.CollectAuto = false;

Assert.IsNull(d.Test.GenObj); // set with first snapshot or first lock, then never null
Assert.IsNull(d.Test.GenObj);

// gen 1
d.Set(1, "one");
Assert.IsTrue(d.Test.NextGen);
Assert.AreEqual(1, d.Test.LiveGen);
Assert.IsNotNull(d.Test.GenObj); // set with lock
Assert.IsNull(d.Test.GenObj);

var s1 = d.CreateSnapshot();
Assert.IsFalse(d.Test.NextGen);
Expand All @@ -1134,7 +1097,7 @@ public void DontPanic()
// writer is scope contextual and scoped
// when disposed, nothing happens
// when the context exists, the writer is released
using (d.GetWriter(scopeProvider))
using (d.GetScopedWriteLock(scopeProvider))
{
d.Set(1, "ein");
Assert.IsTrue(d.Test.NextGen);
Expand Down
13 changes: 7 additions & 6 deletions src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ private class WriteLockInfo
}

// a scope contextual that represents a locked writer to the dictionary
private class ContentStoreWriter : ScopeContextualBase
private class ScopedWriteLock : ScopeContextualBase
{
private readonly WriteLockInfo _lockinfo = new WriteLockInfo();
private readonly ContentStore _store;
private int _released;

public ContentStoreWriter(ContentStore store, bool scoped)
public ScopedWriteLock(ContentStore store, bool scoped)
{
_store = store;
store.Lock(_lockinfo, scoped);
Expand All @@ -114,9 +114,9 @@ public override void Release(bool completed)

// gets a scope contextual representing a locked writer to the dictionary
// TODO: GetScopedWriter? should the dict have a ref onto the scope provider?
public IDisposable GetWriter(IScopeProvider scopeProvider)
public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider)
{
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ContentStoreWriter(this, scoped));
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped));
}

private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
Expand All @@ -137,9 +137,10 @@ private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
// because we are changing things, a new generation
// is created, which will trigger a new snapshot
_nextGen = true;
_genObjs.Enqueue(_genObj = new GenObj(_liveGen));
if (_nextGen)
_genObjs.Enqueue(_genObj = new GenObj(_liveGen));
_liveGen += 1;
_nextGen = true;
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ private void LockAndLoadContent(Action<IScope> action)
// first get a writer, then a scope
// if there already is a scope, the writer will attach to it
// otherwise, it will only exist here - cheap
using (_contentStore.GetWriter(_scopeProvider))
using (_contentStore.GetScopedWriteLock(_scopeProvider))
using (var scope = _scopeProvider.CreateScope())
{
scope.ReadLock(Constants.Locks.ContentTree);
Expand Down Expand Up @@ -401,7 +401,7 @@ private void LoadContentFromLocalDbLocked(IScope scope)
private void LockAndLoadMedia(Action<IScope> action)
{
// see note in LockAndLoadContent
using (_mediaStore.GetWriter(_scopeProvider))
using (_mediaStore.GetScopedWriteLock(_scopeProvider))
using (var scope = _scopeProvider.CreateScope())
{
scope.ReadLock(Constants.Locks.MediaTree);
Expand Down Expand Up @@ -529,7 +529,7 @@ private void LoadMediaFromLocalDbLocked(IScope scope)
private void LockAndLoadDomains()
{
// see note in LockAndLoadContent
using (_domainStore.GetWriter(_scopeProvider))
using (_domainStore.GetScopedWriteLock(_scopeProvider))
using (var scope = _scopeProvider.CreateScope())
{
scope.ReadLock(Constants.Locks.Domains);
Expand Down Expand Up @@ -586,7 +586,7 @@ public override void Notify(ContentCacheRefresher.JsonPayload[] payloads, out bo
return;
}

using (_contentStore.GetWriter(_scopeProvider))
using (_contentStore.GetScopedWriteLock(_scopeProvider))
{
NotifyLocked(payloads, out bool draftChanged2, out bool publishedChanged2);
draftChanged = draftChanged2;
Expand Down Expand Up @@ -682,7 +682,7 @@ public override void Notify(MediaCacheRefresher.JsonPayload[] payloads, out bool
return;
}

using (_mediaStore.GetWriter(_scopeProvider))
using (_mediaStore.GetScopedWriteLock(_scopeProvider))
{
NotifyLocked(payloads, out bool anythingChanged2);
anythingChanged = anythingChanged2;
Expand Down Expand Up @@ -803,7 +803,7 @@ private void Notify<T>(ContentStore store, ContentTypeCacheRefresher.JsonPayload

if (removedIds.Count == 0 && refreshedIds.Count == 0 && otherIds.Count == 0 && newIds.Count == 0) return;

using (store.GetWriter(_scopeProvider))
using (store.GetScopedWriteLock(_scopeProvider))
{
// ReSharper disable AccessToModifiedClosure
action(removedIds, refreshedIds, otherIds, newIds);
Expand All @@ -824,8 +824,8 @@ public override void Notify(DataTypeCacheRefresher.JsonPayload[] payloads)
payload.Removed ? "Removed" : "Refreshed",
payload.Id);

using (_contentStore.GetWriter(_scopeProvider))
using (_mediaStore.GetWriter(_scopeProvider))
using (_contentStore.GetScopedWriteLock(_scopeProvider))
using (_mediaStore.GetScopedWriteLock(_scopeProvider))
{
// TODO: need to add a datatype lock
// this is triggering datatypes reload in the factory, and right after we create some
Expand Down Expand Up @@ -858,7 +858,7 @@ public override void Notify(DomainCacheRefresher.JsonPayload[] payloads)
return;

// see note in LockAndLoadContent
using (_domainStore.GetWriter(_scopeProvider))
using (_domainStore.GetScopedWriteLock(_scopeProvider))
{
foreach (var payload in payloads)
{
Expand Down
18 changes: 7 additions & 11 deletions src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ private class WriteLockInfo
}

// a scope contextual that represents a locked writer to the dictionary
private class SnapDictionaryWriter : ScopeContextualBase
private class ScopedWriteLock : ScopeContextualBase
{
private readonly WriteLockInfo _lockinfo = new WriteLockInfo();
private readonly SnapDictionary<TKey, TValue> _dictionary;
private int _released;

public SnapDictionaryWriter(SnapDictionary<TKey, TValue> dictionary, bool scoped)
public ScopedWriteLock(SnapDictionary<TKey, TValue> dictionary, bool scoped)
{
_dictionary = dictionary;
dictionary.Lock(_lockinfo, scoped);
Expand All @@ -108,14 +108,12 @@ public override void Release(bool completed)
}

// gets a scope contextual representing a locked writer to the dictionary
// fixme GetScopedWriter? should the dict have a ref onto the scope provider?
// fixme this is not a "writer" but a "write lock" => rename GetWriteLock
// the dict is write-locked until the write-lock is released
// which happens when it is disposed (non-scoped)
// or when the scope context exits (scoped)
public IDisposable GetWriter(IScopeProvider scopeProvider)
public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider)
{
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new SnapDictionaryWriter(this, scoped));
return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped));
}

private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
Expand Down Expand Up @@ -143,9 +141,10 @@ private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
// because we are changing things, a new generation
// is created, which will trigger a new snapshot
_nextGen = true; // this is the ONLY place where _nextGen becomes true
_genObjs.Enqueue(_genObj = new GenObj(_liveGen));
if (_nextGen)
_genObjs.Enqueue(_genObj = new GenObj(_liveGen));
_liveGen += 1;
_nextGen = true; // this is the ONLY place where _nextGen becomes true
}
}
}
Expand Down Expand Up @@ -197,9 +196,6 @@ private void Release(WriteLockInfo lockInfo, bool commit = true)
}
}

// fixme - pretend we need to do something that takes time
//System.Threading.Thread.Sleep(TimeSpan.FromSeconds(5));

// decrement the lock count, if counting, then exit the lock
if (lockInfo.Count) _wlocked--;
Monitor.Exit(_wlocko);
Expand Down

0 comments on commit dfe6f20

Please sign in to comment.