diff --git a/src/Umbraco.Core/Scoping/Scope.cs b/src/Umbraco.Core/Scoping/Scope.cs index f6ec36125d..880db57a2b 100644 --- a/src/Umbraco.Core/Scoping/Scope.cs +++ b/src/Umbraco.Core/Scoping/Scope.cs @@ -37,6 +37,8 @@ namespace Umbraco.Core.Scoping private IEventDispatcher _eventDispatcher; private object _dictionaryLocker; + private readonly HashSet _readLocks; + private readonly HashSet _writeLocks; // ReadLocks and WriteLocks if we're the outer most scope it's those owned by the entire chain // If we're a child scope it's those that we have requested. @@ -68,8 +70,10 @@ namespace Umbraco.Core.Scoping Detachable = detachable; _dictionaryLocker = new object(); - ReadLocks = new Dictionary>(); WriteLocks = new Dictionary>(); + ReadLocks = new Dictionary>(); + _writeLocks = new HashSet(); + _readLocks = new HashSet(); #if DEBUG_SCOPES _scopeProvider.RegisterScope(this); @@ -370,8 +374,7 @@ namespace Umbraco.Core.Scoping { // We're the parent scope, make sure that locks of all scopes has been cleared // Since we're only reading we don't have to be in a lock - if (ReadLocks.Values.Any(x => x.Values.Any(value => value != 0)) - || WriteLocks.Values.Any(x => x.Values.Any(value => value != 0))) + if (ReadLocks.Any() || WriteLocks.Any()) { throw new InvalidOperationException($"All scopes has not been disposed from parent scope: {InstanceId}"); } @@ -580,13 +583,10 @@ namespace Umbraco.Core.Scoping { lock (_dictionaryLocker) { - // Reset all values to 0 since the scope is being disposed + // Remove the scope from the dictionary because it's getting disposed. if (ReadLocks.ContainsKey(instanceId)) { - foreach (var key in ReadLocks[instanceId].Keys.ToList()) - { - ReadLocks[instanceId][key] = 0; - } + ReadLocks.Remove(instanceId); } } } @@ -606,13 +606,10 @@ namespace Umbraco.Core.Scoping { lock (_dictionaryLocker) { - // Reset all values to 0 since the scope is being disposed + // Remove the scope from the dictionary because it's getting disposed. if (WriteLocks.ContainsKey(instanceID)) { - foreach (var key in WriteLocks[instanceID].Keys.ToList()) - { - WriteLocks[instanceID][key] = 0; - } + WriteLocks.Remove(instanceID); } } @@ -643,28 +640,6 @@ namespace Umbraco.Core.Scoping WriteLockInner(InstanceId, timeout, lockId); } - /// - /// Determine if a read lock with the specified ID has already been obtained. - /// - /// Id to test. - /// True if no scopes has obtained a read lock with the specific ID yet. - private bool HasReadLock(int lockId) - { - // Check if there is any dictionary with a key equal to lockId - // And check that the value associated with that key is greater than 0, if not it could be because a lock was requested but it failed. - return ReadLocks.Values.Any(x => x.ContainsKey(lockId)); - } - - /// - /// Determine if a write lock with the specified ID has already been obtained. - /// - /// Id to test - /// >True if no scopes has obtained a write lock with the specific ID yet. - private bool HasWriteLock(int lockId) - { - return WriteLocks.Values.Any(x => x.ContainsKey(lockId)); - } - /// /// Handles acquiring a read lock, will delegate it to the parent if there are any. /// @@ -685,9 +660,10 @@ namespace Umbraco.Core.Scoping foreach (var lockId in lockIds) { // Only acquire the lock if we haven't done so yet. - if (!HasReadLock(lockId)) + if (!_readLocks.Contains(lockId)) { IncrementReadLock(lockId, instanceId); + _readLocks.Add(lockId); try { if (timeout is null) @@ -707,6 +683,7 @@ namespace Umbraco.Core.Scoping // Since we at this point have determined that we haven't got any key of LockID, it's safe to completely remove it instead of decrementing. // It needs to be completely removed, because that's how we determine to acquire a lock. ReadLocks[instanceId].Remove(lockId); + _readLocks.Remove(lockId); throw; } } @@ -738,9 +715,10 @@ namespace Umbraco.Core.Scoping foreach (var lockId in lockIds) { // Only acquire lock if we haven't yet (WriteLocks not containing the key) - if (!HasWriteLock(lockId)) + if (!_writeLocks.Contains(lockId)) { IncrementWriteLock(lockId, instanceId); + _writeLocks.Add(lockId); try { if (timeout is null) @@ -758,6 +736,7 @@ namespace Umbraco.Core.Scoping // Since we at this point have determined that we haven't got any key of LockID, it's safe to completely remove it instead of decrementing. // It needs to be completely removed, because that's how we determine to acquire a lock. WriteLocks[instanceId].Remove(lockId); + _writeLocks.Remove(lockId); throw; } } diff --git a/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs b/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs index 789cbeff88..2a4f72f9a1 100644 --- a/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs @@ -254,8 +254,7 @@ namespace Umbraco.Tests.Scoping innerScope.Complete(); } Assert.AreEqual(2, realOuterScope.WriteLocks[realOuterScope.InstanceId][Constants.Locks.ContentTree]); - Assert.AreEqual(0, realOuterScope.WriteLocks[innerscopeId][Constants.Locks.Languages]); - Assert.AreEqual(0, realOuterScope.WriteLocks[innerscopeId][Constants.Locks.ContentTree]); + Assert.IsFalse(realOuterScope.WriteLocks.ContainsKey(innerscopeId)); outerscope.Complete(); } } @@ -287,8 +286,7 @@ namespace Umbraco.Tests.Scoping innerScope.Complete(); } Assert.AreEqual(2, realOuterScope.ReadLocks[outerscope.InstanceId][Constants.Locks.ContentTree]); - Assert.AreEqual(0, realOuterScope.ReadLocks[innerscopeId][Constants.Locks.Languages]); - Assert.AreEqual(0, realOuterScope.ReadLocks[innerscopeId][Constants.Locks.ContentTree]); + Assert.IsFalse(realOuterScope.ReadLocks.ContainsKey(innerscopeId)); outerscope.Complete(); @@ -344,18 +342,14 @@ namespace Umbraco.Tests.Scoping Assert.AreEqual(1, realParentScope.WriteLocks[innerScope1.InstanceId][Constants.Locks.ContentTree], $"innerScope1, innerScope1 instance, after innserScope2 disposed: {nameof(Constants.Locks.ContentTree)}"); Assert.AreEqual(1, realParentScope.WriteLocks[innerScope1.InstanceId][Constants.Locks.ContentTypes], $"innerScope1, innerScope1 instance, after innserScope2 disposed: {nameof(Constants.Locks.ContentTypes)}"); Assert.AreEqual(1, realParentScope.WriteLocks[innerScope1.InstanceId][Constants.Locks.Languages], $"innerScope1, innerScope1 instance, after innserScope2 disposed: {nameof(Constants.Locks.Languages)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope2Id][Constants.Locks.ContentTree], $"innerScope1, innerScope2 instance, after innserScope2 disposed: {nameof(Constants.Locks.ContentTree)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope2Id][Constants.Locks.MediaTypes], $"innerScope1, innerScope2 instance, after innserScope2 disposed: {nameof(Constants.Locks.MediaTypes)}"); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); innerScope1.Complete(); } Assert.AreEqual(1, realParentScope.WriteLocks[realParentScope.InstanceId][Constants.Locks.ContentTree], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.ContentTree)}"); Assert.AreEqual(1, realParentScope.WriteLocks[realParentScope.InstanceId][Constants.Locks.ContentTypes], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.ContentTypes)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope2Id][Constants.Locks.ContentTree], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.ContentTree)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope2Id][Constants.Locks.MediaTypes], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.MediaTypes)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope1Id][Constants.Locks.ContentTree], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.ContentTree)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope1Id][Constants.Locks.ContentTypes], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.ContentTypes)}"); - Assert.AreEqual(0, realParentScope.WriteLocks[innerScope1Id][Constants.Locks.Languages], $"parentScope after inner scopes disposed: {nameof(Constants.Locks.Languages)}"); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope1Id)); parentScope.Complete(); } @@ -408,17 +402,15 @@ namespace Umbraco.Tests.Scoping Assert.AreEqual(1, realParentScope.ReadLocks[innserScope1.InstanceId][Constants.Locks.ContentTree], $"innerScope1, innerScope1 instance, after innerScope2 disposed: {nameof(Constants.Locks.ContentTree)}"); Assert.AreEqual(1, realParentScope.ReadLocks[innserScope1.InstanceId][Constants.Locks.ContentTypes], $"innerScope1, innerScope1 instance, after innerScope2 disposed: {nameof(Constants.Locks.ContentTypes)}"); Assert.AreEqual(1, realParentScope.ReadLocks[innserScope1.InstanceId][Constants.Locks.Languages], $"innerScope1, innerScope1 instance, after innerScope2 disposed: {nameof(Constants.Locks.Languages)}"); - Assert.AreEqual(0, realParentScope.ReadLocks[innerScope2Id][Constants.Locks.ContentTree]); - Assert.AreEqual(0, realParentScope.ReadLocks[innerScope2Id][Constants.Locks.MediaTypes]); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); innserScope1.Complete(); } Assert.AreEqual(1, realParentScope.ReadLocks[realParentScope.InstanceId][Constants.Locks.ContentTree], $"parentScope after innerScope1 disposed: {nameof(Constants.Locks.ContentTree)}"); Assert.AreEqual(1, realParentScope.ReadLocks[realParentScope.InstanceId][Constants.Locks.ContentTypes], $"parentScope after innerScope1 disposed: {nameof(Constants.Locks.ContentTypes)}"); - Assert.AreEqual(0, realParentScope.ReadLocks[innerScope1Id][Constants.Locks.ContentTree]); - Assert.AreEqual(0, realParentScope.ReadLocks[innerScope1Id][Constants.Locks.ContentTypes]); - Assert.AreEqual(0, realParentScope.ReadLocks[innerScope1Id][Constants.Locks.Languages]); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); + Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope1Id)); parentScope.Complete(); }