patterncsharpMinor
Dispose pattern - DisposableObject
Viewed 0 times
disposableobjectpatterndispose
Problem
I am trying to make an universal implementation of
```
public abstract class DisposableObject : IDisposable
{
private bool hasUnmanagedResources;
private bool baseDisposeManagedResourcesCalled;
private bool baseDisposeUnmanagedResourcesCalled;
protected bool IsDisposed { get; private set; }
protected bool IsDisposing { get; private set; }
public DisposableObject()
: this(false)
{
}
public DisposableObject(bool hasUnmanagedResources)
{
this.hasUnmanagedResources = hasUnmanagedResources;
if (!hasUnmanagedResources)
{
GC.SuppressFinalize(this);
}
}
~DisposableObject()
{
Dispose(false);
}
public void Dispose()
{
Dispose(true);
if (hasUnmanagedResources)
{
GC.SuppressFinalize(this);
}
}
protected virtual void DisposeManagedResources()
{
if (!IsDisposing)
{
throw new InvalidOperationException(
"In order to dispose an object call the Dispose method. Do not call DisposeManagedResources method directly." +
GetTypeString());
}
baseDisposeManagedResourcesCalled = true;
}
protected virtual void DisposeUnmanagedResources()
{
if (!IsDisposing)
{
throw new InvalidOperationException(
"In order to dispose an object call the Dispose method. Do not call DisposeUnmanagedResources method directly." +
GetTypeString());
}
baseDisposeUnmanagedResourcesCalled = true;
}
protected void HasUnmanagedResources()
{
if (!hasUnmanagedResources)
{
hasUnmanagedResources = true;
GC.ReRegisterForFinalize(this);
}
}
protected void ThrowIfDisposed()
{
if (IsDisposed)
{
throw new ObjectDisposedException("Object is al
IDisposable (as a base class):```
public abstract class DisposableObject : IDisposable
{
private bool hasUnmanagedResources;
private bool baseDisposeManagedResourcesCalled;
private bool baseDisposeUnmanagedResourcesCalled;
protected bool IsDisposed { get; private set; }
protected bool IsDisposing { get; private set; }
public DisposableObject()
: this(false)
{
}
public DisposableObject(bool hasUnmanagedResources)
{
this.hasUnmanagedResources = hasUnmanagedResources;
if (!hasUnmanagedResources)
{
GC.SuppressFinalize(this);
}
}
~DisposableObject()
{
Dispose(false);
}
public void Dispose()
{
Dispose(true);
if (hasUnmanagedResources)
{
GC.SuppressFinalize(this);
}
}
protected virtual void DisposeManagedResources()
{
if (!IsDisposing)
{
throw new InvalidOperationException(
"In order to dispose an object call the Dispose method. Do not call DisposeManagedResources method directly." +
GetTypeString());
}
baseDisposeManagedResourcesCalled = true;
}
protected virtual void DisposeUnmanagedResources()
{
if (!IsDisposing)
{
throw new InvalidOperationException(
"In order to dispose an object call the Dispose method. Do not call DisposeUnmanagedResources method directly." +
GetTypeString());
}
baseDisposeUnmanagedResourcesCalled = true;
}
protected void HasUnmanagedResources()
{
if (!hasUnmanagedResources)
{
hasUnmanagedResources = true;
GC.ReRegisterForFinalize(this);
}
}
protected void ThrowIfDisposed()
{
if (IsDisposed)
{
throw new ObjectDisposedException("Object is al
Solution
Firstly, to address inheritance concerns, I use a t4 template to get around the single inheritance problem. ie. for every class where I need to introduce dispose support, I generate the an abstract base class that inherits from that class. Works a treat.
Now, to your code. Here are the things that jump out at me:
FWIW, here is an example
```
// Generated by t4 template in Utility project. DO NOT EDIT!
#pragma warning disable 1591, 0612
#region Designer generated code
namespace Foo
{
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Threading;
using Kent.Boogaart.HelperTrinity.Extensions;
///
/// A useful base class for disposable objects that inherit from .
///
///
///
/// This base class simplifies the implementation of disposable objects by providing common functionality such as thread-safety on calls,
/// and a event for pre-disposal notification.
///
///
public abstract class DisposableBase : object, IDisposable
{
private const int DisposalNotStarted = 0;
private const int DisposalStarted = 1;
private const int DisposalComplete = 2;
#if DEBUG
// very useful diagnostics when a failure to dispose is detected
private readonly StackTrace creationStackTrace;
#endif
// see the constants defined above for valid values
private int disposeStage;
#if DEBUG
///
/// Initializes a new instance of the DisposableBase class.
///
protected DisposableBase()
{
this.creationStackTrace = new StackTrace(1, true);
}
///
/// Finalizes an instance of the DisposableBase class.
///
[SuppressMessage("Microsoft.Design", "CA1063", Justification = "The enforced behavior of CA1063 is not thread-safe or full-featured enough for our purposes here.")]
~DisposableBase()
{
var message = string.Format(CultureInfo.InvariantCulture, "Failed to proactively dispose of object, so it is being finalized: {0}.{1}Object creation stack trace:{1}{2}", this.ObjectName, Environment.NewLine, this.creationStackTrace);
Debug.Assert(false, message);
this.Dispose(false);
}
#endif
///
/// Occurs when this object is about to be disposed.
///
public event EventHandler Disposing;
///
/// Gets a value indicating whether this object is in the process of disposing.
///
protected bool IsDisposing
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalStarted, DisposalStarted) == DisposalStarted; }
}
///
/// Gets a value indicating whether this object has been disposed.
///
protected bool IsDisposed
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalComplete, DisposalComplete) == DisposalComplete; }
}
///
/// Gets a value indicating whether this object has been disposed or is in the process of being disposed.
///
protected bool IsDisposedOrDisposing
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalNotStarted, DisposalNotStarted) != DisposalNotStarted; }
}
///
/// Gets the object name, for use in any thrown by this object.
///
///
/// Subclasses can override this property if they would like more control over the object name appearing in any
/// thrown by this DisposableBase. This can be particularly useful in debugging and diagnostic scenarios.
///
///
/// The object name, which defaults to the class name.
///
protected virtual string ObjectName
{
get { return this.GetType().FullName; }
}
///
/// Disposes of this object, if it hasn't already been disposed.
///
[SuppressMessage("Microsoft.Design", "CA1063", Justification = "The enforced behavior of CA1063 is not thread-safe or full-featured enough for our purposes here.")]
[SuppressMessage("Microsoft.Usage", "CA1816", Justification = "GC.SuppressFinalize is called indirectly.")]
public void Dispose()
{
if (Interlocked.CompareExchange(ref this.
Now, to your code. Here are the things that jump out at me:
- Constructors should be
protectedbecause the base class isabstract. This would make the code clearer to me, but YMMV.
- Lose the whole unmanaged resource tracking. It adds complexity without value. Just call
SuppressFinalize()regardless - it's the recommended pattern, anyway.
- Your code is not thread-safe. Consider, for example, what happens if multiple threads call
Dispose()simultaneously. You're got the potential for multiple calls toDisposeManagedResources()etcetera.
FWIW, here is an example
Disposable base class generated by my t4 template:```
// Generated by t4 template in Utility project. DO NOT EDIT!
#pragma warning disable 1591, 0612
#region Designer generated code
namespace Foo
{
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Threading;
using Kent.Boogaart.HelperTrinity.Extensions;
///
/// A useful base class for disposable objects that inherit from .
///
///
///
/// This base class simplifies the implementation of disposable objects by providing common functionality such as thread-safety on calls,
/// and a event for pre-disposal notification.
///
///
public abstract class DisposableBase : object, IDisposable
{
private const int DisposalNotStarted = 0;
private const int DisposalStarted = 1;
private const int DisposalComplete = 2;
#if DEBUG
// very useful diagnostics when a failure to dispose is detected
private readonly StackTrace creationStackTrace;
#endif
// see the constants defined above for valid values
private int disposeStage;
#if DEBUG
///
/// Initializes a new instance of the DisposableBase class.
///
protected DisposableBase()
{
this.creationStackTrace = new StackTrace(1, true);
}
///
/// Finalizes an instance of the DisposableBase class.
///
[SuppressMessage("Microsoft.Design", "CA1063", Justification = "The enforced behavior of CA1063 is not thread-safe or full-featured enough for our purposes here.")]
~DisposableBase()
{
var message = string.Format(CultureInfo.InvariantCulture, "Failed to proactively dispose of object, so it is being finalized: {0}.{1}Object creation stack trace:{1}{2}", this.ObjectName, Environment.NewLine, this.creationStackTrace);
Debug.Assert(false, message);
this.Dispose(false);
}
#endif
///
/// Occurs when this object is about to be disposed.
///
public event EventHandler Disposing;
///
/// Gets a value indicating whether this object is in the process of disposing.
///
protected bool IsDisposing
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalStarted, DisposalStarted) == DisposalStarted; }
}
///
/// Gets a value indicating whether this object has been disposed.
///
protected bool IsDisposed
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalComplete, DisposalComplete) == DisposalComplete; }
}
///
/// Gets a value indicating whether this object has been disposed or is in the process of being disposed.
///
protected bool IsDisposedOrDisposing
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalNotStarted, DisposalNotStarted) != DisposalNotStarted; }
}
///
/// Gets the object name, for use in any thrown by this object.
///
///
/// Subclasses can override this property if they would like more control over the object name appearing in any
/// thrown by this DisposableBase. This can be particularly useful in debugging and diagnostic scenarios.
///
///
/// The object name, which defaults to the class name.
///
protected virtual string ObjectName
{
get { return this.GetType().FullName; }
}
///
/// Disposes of this object, if it hasn't already been disposed.
///
[SuppressMessage("Microsoft.Design", "CA1063", Justification = "The enforced behavior of CA1063 is not thread-safe or full-featured enough for our purposes here.")]
[SuppressMessage("Microsoft.Usage", "CA1816", Justification = "GC.SuppressFinalize is called indirectly.")]
public void Dispose()
{
if (Interlocked.CompareExchange(ref this.
Code Snippets
// Generated by t4 template in Utility project. DO NOT EDIT!
#pragma warning disable 1591, 0612
#region Designer generated code
namespace Foo
{
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Threading;
using Kent.Boogaart.HelperTrinity.Extensions;
/// <summary>
/// A useful base class for disposable objects that inherit from <see cref="object"/>.
/// </summary>
/// <remarks>
/// <para>
/// This base class simplifies the implementation of disposable objects by providing common functionality such as thread-safety on <see cref="Dispose"/> calls,
/// and a <see cref="Disposing"/> event for pre-disposal notification.
/// </para>
/// </remarks>
public abstract class DisposableBase : object, IDisposable
{
private const int DisposalNotStarted = 0;
private const int DisposalStarted = 1;
private const int DisposalComplete = 2;
#if DEBUG
// very useful diagnostics when a failure to dispose is detected
private readonly StackTrace creationStackTrace;
#endif
// see the constants defined above for valid values
private int disposeStage;
#if DEBUG
/// <summary>
/// Initializes a new instance of the DisposableBase class.
/// </summary>
protected DisposableBase()
{
this.creationStackTrace = new StackTrace(1, true);
}
/// <summary>
/// Finalizes an instance of the DisposableBase class.
/// </summary>
[SuppressMessage("Microsoft.Design", "CA1063", Justification = "The enforced behavior of CA1063 is not thread-safe or full-featured enough for our purposes here.")]
~DisposableBase()
{
var message = string.Format(CultureInfo.InvariantCulture, "Failed to proactively dispose of object, so it is being finalized: {0}.{1}Object creation stack trace:{1}{2}", this.ObjectName, Environment.NewLine, this.creationStackTrace);
Debug.Assert(false, message);
this.Dispose(false);
}
#endif
/// <summary>
/// Occurs when this object is about to be disposed.
/// </summary>
public event EventHandler Disposing;
/// <summary>
/// Gets a value indicating whether this object is in the process of disposing.
/// </summary>
protected bool IsDisposing
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalStarted, DisposalStarted) == DisposalStarted; }
}
/// <summary>
/// Gets a value indicating whether this object has been disposed.
/// </summary>
protected bool IsDisposed
{
get { return Interlocked.CompareExchange(ref this.disposeStage, DisposalComplete, DisposalComplete) == DisposalComplete; }
}
/// <summary>
/// Gets a value indicating whether this object haContext
StackExchange Code Review Q#32380, answer score: 9
Revisions (0)
No revisions yet.