Fixing locking issues for document type saves.

src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs

@@ -134,9 +134,10 @@ private void ObtainReadLock()
 
             const string query = "SELECT value FROM umbracoLock WITH (REPEATABLEREAD)  WHERE id=@id";
 
-            db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";");
+            var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}";
 
-            var i = db.ExecuteScalar<int?>(query, new { id = LockId });
+            // execute the lock timeout query and the actual query in a single server roundtrip
+            var i = db.ExecuteScalar<int?>($"{lockTimeoutQuery};{query}", new { id = LockId });
 
             if (i == null)
             {
@@ -169,9 +170,10 @@ private void ObtainWriteLock()
             const string query =
                 @"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id";
 
-            db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";");
+            var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}";
 
-            var i = db.Execute(query, new { id = LockId });
+            // execute the lock timeout query and the actual query in a single server roundtrip
+            var i = db.Execute($"{lockTimeoutQuery};{query}", new { id = LockId });
 
             if (i == 0)
             {

src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs

@@ -154,7 +154,7 @@ private void ObtainWriteLock()
 
             try
             {
-                var i = command.ExecuteNonQuery();
+                var i = db.ExecuteNonQuery(command);
 
                 if (i == 0)
                 {

src/Umbraco.Core/Cache/ObjectCacheAppCache.cs

@@ -9,6 +9,9 @@ namespace Umbraco.Cms.Core.Cache;
 /// </summary>
 public class ObjectCacheAppCache : IAppPolicyCache, IDisposable
 {
+    private static readonly TimeSpan _readLockTimeout = TimeSpan.FromSeconds(5);
+    private static readonly TimeSpan _writeLockTimeout = TimeSpan.FromSeconds(5);
+
     private readonly ReaderWriterLockSlim _locker = new(LockRecursionPolicy.SupportsRecursion);
     private bool _disposedValue;
 
@@ -33,7 +36,10 @@ public ObjectCacheAppCache() =>
         Lazy<object?>? result;
         try
         {
-            _locker.EnterReadLock();
+            if (_locker.TryEnterReadLock(_readLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when getting item");
+            }
             result = MemoryCache.Get(key) as Lazy<object?>; // null if key not found
         }
         finally
@@ -195,7 +201,10 @@ public virtual void Clear(string key)
     {
         try
         {
-            _locker.EnterWriteLock();
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when clearing item");
+            }
             if (MemoryCache[key] == null)
             {
                 return;
@@ -223,8 +232,10 @@ public virtual void ClearOfType(Type type)
         var isInterface = type.IsInterface;
         try
         {
-            _locker.EnterWriteLock();
-
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when clearing by type");
+            }
             // ToArray required to remove
             foreach (var key in MemoryCache
                          .Where(x =>
@@ -259,7 +270,10 @@ public virtual void ClearOfType<T>()
     {
         try
         {
-            _locker.EnterWriteLock();
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when clearing by generic type");
+            }
             Type typeOfT = typeof(T);
             var isInterface = typeOfT.IsInterface;
 
@@ -296,7 +310,10 @@ public virtual void ClearOfType<T>(Func<string, T, bool> predicate)
     {
         try
         {
-            _locker.EnterWriteLock();
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when clearing generic type with predicate");
+            }
             Type typeOfT = typeof(T);
             var isInterface = typeOfT.IsInterface;
 
@@ -338,7 +355,10 @@ public virtual void ClearByKey(string keyStartsWith)
     {
         try
         {
-            _locker.EnterWriteLock();
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cache when clearing with prefix");
+            }
 
             // ToArray required to remove
             foreach (var key in MemoryCache
@@ -365,7 +385,10 @@ public virtual void ClearByRegex(string regex)
 
         try
         {
-            _locker.EnterWriteLock();
+            if (_locker.TryEnterWriteLock(_writeLockTimeout) is false)
+            {
+                throw new TimeoutException("Timeout exceeded to the memory cach when clearing by regex");
+            }
 
             // ToArray required to remove
             foreach (var key in MemoryCache

src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs

@@ -1,3 +1,4 @@
+using System.Data.Common;
 using NPoco;
 using Umbraco.Cms.Infrastructure.Migrations.Install;
 
@@ -33,4 +34,7 @@ public interface IUmbracoDatabase : IDatabase
     bool IsUmbracoInstalled();
 
     DatabaseSchemaResult ValidateSchema();
+
+    /// <returns>The number of rows affected.</returns>
+    int ExecuteNonQuery(DbCommand command) => command.ExecuteNonQuery();
 }

src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs

@@ -224,6 +224,14 @@ public DatabaseSchemaResult ValidateSchema()
         return databaseSchemaValidationResult ?? new DatabaseSchemaResult();
     }
 
+    public int ExecuteNonQuery(DbCommand command)
+    {
+        OnExecutingCommand(command);
+        var i = command.ExecuteNonQuery();
+        OnExecutedCommand(command);
+        return i;
+    }
+
     /// <summary>
     ///     Returns true if Umbraco database tables are detected to be installed
     /// </summary>

src/Umbraco.Infrastructure/Scoping/Scope.cs

@@ -23,10 +23,9 @@ internal class Scope : ICoreScope, IScope, Core.Scoping.IScope
         private readonly bool _autoComplete;
         private readonly CoreDebugSettings _coreDebugSettings;
 
-        private readonly object _dictionaryLocker;
         private readonly IEventAggregator _eventAggregator;
         private readonly IsolationLevel _isolationLevel;
-        private readonly object _lockQueueLocker = new();
+        private readonly object _locker = new();
         private readonly ILogger<Scope> _logger;
         private readonly MediaFileManager _mediaFileManager;
         private readonly RepositoryCacheMode _repositoryCacheMode;
@@ -87,7 +86,6 @@ private Scope(
             _scopeFileSystem = scopeFileSystems;
             _autoComplete = autoComplete;
             Detachable = detachable;
-            _dictionaryLocker = new object();
 
 #if DEBUG_SCOPES
             _scopeProvider.RegisterScope(this);
@@ -562,7 +560,7 @@ public void Dispose()
                 DisposeLastScope();
             }
 
-            lock (_lockQueueLocker)
+            lock (_locker)
             {
                 _queuedLocks?.Clear();
             }
@@ -573,24 +571,24 @@ public void Dispose()
         public void EagerReadLock(params int[] lockIds) => EagerReadLockInner(InstanceId, null, lockIds);
 
         /// <inheritdoc />
-        public void ReadLock(params int[] lockIds) => LazyReadLockInner(InstanceId, lockIds);
+        public void ReadLock(params int[] lockIds) => EagerReadLockInner(InstanceId, null, lockIds);
 
         public void EagerReadLock(TimeSpan timeout, int lockId) =>
             EagerReadLockInner(InstanceId, timeout, lockId);
 
         /// <inheritdoc />
-        public void ReadLock(TimeSpan timeout, int lockId) => LazyReadLockInner(InstanceId, timeout, lockId);
+        public void ReadLock(TimeSpan timeout, int lockId) => EagerReadLockInner(InstanceId, timeout, lockId);
 
         public void EagerWriteLock(params int[] lockIds) => EagerWriteLockInner(InstanceId, null, lockIds);
 
         /// <inheritdoc />
-        public void WriteLock(params int[] lockIds) => LazyWriteLockInner(InstanceId, lockIds);
+        public void WriteLock(params int[] lockIds) => EagerWriteLockInner(InstanceId, null, lockIds);
 
         public void EagerWriteLock(TimeSpan timeout, int lockId) =>
             EagerWriteLockInner(InstanceId, timeout, lockId);
 
         /// <inheritdoc />
-        public void WriteLock(TimeSpan timeout, int lockId) => LazyWriteLockInner(InstanceId, timeout, lockId);
+        public void WriteLock(TimeSpan timeout, int lockId) => EagerWriteLockInner(InstanceId, timeout, lockId);
 
         /// <summary>
         ///     Used for testing. Ensures and gets any queued read locks.
@@ -659,7 +657,7 @@ private void EnsureDbLocks()
             }
             else
             {
-                lock (_lockQueueLocker)
+                lock (_locker)
                 {
                     if (_queuedLocks?.Count > 0)
                     {
@@ -970,7 +968,7 @@ private void ClearLocks(Guid instanceId)
             }
             else
             {
-                lock (_dictionaryLocker)
+                lock (_locker)
                 {
                     _readLocksDictionary?.Remove(instanceId);
                     _writeLocksDictionary?.Remove(instanceId);
@@ -1045,7 +1043,7 @@ public void LazyWriteLockInner(Guid instanceId, TimeSpan timeout, int lockId)
 
         private void LazyLockInner(DistributedLockType lockType, Guid instanceId, params int[] lockIds)
         {
-            lock (_lockQueueLocker)
+            lock (_locker)
             {
                 if (_queuedLocks == null)
                 {
@@ -1061,7 +1059,7 @@ private void LazyLockInner(DistributedLockType lockType, Guid instanceId, params
 
         private void LazyLockInner(DistributedLockType lockType, Guid instanceId, TimeSpan timeout, int lockId)
         {
-            lock (_lockQueueLocker)
+            lock (_locker)
             {
                 if (_queuedLocks == null)
                 {
@@ -1088,7 +1086,7 @@ private void EagerReadLockInner(Guid instanceId, TimeSpan? timeout, params int[]
             }
             else
             {
-                lock (_dictionaryLocker)
+                lock (_locker)
                 {
                     foreach (var lockId in lockIds)
                     {
@@ -1122,7 +1120,7 @@ private void EagerWriteLockInner(Guid instanceId, TimeSpan? timeout, params int[
             }
             else
             {
-                lock (_dictionaryLocker)
+                lock (_locker)
                 {
                     foreach (var lockId in lockIds)
                     {

src/Umbraco.PublishedCache.NuCache/ContentStore.cs

@@ -27,6 +27,8 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache;
 /// </remarks>
 public class ContentStore
 {
+    private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30);
+
     // TODO: collection trigger (ok for now)
     // see SnapDictionary notes
     private const long CollectMinGenDelta = 8;
@@ -330,7 +332,12 @@ private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
             throw new InvalidOperationException("Recursive locks not allowed");
         }
 
-        Monitor.Enter(_wlocko, ref lockInfo.Taken);
+        Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken);
+
+        if (Monitor.IsEntered(_wlocko) is false)
+        {
+            throw new TimeoutException("Could not enter monitor before timeout in content store");
+        }
 
         lock (_rlocko)
         {

src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs

@@ -127,9 +127,25 @@ public void Rebuild(
     {
         using (ICoreScope scope = ScopeProvider.CreateCoreScope(repositoryCacheMode: RepositoryCacheMode.Scoped))
         {
-            scope.ReadLock(Constants.Locks.ContentTree);
-            scope.ReadLock(Constants.Locks.MediaTree);
-            scope.ReadLock(Constants.Locks.MemberTree);
+            if (contentTypeIds is null && mediaTypeIds is null && memberTypeIds is null)
+            {
+                scope.ReadLock(Constants.Locks.ContentTree,Constants.Locks.MediaTree,Constants.Locks.MemberTree);
+            }
+
+            if (contentTypeIds is not null && contentTypeIds.Any())
+            {
+                scope.ReadLock(Constants.Locks.ContentTree);
+            }
+
+            if (mediaTypeIds is not null && mediaTypeIds.Any())
+            {
+                scope.ReadLock(Constants.Locks.MediaTree);
+            }
+
+            if (memberTypeIds is not null && memberTypeIds.Any())
+            {
+                scope.ReadLock(Constants.Locks.MemberTree);
+            }
 
             _repository.Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds);
 

src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs

@@ -9,6 +9,8 @@ public class SnapDictionary<TKey, TValue>
     where TValue : class
     where TKey : notnull
 {
+    private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30);
+
     // minGenDelta to be adjusted
     // we may want to throttle collects even if delta is reached
     // we may want to force collect if delta is not reached but very old
@@ -198,7 +200,12 @@ private void Lock(WriteLockInfo lockInfo, bool forceGen = false)
             throw new InvalidOperationException("Recursive locks not allowed");
         }
 
-        Monitor.Enter(_wlocko, ref lockInfo.Taken);
+        Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken);
+
+        if (Monitor.IsEntered(_wlocko) is false)
+        {
+            throw new TimeoutException("Could not enter the monitor before timeout in SnapDictionary");
+        }
 
         lock (_rlocko)
         {