Well, maybe that's not
exactly how he put it; but he referenced
this post, where I wrote:
Richard Hale Shaw makes an interesting argument against the C# using statement (not the using directive; and thank you, C# team, for that bit of confusing language). I disagree with him; but it will take time and sleep before I can fully explain why. The short preview: he says you can't force people to use your class correctly; I say I can, and I'll show you how, soon.
And he writes:
Always wondered what you had in mind.
Well, he's right: it's way past time I finished this thought!
Fair warning: the rest of this post is code, code, code! Instead of reading further, you non-programmers may instead prefer to
go look at the cute squirrel.
****************************************************************
OK, now that only geeks and masochists are left, let's review the highlights of Richard's argument for why the C#
using statement isn't very helpful. Here are some excerpts and comments:
And more and more developers are discovering the third variation or using statement: using using for early disposal:
using(MyObj obj = new MyObj())
{
obj.DoStuff();
}
When the C# compiler encounters this use of using, it will only compile correctly if the target — in this case, obj — implements IDisposable (and therefore, IDisposable.Dispose).
Actually, I recently discovered this isn't
quite true. The class of the object need not implement IDisposable,
as long as one of its ancestor classes does. As long as the object may be cast to an IDisposable, it doesn't matter whether that's through the ultimate class or some ancestor class. Thus, for instance, a StreamWriter object can be used in a
using statement, even though StreamWriter doesn't implement IDisposable. Why? Because StreamWriter inherits from TextWriter, and TextWriter
does implement IDisposable. For a while, I would diligently look up each class before using it in a
using statement, checking to see if it implements IDisposable. Now I've decided that, when in doubt, I'll just stick it in a
using statement, and let the compiler tell me if there's no IDisposable to be found. If it does any work with files or ports or graphics resources, I just assume IDisposable until proven otherwise.
Consider GraphicsState, the class that saves the state of a Graphics object so that you can restore it later. To me, this class all but
screams to be used like this:
Graphics g; // This probably came in as some parameter somewhere.
...
using (GraphicsState gs = g.Save() ) // NOTE: This will not compile!
{
// Do stuff that changes g, counting on gs
// to clean it all up at the end of the using
// statement.
}
But nope, that doesn't work: GraphicsState doesn't implement IDisposable. The best you can do is something like this:
Graphics g; // This probably came in as some parameter somewhere.
...
GraphicsState gs = g.Save();
try
{
// Do stuff that changes g. You'll clean
// it up later.
}
finally
{
// And THIS is later.
g.Restore(gs);
}
Of course, there's always this workaround:
// Can't inherit from GraphicsState. It's marked sealed.
// Best you can do is get one from a parameter.
// NOTE: This class is ONLY AN EXAMPLE. It needs a LOT
// more work to be production-ready.
class DisposableState : IDisposable
{
private GraphicsState mState = null;
private Graphics mGraphics = null;
public DisposableState(Graphics g)
{
mGraphics = g;
mState = g.Save();
}
// NOTE: This method is PARTICULARLY lacking in
// important design features, as Richard discussed in
// his post, and as Bill Wagner explains in great
// detail in Effective C#.
public void Dispose()
{
if ( ( mGraphics != null ) && ( mState != null ) )
{
mGraphics.Restore( mState );
mGraphics = null;
mState = null;
}
}
}
...
Graphics g; // This probably came in as some parameter somewhere.
...
using (DisposableState ds = new DisposableState( g ))
{
// And NOW you can do stuff that changes g,
// secure in the knowledge that ds will clean
// it all up at the end of the using statement.
}
This still needs some design thought in all sorts of ways, particularly in regards to what happens if someone doesn't dispose of the DisposableState. But then again, that's back to the point of this post: we're going to make it really hard to
not dispose of a disposable item.
Continuing with excerpts from Richard's post:
In the resulting IL, the compiler takes what you put inside the block (or if you didn't use a block, the single statement that would follow the using statement) with a try...finally block:
IL_0000: newobj instance void ObjectDisposal.MyObj::.ctor()
IL_0005: stloc.0
.try
{
IL_0006: ldloc.0
IL_0007: callvirt instance void ObjectDisposal.MyObj::DoStuff()
IL_000c: leave.s IL_0018
} // end .try
finally
{
IL_000e: ldloc.0
IL_000f: brfalse.s IL_0017
IL_0011: ldloc.0
IL_0012: callvirt instance void [mscorlib]System.IDisposable::Dispose()
IL_0017: endfinally
} // end handler
Now here's a difference between Richard and me: he loves IL code, while I'd rather not look at it unless I have to. What he's saying, in plain C#, is that the
using block above is equivalent to this:
MyObj obj = new MyObj();
try
{
obj.DoStuff();
} // end .try
finally
{
obj.Dispose();
endfinally
} // end handler
There, isn't that a lot more comprehensible all all that nasty IL code?
Unfortunately, even though you'll often see people "translate" that
using block that way, it's also
wrong. Foreshadowing one of Richard's later points, it's important to note that in this
using block...
using(MyObj obj = new MyObj())
{
obj.DoStuff();
}
...the object obj is out of scope at the end of the block, and may no longer be referenced. Because it was declared within the
using statement, its scope ends when the
using statement ends — which means, in fact, the closing curly brace. So a truly proper C# "translation" of that
using block would look like this:
{
MyObj obj = new MyObj();
try
{
obj.DoStuff();
} // end .try
finally
{
obj.Dispose();
endfinally
}
} // end handler
Note the extra curly braces needed to define a scope block that encompasses obj.
Continuing again with excerpts from Richard's post:
But there's nothing automatic about using statements: the client or user of the class has to add them. That's no more automatic than manually calling Dispose yourself.
And here's where I'm going to differ with Richard: while we can't make
using automatic, we can add code that makes it extremely visible when someone has failed to dispose of resources properly. We can't force client code to be written well, but we can nudge really strongly. I'll explain the details at the end of this post.
There's also a way in which
using is, if not automatic, then easier. For one thing, it's just plain less typing and less formatting than a
try/
finally. I tell my students that I'm the laziest programmer on the planet, and I want to make my job as easy as possible. And I encourage them to adopt the same attitude: they should think, and the tool should work, not the other way around. Plus
using is almost always a good habit if not misused (as Richard discusses below); and I like to program good habits into my fingers, so that good code just sorta naturally flows out. Almost automatic, you might say.
I would also contend (since I can't help being pedantic) that there's something
slightly automatic about
using: it provides that implicit scope block described above, which lets the compiler tell you when someone tries to access the disposed object out of scope. Of course, that will lead into a problem that Richard very properly describes farther down:
And if the syntax above was the only way you could use using (where the target is instantiated in the parameter list), I'd have no objection to it whatsover. But the fact is that the target can be instantiated anytime prior to the using statement as well:
MyObj obj = new MyObj();
...
using(obj)
{
obj.DoStuff();
}
My problem with this is that you could still have what appear to be valid calls to methods/properties on what is now a disposed object:
MyObj obj = new MyObj();
...
using(obj)
{
obj.DoStuff();
}
...
obj.DoStuff();
Oh, and the compiler is NOT going to help you here.
Yep. Absolutely. This is a real problem, and you have to tread cautiously. I have run into a case now and then where I wanted to use Dispose on a previously allocated object. It's uncommon, but not necessarily wrong.
We should always prefer compiler-automatic to catch problems whenever possible. Relying on people to get it right can get us into trouble, because people are, well, people. They make mistakes. As Richard wrote:
The Dispose Pattern says that once Dispose has been called, subsequent calls to Dispose should be benign (do nothing) but subsequent calls to any other public operation should throw an ObjectDisposedException. So now we have a case where a developer thinks they've automated (or semi-automated) resource disposal, but could use the object again as well.
It's just too confusing: there's no way to know that the object has been disposed unless you know what the using statement does internally. If you know that, of course, you'd make it a Best Practice to avoid using using, or avoid using using without instantiating the target in the parameter list. I know this. You know this if you've attended one of my classes, or someone else's class, or read it in a book or figured it out yourself.
But how do you count on someone else — who's now charged with modifying and maintaining this code base — knowing it? You can't and you don't.
To me, Richard hasn't diagnosed a problem with
using; he has diagnosed a problem with IDisposable, and with .NET, and in fact with resource management in general in a memory-managed environment: even though the environment is cleaning up discarded memory for you, you can't guarantee people will free up their non-memory resources when they're done with them.
In other words, he has detected the Return of the Sandwich.
Back in the bad old days of Windows 3.0 programming, those of us schooled by
The Great One had to burn into our brains one overriding design limit: 64K. For you young'uns, a K is a kilobyte of memory. Yes, as in 1,024 bytes. Yes, 64K is probably less memory than you have in a cheap cell phone today. But that was all we had back then for certain types of data. How much room was there for
all of the window information for
all apps running simultaneously under Win3.0? 64K. How much room was there for
all the other drawing stuff — fonts, pens, brushes, regions, etc. — for
all apps running simultaneously under Win3.0? Another 64K. I'm not talking 64K per app; I'm talking 64K for the whole system. If my app grabbed 63.99K of either the window heap or the resource heap, your app was probably going to fail when it tried to display itself; and unless you wrote your code to handle that failure gracefully (and tested that failure), my
sloppy code could make
your code crash in a most ugly fashion. A carelessly written app could sometimes crash all of Windows with less than a score of mouse clicks.
So how did one write code carefully? With the Sandwich Pattern:
BREAD: Allocate some resource.
MEAT: Use the resource.
BREAD: Free the resource.
All good Win 3.0 code was full of Sandwiches. You need to print, and you want to use a special font?
BREAD: Allocate the font.
MEAT: Print with the font.
BREAD: Free the font.
There were lots of variations of Sandwiches, and Sandwiches within Sandwiches; but it all came down to the Sandwich Pattern.
Unfortunately, not everyone programming learned from The Great One. Some learned from following examples in magazines. Some learned from watching what others did. Some learned by visiting BBSes and chat rooms, and just generally picking up folklore. Some learned by trial and error. And even some who learned from The Great One didn't really get the importance of resource management.
And so some folks simply didn't use Sandwiches. And thus were many a user's day ruined when some Windows app crashed, or even Windows itself; and often the culprit app was impossible to determine, because the app that crashed was the one that couldn't get resources,
not the one that refused to give them up.
Now resource management and the Sandwich Pattern are by no means unique to Win 3.0, or even to Windows at all. (Win 3.0 just happened to be extraordinarily vulnerable to the problem.) So one of the important design elements of the C++ language was constructors and destructors: a constructor gets called when an obect gets created; and a destructor gets called when it gets destroyed. In a sense, they are the Sandwich Pattern built into the object, with the constructor and destructor serving as the bread. And this was very much by design: if you grabbed all your resources during the constructor and freed them in your destructor, then the code that used your object never had to worry about — or even know
about — your resource management. If you wrote a library for managing a resource, you could count on destructors to free up resources.
Except... Well, except there are still sloppy humans writing code; and they still sometimes make mistakes. If they created an object locally, the compiler would automate destructing the object when it went out of scope. But if they created the object on the heap, then it only got destroyed when they intentionally destroyed it. Like the Sandwich Pattern, the Construct/Destruct Pattern only works when you actually follow it.
Now as it happens, one of the most commonly lost resources in this situation is memory itself; and as it also happens, that's one that can be managed well automatically, with a good environment. And so that's what managed environments like .NET and Java typically do: they keep track of objects, and get rid of them automatically when they're no longer used.
And then .NET lets you
almost have the Construct/Destruct Pattern via Finalizers: you can clean up any final resources in the finalizer. Of course, that's probably too late, but at least you tried.
And .NET also lets you
almost have the Sandwich Pattern via the Dispose Pattern; but as Richard has correctly pointed out, we're back to a world where the Sandwich only works if everyone knows — and remembers — to use it.
So if the Sandwich only works if everyone knows — and remembers — to use it, that to me is not a
using problem:
using is just a particular form of the Dispose Pattern, which is a specialized form of the Sandwich Pattern, which relies on fallible humans. The problem is rather that sometimes people don't clean up, whether they're using
using or
try/
finally or whatever. And there's just no way for the compiler to detect that. Now the runtime environment can catch it, because things will fail; but when it does, the results are usually really ugly. We're not back in the days of the 64K limit, and it's pretty hard for an app to crash another app these days; but a poorly performing app can certainly drag down the whole system.
But even worse is that often things
won't fail. On your testing platform, the sloppy code never happens to hit a limit, so you never notice the problem until you ship. And then, of course, your customers find it. Or maybe your testers can tell that something's wrong, because the system starts misbehaving; but they can't diagnose what went wrong. In some ways, it's back to the bad old Win3.0 days: the problem doesn't pop up with the resources that haven't been freed; it pops up in other places, where requests for new resources fail, and then code crashes when it can't get those resources. (Yes, good code should be written to test for a failure to get a resource, and to fail gracefully in response. Wanna bet on how many programs actually do that?)
****************************************************************
So it seemed to me that what we really needed was a way to detect and report undisposed objects. And with a little thought, I realized I could do just that. The result is the class I call MustDispose. Here's the code, all in one chunk. After it's done, I'll dissect it for the important parts.
////////////////////////////////////////////////////////////////////////
// Class MustDispose
// Copyright (c) 2006 by Martin L. Shoemaker, The Tablet UML Company
// Permission is granted to reuse this code within any project, in whole
// or in part or in altered form, as long as this copyright statement
// is included along with any portion or modification of this code or
// of any work derived from this code.
////////////////////////////////////////////////////////////////////////
// Using the following namespace.
using System;
using System.Collections.Generic;
using System.Text;
using System.Diagnostics;
using System.Reflection;
// MustDispose is part of the NutsAndBolts library.
namespace TabletUMLCompany.NutsAndBolts
{
/// <summary>
/// Class MustDispose. Use this within an IDisposable implementation
/// to ensure that either the object is disposed, or a record is made.
/// </summary>
/// <example>
/// public class DisposableClass : IDisposable
/// {
/// // The single MustDispose.
/// private MustDispose mDispose = new MustDispose( true , true , "This object holds a brush, which must be released back to Windows when no longer needed." );
///
/// // The brush that we must release when we're done.
/// protected Brush mBrush = null;
///
/// // The color.
/// private Color mColor = Color.White;
///
/// // The color. We will always create a matching brush.
/// public Color Color
/// {
/// get
/// {
/// // Note how we use the Test method of the MustDispose
/// // to verify that we're not disposed yet. It will
/// // throw the appropriate exception if we ARE disposed.
/// mDispose.Test();
/// return mColor;
/// }
/// set
/// {
/// // Note how we use the Test method of the MustDispose
/// // to verify that we're not disposed yet. It will
/// // throw the appropriate exception if we ARE disposed.
/// mDispose.Test();
/// mColor = value;
/// DisposeBrush();
/// mBrush = new SolidBrush(mColor);
/// }
/// }
///
/// // Clean up the brush.
/// private void DisposeBrush()
/// {
/// // Note that we DON'T need a call to mDispose.Test() here.
/// // Since this is a private method, we're going to assume that
/// // we'll only call it when appropriate.
///
/// if (mBrush != null)
/// {
/// mBrush.Dispose();
/// mBrush = null;
/// }
/// }
///
/// // The Finalizer. Clean up here, if nowhere else.
/// ~DisposableClass()
/// {
/// // Call the internal Dispose, as per Wagner.
/// Dispose(false);
/// }
///
/// // The internal Dispose, as per Wagner.
/// protected virtual void Dispose(bool disposing)
/// {
/// // Ask the MustDispose whether it has been disposed yet.
/// if (!mDispose.Disposed)
/// {
/// if (disposing)
/// {
/// DisposeBrush();
/// }
/// }
///
/// // Dispose the MustDispose. VERY IMPORTANT! Otherwise, the
/// // MustDispose will falsely report resource leaks.
/// mDispose.Dispose();
/// }
///
/// // The standard Dispose implementation, as per Wagner.
/// public void Dispose()
/// {
/// Dispose(true);
/// GC.SuppressFinalize(this);
/// }
/// }
/// </example>
public class MustDispose : IDisposable
{
#region Fields.
/// <summary>
/// The type name of the object which was allocated but not disposed.
/// Defaults to this type name.
/// </summary>
private string mOwnerTypeName = "MustDispose";
#endregion
#region Properties with fields.
/// <summary>
/// Have we disposed this MustDispose yet?
/// </summary>
private bool mDisposed = false;
/// <summary>
/// Have we disposed this MustDispose yet? Read only.
/// </summary>
public bool Disposed
{
get { return mDisposed; }
}
/// <summary>
/// Are we supposed to throw an exception when an object is lost?
/// </summary>
private bool mThrow = true;
/// <summary>
/// Are we supposed to throw an exception when an object is lost?
/// </summary>
public bool Throw
{
get { return mThrow; }
set { mThrow = value; }
}
/// <summary>
/// Are we supposed to log an event when an object is lost?
/// </summary>
private bool mLog = false;
/// <summary>
/// Are we supposed to log an event when an object is lost?
/// </summary>
public bool Log
{
get { return mLog; }
set { mLog = value; }
}
/// <summary>
/// An optional reason why Dispose is required.
/// </summary>
private string mReason = "";
/// <summary>
/// An optional reason why Dispose is required.
/// </summary>
public string Reason
{
get { return mReason; }
set { mReason = value; }
}
#endregion
#region Construction and initialization.
/// <summary>
/// Simple constructor. Sets the type name.
/// </summary>
public MustDispose()
{
mOwnerTypeName = GetUndisposedTypeName();
}
/// <summary>
/// Another constructor. Sets the control flags.
/// </summary>
/// <param name="throwIfLost">True if we should throw an exception for a lost object.</param>
/// <param name="logIfLost">True if we should log an event for a lost object.</param>
public MustDispose(bool throwIfLost, bool logIfLost)
: this()
{
mThrow = throwIfLost;
mLog = logIfLost;
}
/// <summary>
/// Most detailed constructor. Sets the control flags, and also a reason why
/// disposing would be a good idea.
/// </summary>
/// <param name="throwIfLost">True if we should throw an exception for a lost object.</param>
/// <param name="logIfLost">True if we should log an event for a lost object.</param>
/// <param name="reason">What will go wrong if we don't dispose of the owner obect?</param>
public MustDispose(bool throwIfLost, bool logIfLost, string reason)
: this(throwIfLost, logIfLost)
{
mReason = reason;
}
#endregion
#region Destruction and clean-up.
/// <summary>
/// Finalizer.
/// </summary>
~MustDispose()
{
// Call the inner Dispose, as per Wagner.
Dispose(false);
}
/// <summary>
/// The internal Dispose, as described in Wagner's Effective C#.
/// The purpose of the internal Dispose is to have a Dispose with chaining up an
/// inheritance hierarchy, via virtual and override. This class is a root class,
/// so does not need to chain up. At this time, there are no derived classes;
/// but we're allowing for the possibility.
/// </summary>
/// <param name="disposing">
/// True if this was called as part of an IDisposable.Dispose call; false if this
/// was called as part of a finalizer.
/// </param>
protected virtual void Dispose(bool disposing)
{
// Multiple Dispose calls must be nondestructive.
if ( !mDisposed)
{
// We're disposed now. We need to set this flag RIGHT NOW. Otherwise, if we get
// some sort of exception below, we'll fail the Dispose Pattern rule that
// multiple Dispose calls must be non-destructive. That's actually not very
// likely to be a problem (because any exceptions are only likely to occur
// during finalization and Dispose calls after that are highly unlikely); but
// better safe than sorry.
mDisposed = true;
// If we're called as part of a finalizer and we haven't been disposed
// yet, that's a problem. The owner object expected to be disposed, dang it!
if (!disposing)
{
// First, assert that we're disposing. Then possibly take harsher measures.
System.Diagnostics.Debug.Assert(disposing, GetNotDisposedMessage());
// Throw an exception, if expected.
if (Throw)
{
ThrowNotDisposed();
}
// Log an event, if expected.
if (Log)
{
LogNotDisposed();
}
}
}
}
#endregion
#region IDisposable Members
/// <summary>
/// The external dispose.
/// </summary>
public void Dispose()
{
// Call the internal Dispose, as per Wagner.
Dispose(true);
// Suppress finalization. Not needed for final clean-up now.
GC.SuppressFinalize(this);
}
#endregion
#region Public worker methods.
/// <summary>
/// Test to ensure this has not been disposed yet. Throw the
/// appropriate exception if it has.
/// </summary>
public void Test()
{
if (mDisposed)
{
ThrowDisposed();
}
}
#endregion
#region Private worker methods.
/// <summary>
/// Get the message to use when warning about a failure to dispose.
/// </summary>
/// <returns>The message.</returns>
private string GetNotDisposedMessage()
{
string result = "Object of type " + mOwnerTypeName + " was not disposed. Possible resource leak!";
if (Reason != "")
{
result += Environment.NewLine + Reason;
}
return result;
}
/// <summary>
/// Get the message to use when warning about using a disposed object.
/// </summary>
/// <returns>The message.</returns>
private string GetDisposedMessage()
{
return "Object of type " + mOwnerTypeName + " was used after being disposed. Possible corrupt resources!";
}
/// <summary>
/// Get the type name of the object which was not disposed.
/// </summary>
/// <returns>The type name.</returns>
private string GetUndisposedTypeName()
{
try
{
// We're going to use System.Diagnostics and System.Reflection
// to find the owner object. Start by getting a stack trace.
StackTrace st = new StackTrace();
// Walk the stack, looking for a type other than MustDispose.
// That will be the owner type.
Type tThis = this.GetType();
for (int i = 0; i < st.FrameCount; i++)
{
// Get the stack frame and the method.
StackFrame sf = st.GetFrame(i);
MethodBase m = sf.GetMethod();
// Compare to MustDispose.
if (m.DeclaringType != tThis)
{
return m.DeclaringType.Name;
}
}
}
catch
{
}
// Never found an owner.
return this.GetType().Name;
}
/// <summary>
/// Throw a not-disposed exception, with explanation.
/// </summary>
private void ThrowNotDisposed()
{
throw new Exception(GetNotDisposedMessage());
}
/// <summary>
/// Log a not-disposed exception, with explanation.
/// </summary>
private void LogNotDisposed()
{
string appName = System.IO.Path.GetFileName(Process.GetCurrentProcess().MainModule.FileName);
EventLog.WriteEntry(appName, GetDisposedMessage());
}
/// <summary>
/// Throw a disposed exception, with explanation.
/// </summary>
private void ThrowDisposed()
{
throw new ObjectDisposedException(mOwnerTypeName, GetDisposedMessage());
}
#endregion
}
}
|
Whew! There's a lot of code there, especially when you comment it thoroughly. So as promised, I'll give you some dissection.
****************************************************************
But before that, let's get to the good news: because MustDispose is so elaborately crafted, it's really easy to use in any class you design to be disposed. Stealing from the example code above, let's look at a class that must be disposed:
public class DisposableClass : IDisposable
{
// The single MustDispose.
private MustDispose mDispose = new MustDispose( true , true , "This object holds a brush, which must be released back to Windows when no longer needed." );
// The brush that we must release when we're done.
protected Brush mBrush = null;
// The color.
private Color mColor = Color.White;
// The color. We will always create a matching brush.
public Color Color
{
get
{
// Note how we use the Test method of the MustDispose
// to verify that we're not disposed yet. It will
// throw the appropriate exception if we ARE disposed.
mDispose.Test();
return mColor;
}
set
{
// Note how we use the Test method of the MustDispose
// to verify that we're not disposed yet. It will
// throw the appropriate exception if we ARE disposed.
mDispose.Test();
mColor = value;
DisposeBrush();
mBrush = new SolidBrush(mColor);
}
}
// Clean up the brush.
private void DisposeBrush()
{
// Note that we DON'T need a call to mDispose.Test() here.
// Since this is a private method, we're going to assume that
// we'll only call it when appropriate.
if (mBrush != null)
{
mBrush.Dispose();
mBrush = null;
}
}
// The Finalizer. Clean up here, if nowhere else.
~DisposableClass()
{
// Call the internal Dispose, as per Wagner.
Dispose(false);
}
// The internal Dispose, as per Wagner.
protected virtual void Dispose(bool disposing)
{
// Ask the MustDispose whether it has been disposed yet.
if (!mDispose.Disposed)
{
if (disposing)
{
DisposeBrush();
}
}
// Dispose the MustDispose. VERY IMPORTANT! Otherwise, the
// MustDispose will falsely report resource leaks.
mDispose.Dispose();
}
// The standard Dispose implementation, as per Wagner.
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
}
A lot of what you see there is just a robust Dispose() implementation, as described in Bill Wagner's book; and much of the rest is an example of grabbing and managing a resource (a Brush, in this example). The only place where the MustDispose comes into play is these sections:
// The single MustDispose.
private MustDispose mDispose = new MustDispose( true , true , "This object holds a brush, which must be released back to Windows when no longer needed." );
To use a MustDispose, start by creating it as part of initializing or constructing your class.
// Note how we use the Test method of the MustDispose
// to verify that we're not disposed yet. It will
// throw the appropriate exception if we ARE disposed.
mDispose.Test();
We put a call to
mDispose.Test() at the start of each method. If the MustDispose has been disposed (and hence this object has as well), an ObjectDisposedException will be thrown, as required by the Dispose Pattern.
// The internal Dispose, as per Wagner.
protected virtual void Dispose(bool disposing)
{
// Ask the MustDispose whether it has been disposed yet.
if (!mDispose.Disposed)
{
if (disposing)
{
DisposeBrush();
}
}
// Dispose the MustDispose. VERY IMPORTANT! Otherwise, the
// MustDispose will falsely report resource leaks.
mDispose.Dispose();
}
Note how, rather than maintain a separate boolean flag in the owner object, we simply ask the MustDispose to determine whether we have been disposed yet. Note also that the last thing our Dispose must do is dispose the MusDispose.
So really, using a MustDispose is quite easy:
- Create it as part of initializing your disposable class.
- Test it whenever you want to ensure that your disposable class has not yet been disposed.
- Use it as a boolean flag for whether your disposable class has been disposed yet or not.
- Dispose of it when you dispose of your disposable class.
When you use MustDispose in these ways, it will automatically detect when you try to use an already-disposed object; and just as important, it will scream like a banshee when a disposable object is finalized instead of disposed. Most likely, that will all happen as the app is shutting down. If your client developers have been particularly sloppy, they (or your testers, or your customers) will probably get a slew of messages at app shutdown. They may also occur while running the app, but that's up to the .NET garbage collector: it finalizes objects when it decides to, and you have little say in the matter. (Again, see Bill Wagner's book.)
****************************************************************
So the tricky part, then, is in MustDispose. Let's dissect that, or at least the high points:
/// <summary>
/// Simple constructor. Sets the type name.
/// </summary>
public MustDispose()
{
mOwnerTypeName = GetUndisposedTypeName();
}
The simple constructor reads the owner type name, as described below. The other constructors add functionality to the simple constructor.
/// <summary>
/// The internal Dispose, as described in Wagner's Effective C#.
/// The purpose of the internal Dispose is to have a Dispose with chaining up an
/// inheritance hierarchy, via virtual and override. This class is a root class,
/// so does not need to chain up. At this time, there are no derived classes;
/// but we're allowing for the possibility.
/// </summary>
/// <param name="disposing">
/// True if this was called as part of an IDisposable.Dispose call; false if this
/// was called as part of a finalizer.
/// </param>
protected virtual void Dispose(bool disposing)
{
// Multiple Dispose calls must be nondestructive.
if ( !mDisposed)
{
// We're disposed now. We need to set this flag RIGHT NOW. Otherwise, if we get
// some sort of exception below, we'll fail the Dispose Pattern rule that
// multiple Dispose calls must be non-destructive. That's actually not very
// likely to be a problem (because any exceptions are only likely to occur
// during finalization and Dispose calls after that are highly unlikely); but
// better safe than sorry.
mDisposed = true;
// If we're called as part of a finalizer and we haven't been disposed
// yet, that's a problem. The owner object expected to be disposed, dang it!
if (!disposing)
{
// First, assert that we're disposing. Then possibly take harsher measures.
System.Diagnostics.Debug.Assert(disposing, GetNotDisposedMessage());
// Throw an exception, if expected.
if (Throw)
{
ThrowNotDisposed();
}
// Log an event, if expected.
if (Log)
{
LogNotDisposed();
}
}
}
}
The internal Dispose really isn't all that complicated. It says simply: "If we're being finalized, assert that that's a problem. Also log and throw an exception, if so instructed."
/// <summary>
/// Get the type name of the object which was not disposed.
/// </summary>
/// <returns>The type name.</returns>
private string GetUndisposedTypeName()
{
try
{
// We're going to use System.Diagnostics and System.Reflection
// to find the owner object. Start by getting a stack trace.
StackTrace st = new StackTrace();
// Walk the stack, looking for a type other than MustDispose.
// That will be the owner type.
Type tThis = this.GetType();
for (int i = 0; i < st.FrameCount; i++)
{
// Get the stack frame and the method.
StackFrame sf = st.GetFrame(i);
MethodBase m = sf.GetMethod();
// Compare to MustDispose.
if (m.DeclaringType != tThis)
{
return m.DeclaringType.Name;
}
}
}
catch
{
}
// Never found an owner.
return this.GetType().Name;
}
This function is the key. I wanted MustDispose to find the owner name by itself, without having to be passed it as a parameter. Why? Two reasons:
- The class name might change during refactoring (or less structured maintenance). I didn't want the connection between the real name and the reported name to be "brittle".
- Sure as shootin', somebody's going to be too lazy to type the line of code that creates a MustDispose, and will just copy-and-paste it from another class. (Me!) And when they do, if there's a name parameter required, sure as shootin' they're gonna forget to change it. (Me again!) And then the whole purpose of MustDispose will be weakened: it will report that you failed to dispose, but it will report the wrong thing was undisposed.
So instead, I used System.Diagnostics to get a stack trace, and then walked it backwards using System.Reflection to compare types until I found a type other than MustDispose. That is, naturally, the owner type.
The only downside to this approach is security. I'm no code access security expert; but I have to believe there are times when the System.Diagnostics or System.Reflection operations that I use are restricted. More research is called for there.
/// <summary>
/// Log a not-disposed exception, with explanation.
/// </summary>
private void LogNotDisposed()
{
string appName = System.IO.Path.GetFileName(Process.GetCurrentProcess().MainModule.FileName);
EventLog.WriteEntry(appName, GetDisposedMessage());
}
For a very simplistic way of identifying the app in the Event Log, I used Process.GetCurrentProcess().MainModule.FileName. That may also present some security problems. Again, more research is needed.
****************************************************************
That's the code: how to use MustDispose, and how it works. But there's one missing piece of the solution here: the MustDispose process.
What you're going to have to decide in your code is when to use MustDispose, how "loud" to make the messages, and what do you do about them? Do you turn them up really loud, and then have testers refuse to approve a build that has MustDispose errors? Do you make them really quiet and ship the code anyway? I can't make those calls for you. I've given you the tools, but now you have to decide how to use them.
****************************************************************
So that's it. I do agree with Richard that failure to dispose is a problem; I disagree that it's a problem peculiar to the
using statement. It's a problem with resource management and disposal in general. And nothing can make disposal automatic, it seems, no matter how hard we try. But with the proper use of MustDispose, it's at least easier to detect and diagnose when you have disposal failures.
Remember what I said?
I disagree with him; but it will take time and sleep before I can fully explain why.
I wasn't kidding!
