diff --git a/src/Umbraco.Core/Scoping/Scope.cs b/src/Umbraco.Core/Scoping/Scope.cs index 880db57a2b..0a3051855a 100644 --- a/src/Umbraco.Core/Scoping/Scope.cs +++ b/src/Umbraco.Core/Scoping/Scope.cs @@ -37,13 +37,13 @@ namespace Umbraco.Core.Scoping private IEventDispatcher _eventDispatcher; private object _dictionaryLocker; - private readonly HashSet _readLocks; - private readonly HashSet _writeLocks; + private HashSet _readLocks; + private 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. - internal readonly Dictionary> ReadLocks; - internal readonly Dictionary> WriteLocks; + internal Dictionary> ReadLocks { get; private set; } + internal Dictionary> WriteLocks { get; private set; } // initializes a new scope private Scope(ScopeProvider scopeProvider, @@ -70,10 +70,6 @@ namespace Umbraco.Core.Scoping Detachable = detachable; _dictionaryLocker = new object(); - WriteLocks = new Dictionary>(); - ReadLocks = new Dictionary>(); - _writeLocks = new HashSet(); - _readLocks = new HashSet(); #if DEBUG_SCOPES _scopeProvider.RegisterScope(this); @@ -374,7 +370,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.Any() || WriteLocks.Any()) + if (ReadLocks is not null && ReadLocks.Any() || WriteLocks is not null && WriteLocks.Any()) { throw new InvalidOperationException($"All scopes has not been disposed from parent scope: {InstanceId}"); } @@ -529,6 +525,9 @@ namespace Umbraco.Core.Scoping private void IncrementWriteLock(int lockId, Guid instanceId) { // Since we've already checked that we're the parent in the WriteLockInner method, we don't need to check again. + // If it's the very first time a lock has been requested the WriteLocks dict hasn't been instantiated yet. + WriteLocks ??= new Dictionary>(); + // Try and get the dict associated with the scope id. var locksDictFound = WriteLocks.TryGetValue(instanceId, out var locksDict); if (locksDictFound) @@ -555,6 +554,9 @@ namespace Umbraco.Core.Scoping private void IncrementReadLock(int lockId, Guid instanceId) { // Since we've already checked that we're the parent in the WriteLockInner method, we don't need to check again. + // If it's the very first time a lock has been requested the ReadLocks dict hasn't been instantiated yet. + ReadLocks ??= new Dictionary>(); + var locksDictFound = ReadLocks.TryGetValue(instanceId, out var locksDict); if (locksDictFound) { @@ -581,6 +583,12 @@ namespace Umbraco.Core.Scoping } else { + if (ReadLocks is null) + { + // If ReadLocks is null, no locks has been requested and we don't have to do anything. + return; + } + lock (_dictionaryLocker) { // Remove the scope from the dictionary because it's getting disposed. @@ -604,6 +612,11 @@ namespace Umbraco.Core.Scoping } else { + if (WriteLocks is null) + { + return; + } + lock (_dictionaryLocker) { // Remove the scope from the dictionary because it's getting disposed. @@ -656,6 +669,7 @@ namespace Umbraco.Core.Scoping lock (_dictionaryLocker) { + _readLocks ??= new HashSet(); // If we are the parent, then handle the lock request. foreach (var lockId in lockIds) { @@ -712,6 +726,7 @@ namespace Umbraco.Core.Scoping lock (_dictionaryLocker) { + _writeLocks ??= new HashSet(); foreach (var lockId in lockIds) { // Only acquire lock if we haven't yet (WriteLocks not containing the key) diff --git a/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs b/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs index 2a4f72f9a1..579c0cc7cf 100644 --- a/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeUnitTests.cs @@ -342,14 +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.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); + Assert.IsFalse(realParentScope.WriteLocks.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.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope2Id)); - Assert.IsFalse(realParentScope.ReadLocks.ContainsKey(innerScope1Id)); + Assert.IsFalse(realParentScope.WriteLocks.ContainsKey(innerScope2Id)); + Assert.IsFalse(realParentScope.WriteLocks.ContainsKey(innerScope1Id)); parentScope.Complete(); } @@ -454,15 +454,23 @@ namespace Umbraco.Tests.Scoping var scopeprovider = GetScopeProvider(out var syntaxProviderMock); var scope = (Scope) scopeprovider.CreateScope(); - var readDict = new Dictionary(); - readDict[Constants.Locks.Languages] = 1; - scope.ReadLocks[Guid.NewGuid()] = readDict; + try + { + // Request a lock to create the ReadLocks dict. + scope.ReadLock(Constants.Locks.Domains); - Assert.Throws(() => scope.Dispose()); + var readDict = new Dictionary(); + readDict[Constants.Locks.Languages] = 1; + scope.ReadLocks[Guid.NewGuid()] = readDict; - // We have to clear so we can properly dispose the scope, otherwise it'll mess with other tests. - scope.ReadLocks.Clear(); - scope.Dispose(); + Assert.Throws(() => scope.Dispose()); + } + finally + { + // We have to clear so we can properly dispose the scope, otherwise it'll mess with other tests. + scope.ReadLocks?.Clear(); + scope.Dispose(); + } } [Test] @@ -471,15 +479,47 @@ namespace Umbraco.Tests.Scoping var scopeprovider = GetScopeProvider(out var syntaxProviderMock); var scope = (Scope) scopeprovider.CreateScope(); - var writeDict = new Dictionary(); - writeDict[Constants.Locks.Languages] = 1; - scope.WriteLocks[Guid.NewGuid()] = writeDict; + try + { + // Request a lock to create the ReadLocks dict. + scope.WriteLock(Constants.Locks.Domains); - Assert.Throws(() => scope.Dispose()); + var writeDict = new Dictionary(); + writeDict[Constants.Locks.Languages] = 1; + scope.WriteLocks[Guid.NewGuid()] = writeDict; - // We have to clear so we can properly dispose the scope, otherwise it'll mess with other tests. - scope.WriteLocks.Clear(); - scope.Dispose(); + Assert.Throws(() => scope.Dispose()); + } + finally + { + // We have to clear so we can properly dispose the scope, otherwise it'll mess with other tests. + scope.WriteLocks?.Clear(); + scope.Dispose(); + } + } + + [Test] + public void WriteLocks_Not_Created_Until_First_Lock() + { + var scopeProvider = GetScopeProvider(out var syntaxProviderMock); + + using (var scope = scopeProvider.CreateScope()) + { + var realScope = (Scope) scope; + Assert.IsNull(realScope.WriteLocks); + } + } + + [Test] + public void ReadLocks_Not_Created_Until_First_Lock() + { + var scopeProvider = GetScopeProvider(out var syntaxProviderMock); + + using (var scope = scopeProvider.CreateScope()) + { + var realScope = (Scope) scope; + Assert.IsNull(realScope.ReadLocks); + } } } }