diff --git a/src/Umbraco.Core/MonitorLock.cs b/src/Umbraco.Core/MonitorLock.cs
index 45dbdbbd10..d8885ed256 100644
--- a/src/Umbraco.Core/MonitorLock.cs
+++ b/src/Umbraco.Core/MonitorLock.cs
@@ -4,6 +4,7 @@ namespace Umbraco.Cms.Core;
/// Provides an equivalent to the c# lock statement, to be used in a using block.
///
/// Ie replace lock (o) {...} by using (new MonitorLock(o)) { ... }
+[Obsolete("Use System.Threading.Lock instead. This will be removed in a future version.")]
public class MonitorLock : IDisposable
{
private readonly bool _entered;
diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs
index 0230032dc2..d706301ff8 100644
--- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs
+++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs
@@ -52,9 +52,9 @@ public class ContentStore
// SnapDictionary has unit tests to ensure it all works correctly
// For locking information, see SnapDictionary
private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor;
- private readonly object _rlocko = new();
private readonly IVariationContextAccessor _variationContextAccessor;
- private readonly object _wlocko = new();
+ private readonly object _rlocko = new();
+ private readonly SemaphoreSlim _writeLock = new(1);
private Task? _collectTask;
private GenObj? _genObj;
private long _liveGen;
@@ -319,7 +319,7 @@ public class ContentStore
private void EnsureLocked()
{
- if (!Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount != 0)
{
throw new InvalidOperationException("Write lock must be acquried.");
}
@@ -327,14 +327,16 @@ public class ContentStore
private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
- if (Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount == 0)
{
throw new InvalidOperationException("Recursive locks not allowed");
}
- Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken);
-
- if (Monitor.IsEntered(_wlocko) is false)
+ if (_writeLock.Wait(_monitorTimeout))
+ {
+ lockInfo.Taken = true;
+ }
+ else
{
throw new TimeoutException("Could not enter monitor before timeout in content store");
}
@@ -344,6 +346,7 @@ public class ContentStore
// see SnapDictionary
try
{
+ // Run all code in finally to ensure ThreadAbortException does not interrupt execution
}
finally
{
@@ -374,6 +377,7 @@ public class ContentStore
// see SnapDictionary
try
{
+ // Run all code in finally to ensure ThreadAbortException does not interrupt execution
}
finally
{
@@ -409,7 +413,7 @@ public class ContentStore
{
if (lockInfo.Taken)
{
- Monitor.Exit(_wlocko);
+ _writeLock.Release();
}
}
}
@@ -1817,7 +1821,7 @@ public class ContentStore
// 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 (Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount == 0)
{
// write-locked, cannot use latest gen (at least 1) so use previous
var snapGen = _nextGen ? _liveGen - 1 : _liveGen;
@@ -1829,8 +1833,7 @@ public class ContentStore
}
else if (_genObj.Gen != snapGen)
{
- throw new PanicException(
- $"The generation {_genObj.Gen} does not equal the snapshot generation {snapGen}");
+ throw new PanicException($"The generation {_genObj.Gen} does not equal the snapshot generation {snapGen}");
}
}
else
diff --git a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs
index b6c87e22bb..70bdcfe038 100644
--- a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs
+++ b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs
@@ -28,13 +28,9 @@ public class SnapDictionary
// 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> _items;
private readonly object _rlocko = new();
- private readonly object _wlocko = new();
+ private readonly SemaphoreSlim _writeLock = new(1);
private Task? _collectTask;
private GenObj? _genObj;
private long _liveGen;
@@ -187,7 +183,7 @@ public class SnapDictionary
private void EnsureLocked()
{
- if (!Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount != 0)
{
throw new InvalidOperationException("Write lock must be acquried.");
}
@@ -195,14 +191,16 @@ public class SnapDictionary
private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
{
- if (Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount == 0)
{
throw new InvalidOperationException("Recursive locks not allowed");
}
- Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken);
-
- if (Monitor.IsEntered(_wlocko) is false)
+ if (_writeLock.Wait(_monitorTimeout))
+ {
+ lockInfo.Taken = true;
+ }
+ else
{
throw new TimeoutException("Could not enter the monitor before timeout in SnapDictionary");
}
@@ -217,6 +215,7 @@ public class SnapDictionary
// RuntimeHelpers.PrepareConstrainedRegions();
try
{
+ // Run all code in finally to ensure ThreadAbortException does not interrupt execution
}
finally
{
@@ -244,43 +243,48 @@ public class SnapDictionary
return;
}
- if (commit == false)
+ try
{
- lock (_rlocko)
+ if (commit == false)
{
- try
+ lock (_rlocko)
{
- }
- finally
- {
- // forget about the temp. liveGen
- _nextGen = false;
- _liveGen -= 1;
- }
- }
-
- foreach (KeyValuePair> item in _items)
- {
- LinkedNode? link = item.Value;
- if (link.Gen <= _liveGen)
- {
- continue;
+ try
+ {
+ // Run all code in finally to ensure ThreadAbortException does not interrupt execution
+ }
+ finally
+ {
+ // forget about the temp. liveGen
+ _nextGen = false;
+ _liveGen -= 1;
+ }
}
- TKey key = item.Key;
- if (link.Next == null)
+ foreach (KeyValuePair> item in _items)
{
- _items.TryRemove(key, out link);
- }
- else
- {
- _items.TryUpdate(key, link.Next, link);
+ LinkedNode? link = item.Value;
+ if (link.Gen <= _liveGen)
+ {
+ continue;
+ }
+
+ TKey key = item.Key;
+ if (link.Next == null)
+ {
+ _items.TryRemove(key, out link);
+ }
+ else
+ {
+ _items.TryUpdate(key, link.Next, link);
+ }
}
}
}
-
- // TODO: Shouldn't this be in a finally block?
- Monitor.Exit(_wlocko);
+ finally
+ {
+ _writeLock.Release();
+ }
}
#endregion
@@ -434,7 +438,7 @@ public class SnapDictionary
// 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 (Monitor.IsEntered(_wlocko))
+ if (_writeLock.CurrentCount == 0)
{
// write-locked, cannot use latest gen (at least 1) so use previous
var snapGen = _nextGen ? _liveGen - 1 : _liveGen;
@@ -624,7 +628,7 @@ public class SnapDictionary
public bool NextGen => _dict._nextGen;
- public bool IsLocked => Monitor.IsEntered(_dict._wlocko);
+ public bool IsLocked => _dict._writeLock.CurrentCount == 0;
public bool CollectAuto
{
diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PublishedCache/ContentCacheTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PublishedCache/ContentCacheTests.cs
new file mode 100644
index 0000000000..6507bd6cda
--- /dev/null
+++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PublishedCache/ContentCacheTests.cs
@@ -0,0 +1,77 @@
+using Microsoft.Extensions.Logging;
+using NUnit.Framework;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Hosting;
+using Umbraco.Cms.Core.Models.PublishedContent;
+using Umbraco.Cms.Core.PublishedCache;
+using Umbraco.Cms.Infrastructure.PublishedCache;
+using Umbraco.Cms.Infrastructure.PublishedCache.DataSource;
+using Umbraco.Cms.Tests.Common.Builders;
+using Umbraco.Cms.Tests.Common.Builders.Extensions;
+using Umbraco.Cms.Tests.Common.Testing;
+using Umbraco.Cms.Tests.Integration.Testing;
+
+namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PublishedCache;
+
+[TestFixture]
+[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
+public class ContentCacheTests : UmbracoIntegrationTestWithContent
+{
+ private ContentStore GetContentStore()
+ {
+ var path = Path.Combine(GetRequiredService().LocalTempPath, "NuCache");
+ Directory.CreateDirectory(path);
+
+ var localContentDbPath = Path.Combine(path, "NuCache.Content.db");
+ var localContentDbExists = File.Exists(localContentDbPath);
+ var contentDataSerializer = new ContentDataSerializer(new DictionaryOfPropertyDataSerializer());
+ var localContentDb = BTree.GetTree(localContentDbPath, localContentDbExists, new NuCacheSettings(), contentDataSerializer);
+
+ return new ContentStore(
+ GetRequiredService(),
+ GetRequiredService(),
+ LoggerFactory.CreateLogger(),
+ LoggerFactory,
+ GetRequiredService(), // new NoopPublishedModelFactory
+ localContentDb);
+ }
+
+ private ContentNodeKit CreateContentNodeKit()
+ {
+ var contentData = new ContentDataBuilder()
+ .WithName("Content 1")
+ .WithProperties(new PropertyDataBuilder()
+ .WithPropertyData("welcomeText", "Welcome")
+ .WithPropertyData("welcomeText", "Welcome", "en-US")
+ .WithPropertyData("welcomeText", "Willkommen", "de")
+ .WithPropertyData("welcomeText", "Welkom", "nl")
+ .WithPropertyData("welcomeText2", "Welcome")
+ .WithPropertyData("welcomeText2", "Welcome", "en-US")
+ .WithPropertyData("noprop", "xxx")
+ .Build())
+ .Build();
+
+ return ContentNodeKitBuilder.CreateWithContent(
+ ContentType.Id,
+ 1,
+ "-1,1",
+ draftData: contentData,
+ publishedData: contentData);
+ }
+
+ [Test]
+ public async Task SetLocked()
+ {
+ var contentStore = GetContentStore();
+
+ using (contentStore.GetScopedWriteLock(ScopeProvider))
+ {
+ var contentNodeKit = CreateContentNodeKit();
+
+ contentStore.SetLocked(contentNodeKit);
+
+ // Try running the same operation again in an async task
+ await Task.Run(() => contentStore.SetLocked(contentNodeKit));
+ }
+ }
+}