diff --git a/src/Umbraco.Core/ObjectResolution/ManyObjectsResolverBase.cs b/src/Umbraco.Core/ObjectResolution/ManyObjectsResolverBase.cs index 4f3bc2665b..5a1514d130 100644 --- a/src/Umbraco.Core/ObjectResolution/ManyObjectsResolverBase.cs +++ b/src/Umbraco.Core/ObjectResolution/ManyObjectsResolverBase.cs @@ -169,47 +169,46 @@ namespace Umbraco.Core.ObjectResolution { get { - // ensure we can - if (CanResolveBeforeFrozen == false) - Resolution.EnsureIsFrozen(); + using (Resolution.Reader(CanResolveBeforeFrozen)) + { + // note: we apply .ToArray() to the output of CreateInstance() because that is an IEnumerable that + // comes from the PluginManager we want to be _sure_ that it's not a Linq of some sort, but the + // instances have actually been instanciated when we return. - // note: we apply .ToArray() to the output of CreateInstance() because that is an IEnumerable that - // comes from the PluginManager we want to be _sure_ that it's not a Linq of some sort, but the - // instances have actually been instanciated when we return. + switch (LifetimeScope) + { + case ObjectLifetimeScope.HttpRequest: + // create new instances per HttpContext + using (var l = new UpgradeableReadLock(_lock)) + { + // create if not already there + if (CurrentHttpContext.Items[_httpContextKey] == null) + { + l.UpgradeToWriteLock(); + CurrentHttpContext.Items[_httpContextKey] = CreateInstances().ToArray(); + } + return (TResolved[])CurrentHttpContext.Items[_httpContextKey]; + } - switch (LifetimeScope) - { - case ObjectLifetimeScope.HttpRequest: - // create new instances per HttpContext - using (var l = new UpgradeableReadLock(_lock)) - { - // create if not already there - if (CurrentHttpContext.Items[_httpContextKey] == null) - { - l.UpgradeToWriteLock(); - CurrentHttpContext.Items[_httpContextKey] = CreateInstances().ToArray(); - } - return (TResolved[])CurrentHttpContext.Items[_httpContextKey]; - } + case ObjectLifetimeScope.Application: + // create new instances per application + using (var l = new UpgradeableReadLock(_lock)) + { + // create if not already there + if (_applicationInstances == null) + { + l.UpgradeToWriteLock(); + _applicationInstances = CreateInstances().ToArray(); + } + return _applicationInstances; + } - case ObjectLifetimeScope.Application: - // create new instances per application - using(var l = new UpgradeableReadLock(_lock)) - { - // create if not already there - if (_applicationInstances == null) - { - l.UpgradeToWriteLock(); - _applicationInstances = CreateInstances().ToArray(); - } - return _applicationInstances; - } - - case ObjectLifetimeScope.Transient: - default: - // create new instances each time - return CreateInstances().ToArray(); - } + case ObjectLifetimeScope.Transient: + default: + // create new instances each time + return CreateInstances().ToArray(); + } + } } } diff --git a/src/Umbraco.Core/ObjectResolution/Resolution.cs b/src/Umbraco.Core/ObjectResolution/Resolution.cs index d9e5977e4a..17097ee7a5 100644 --- a/src/Umbraco.Core/ObjectResolution/Resolution.cs +++ b/src/Umbraco.Core/ObjectResolution/Resolution.cs @@ -1,5 +1,7 @@ using System; +using System.Linq; using System.Threading; +using Umbraco.Core.Logging; namespace Umbraco.Core.ObjectResolution { @@ -12,9 +14,10 @@ namespace Umbraco.Core.ObjectResolution /// internal static class Resolution { - private static readonly ReaderWriterLockSlim _lock = new ReaderWriterLockSlim(); + private static readonly ReaderWriterLockSlim ConfigurationLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion); + private volatile static bool _isFrozen; - /// + /// /// Occurs when resolution is frozen. /// /// Occurs only once, since resolution can be frozen only once. @@ -23,14 +26,27 @@ namespace Umbraco.Core.ObjectResolution /// /// Gets or sets a value indicating whether resolution of objects is frozen. /// - public static bool IsFrozen { get; private set; } - - public static void EnsureIsFrozen() + // internal for unit tests, use ReadFrozen if you want to be sure + internal static bool IsFrozen { - if (!IsFrozen) - throw new InvalidOperationException("Resolution is not frozen, it is not yet possible to get values from it."); + get + { + using (new ReadLock(ConfigurationLock)) + { + return _isFrozen; + } + } } + public static IDisposable Reader(bool canReadUnfrozen = false) + { + IDisposable l = new ReadLock(ConfigurationLock); + if (canReadUnfrozen || _isFrozen) return l; + + l.Dispose(); + throw new InvalidOperationException("Resolution is not frozen, it is not yet possible to get values from it."); + } + /// /// Returns a disposable object that represents safe access to unfrozen resolution configuration. /// @@ -39,13 +55,11 @@ namespace Umbraco.Core.ObjectResolution { get { - IDisposable l = new WriteLock(_lock); - if (Resolution.IsFrozen) - { - l.Dispose(); - throw new InvalidOperationException("Resolution is frozen, it is not possible to configure it anymore."); - } - return l; + IDisposable l = new WriteLock(ConfigurationLock); + if (_isFrozen == false) return l; + + l.Dispose(); + throw new InvalidOperationException("Resolution is frozen, it is not possible to configure it anymore."); } } @@ -65,21 +79,24 @@ namespace Umbraco.Core.ObjectResolution // keep the class here because it needs write-access to Resolution.IsFrozen private class DirtyBackdoor : IDisposable { - private static readonly System.Threading.ReaderWriterLockSlim _dirtyLock = new ReaderWriterLockSlim(); - private IDisposable _lock; - private bool _frozen; + private readonly IDisposable _lock; + private readonly bool _frozen; public DirtyBackdoor() { - _lock = new WriteLock(_dirtyLock); - _frozen = Resolution.IsFrozen; - Resolution.IsFrozen = false; + LogHelper.Debug(typeof(DirtyBackdoor), "Creating back door for resolution"); + + _lock = new WriteLock(ConfigurationLock); + _frozen = _isFrozen; + _isFrozen = false; } public void Dispose() { - Resolution.IsFrozen = _frozen; + LogHelper.Debug(typeof(DirtyBackdoor), "Disposing back door for resolution"); + + _isFrozen = _frozen; _lock.Dispose(); } } @@ -90,11 +107,17 @@ namespace Umbraco.Core.ObjectResolution /// resolution is already frozen. public static void Freeze() { - if (Resolution.IsFrozen) - throw new InvalidOperationException("Resolution is frozen. It is not possible to freeze it again."); + LogHelper.Debug(typeof(Resolution), "Freezing resolution"); - IsFrozen = true; - if (Frozen != null) + using (new WriteLock(ConfigurationLock)) + { + if (_isFrozen) + throw new InvalidOperationException("Resolution is frozen. It is not possible to freeze it again."); + + _isFrozen = true; + } + + if (Frozen != null) Frozen(null, null); } @@ -104,7 +127,20 @@ namespace Umbraco.Core.ObjectResolution /// To be used in unit tests. internal static void Reset() { - IsFrozen = false; + LogHelper.Debug(typeof(Resolution), "Resetting resolution"); + + /* + var trace = new System.Diagnostics.StackTrace(); + var testing = trace.GetFrames().Any(frame => + frame.GetMethod().DeclaringType.FullName.StartsWith("Umbraco.Tests")); + if (testing == false) + throw new InvalidOperationException("Only unit tests can reset configuration."); + */ + + using (new WriteLock(ConfigurationLock)) + { + _isFrozen = false; + } Frozen = null; } } diff --git a/src/Umbraco.Core/ObjectResolution/SingleObjectResolverBase.cs b/src/Umbraco.Core/ObjectResolution/SingleObjectResolverBase.cs index b4a54f66b6..27a82b17ef 100644 --- a/src/Umbraco.Core/ObjectResolution/SingleObjectResolverBase.cs +++ b/src/Umbraco.Core/ObjectResolution/SingleObjectResolverBase.cs @@ -102,10 +102,7 @@ namespace Umbraco.Core.ObjectResolution { get { - // ensure we can - if (CanResolveBeforeFrozen == false) - Resolution.EnsureIsFrozen(); - + using (Resolution.Reader(CanResolveBeforeFrozen)) using (new ReadLock(_lock)) { if (!_canBeNull && _value == null) diff --git a/src/Umbraco.Tests/Resolvers/ResolutionTests.cs b/src/Umbraco.Tests/Resolvers/ResolutionTests.cs index 8e9da977a2..37cf8bcb04 100644 --- a/src/Umbraco.Tests/Resolvers/ResolutionTests.cs +++ b/src/Umbraco.Tests/Resolvers/ResolutionTests.cs @@ -87,14 +87,16 @@ namespace Umbraco.Tests.Resolvers [ExpectedException(typeof(InvalidOperationException))] public void ResolutionCanDetectIfNotFrozen() { - Resolution.EnsureIsFrozen(); // throws + using (Resolution.Reader()) // throws + {} } [Test] public void ResolutionCanEnsureIsFrozen() { Resolution.Freeze(); - Resolution.EnsureIsFrozen(); + using (Resolution.Reader()) // ok + {} } [Test]