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; } }
}
}