From 161f2eda86f5f3494ae9a9d3109344e6bd7a7949 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 14 Jul 2015 15:55:56 +0200 Subject: [PATCH] DisposableObject - cleanup & use --- src/Umbraco.Core/DisposableObject.cs | 67 ++++++++----------- .../Scheduling/BackgroundTaskRunnerTests.cs | 2 +- .../Scheduling/LatchedBackgroundTaskBase.cs | 31 ++------- 3 files changed, 34 insertions(+), 66 deletions(-) diff --git a/src/Umbraco.Core/DisposableObject.cs b/src/Umbraco.Core/DisposableObject.cs index f054b53c98..516a9712e5 100644 --- a/src/Umbraco.Core/DisposableObject.cs +++ b/src/Umbraco.Core/DisposableObject.cs @@ -4,69 +4,58 @@ using System.Threading; namespace Umbraco.Core { /// - /// Abstract implementation of logic commonly required to safely handle disposable unmanaged resources. + /// Abstract implementation of IDisposable. /// + /// + /// Can also be used as a pattern for when inheriting is not possible. + /// + /// See also: https://msdn.microsoft.com/en-us/library/b1yfkh5e%28v=vs.110%29.aspx + /// See also: https://lostechies.com/chrispatterson/2012/11/29/idisposable-done-right/ + /// + /// Note: if an object's ctor throws, it will never be disposed, and so if that ctor + /// has allocated disposable objects, it should take care of disposing them. + /// public abstract class DisposableObject : IDisposable { private bool _disposed; - private readonly ReaderWriterLockSlim _disposalLocker = new ReaderWriterLockSlim(); + private readonly object _locko = new object(); - /// - /// Gets a value indicating whether this instance is disposed. - /// - /// - /// true if this instance is disposed; otherwise, false. - /// - public bool IsDisposed - { - get { return _disposed; } - } + // gets a value indicating whether this instance is disposed. + // for internal tests only (not thread safe) + //TODO make this internal + rename "Disposed" when we can break compatibility + public bool IsDisposed { get { return _disposed; } } - /// - /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. - /// - /// 2 + // implements IDisposable public void Dispose() { Dispose(true); - - // Use SupressFinalize in case a subclass of this type implements a finalizer. GC.SuppressFinalize(this); } + // finalizer ~DisposableObject() { - // Run dispose but let the class know it was due to the finalizer running. Dispose(false); } - protected virtual void Dispose(bool disposing) + //TODO make this private, non-virtual when we can break compatibility + protected virtual void Dispose(bool disposing) { - // Only operate if we haven't already disposed - if (IsDisposed || !disposing) return; + lock (_locko) + { + if (_disposed) return; + _disposed = true; + } - using (new WriteLock(_disposalLocker)) - { - // Check again now we're inside the lock - if (IsDisposed) return; + DisposeUnmanagedResources(); - // Call to actually release resources. This method is only - // kept separate so that the entire disposal logic can be used as a VS snippet - DisposeResources(); - - // Indicate that the instance has been disposed. - _disposed = true; - } + if (disposing) + DisposeResources(); } - /// - /// Handles the disposal of resources. Derived from abstract class which handles common required locking logic. - /// protected abstract void DisposeResources(); protected virtual void DisposeUnmanagedResources() - { - - } + { } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 812f5ed5e4..be681c9e72 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -547,7 +547,7 @@ namespace Umbraco.Tests.Scheduling runner.Shutdown(false, true); // check that task has been disposed (timer has been killed, etc) - Assert.IsTrue(task.Disposed); + Assert.IsTrue(task.IsDisposed); } } diff --git a/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs b/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs index 1fdb72bbb0..3315fa7c34 100644 --- a/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs @@ -1,13 +1,13 @@ using System; using System.Threading; using System.Threading.Tasks; +using Umbraco.Core; namespace Umbraco.Web.Scheduling { - internal abstract class LatchedBackgroundTaskBase : ILatchedBackgroundTask + internal abstract class LatchedBackgroundTaskBase : DisposableObject, ILatchedBackgroundTask { private readonly ManualResetEventSlim _latch; - private bool _disposed; protected LatchedBackgroundTaskBase() { @@ -51,34 +51,13 @@ namespace Umbraco.Web.Scheduling public abstract bool RunsOnShutdown { get; } - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - // the task is going to be disposed again after execution, + // the task is going to be disposed after execution, // unless it is latched again, thus indicating it wants to // remain active - protected virtual void Dispose(bool disposing) + protected override void DisposeResources() { - if (disposing == false) return; - - // lock on _latch instead of creating a new object as _latch is - // private, non-null, readonly - so safe here - lock (_latch) - { - if (_disposed) return; - _disposed = true; - _latch.Dispose(); - DisposeResources(); - } + _latch.Dispose(); } - - protected virtual void DisposeResources() - { } - - public bool Disposed { get { return _disposed; } } } }