How do you prevent IDisposable from spreading to all your classes?
Start with these simple classes...
Let's say I have a simple set of classes like this:
class Bus
{
Driver busDriver = new Driver();
}
class Driver
{
Shoe[] shoes = { new Shoe(), new Shoe() };
}
class Shoe
{
Shoelace lace = new Shoelace();
}
class Shoelace
{
bool tied = false;
}
A Bus has a Driver , the Driver has two Shoe s, each Shoe has a Shoelace . All very silly.
Add an IDisposable object to Shoelace
Later I decide that some operation on the Shoelace could be multi-threaded, so I add an EventWaitHandle for the threads to communicate with. So Shoelace now looks like this:
class Shoelace
{
private AutoResetEvent waitHandle = new AutoResetEvent(false);
bool tied = false;
// ... other stuff ..
}
Implement IDisposable on Shoelace
But now Microsoft's FxCop will complain: "Implement IDisposable on 'Shoelace' because it creates members of the following IDisposable types: 'EventWaitHandle'."
Okay, I implement IDisposable on Shoelace and my neat little class becomes this horrible mess:
class Shoelace : IDisposable
{
private AutoResetEvent waitHandle = new AutoResetEvent(false);
bool tied = false;
private bool disposed = false;
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
~Shoelace()
{
Dispose(false);
}
protected virtual void Dispose(bool disposing)
{
if (!this.disposed)
{
if (disposing)
{
if (waitHandle != null)
{
waitHandle.Close();
waitHandle = null;
}
}
// No unmanaged resources to release otherwise they'd go here.
}
disposed = true;
}
}
Or (as pointed out by commenters) since Shoelace itself has no unmanaged resources, I might use the simpler dispose implementation without needing the Dispose(bool) and Destructor:
class Shoelace : IDisposable
{
private AutoResetEvent waitHandle = new AutoResetEvent(false);
bool tied = false;
public void Dispose()
{
if (waitHandle != null)
{
waitHandle.Close();
waitHandle = null;
}
GC.SuppressFinalize(this);
}
}
Watch in horror as IDisposable spreads
Right that's that fixed. But now FxCop will complain that Shoe creates a Shoelace , so Shoe must be IDisposable too.
And Driver creates Shoe so Driver must be IDisposable . And Bus creates Driver so Bus must be IDisposable and so on.
Suddenly my small change to Shoelace is causing me a lot of work and my boss is wondering why I need to checkout Bus to make a change to Shoelace .
The Question
How do you prevent this spread of IDisposable , but still ensure that your unmanaged objects are properly disposed?
You can't really "prevent" IDisposable from spreading. Some classes need to be disposed, like AutoResetEvent , and the most efficient way is to do it in the Dispose() method to avoid the overhead of finalizers. But this method must be called somehow, so exactly as in your example the classes that encapsulate or contain IDisposable have to dispose these, so they have to be disposable as well, etc. The only way to avoid it is to:
using pattern) In some cases IDisposable can be ignored because it supports an optional case. For example, WaitHandle implements IDisposable to support a named Mutex. If a name is not being used, the Dispose method does nothing. MemoryStream is another example, it uses no system resources and its Dispose implementation also does nothing. Careful thinking about whether an unmanaged resource is being used or not can be instructional. So can examining the available sources for the .net libraries or using a decompiler.
In terms of correctness, you can't prevent the spread of IDisposable through an object relationship if a parent object creates and essentially owns a child object which must now be disposable. FxCop is correct in this situation and the parent must be IDisposable.
What you can do is avoid adding an IDisposable to a leaf class in your object hierarchy. This is not always an easy task but it's an interesting exercise. From a logical perspective, there is no reason that a ShoeLace needs to be disposable. Instead of adding a WaitHandle here, is it also possible to add an association between a ShoeLace and a WaitHandle at the point it's used. The simplest way is through an Dictionary instance.
If you can move the WaitHandle into a loose association via a map at the point the WaitHandle is actually used then you can break this chain.
To prevent IDisposable from spreading, you should try to encapsulate the use of a disposable object inside of a single method. Try to design Shoelace differently:
class Shoelace {
bool tied = false;
public void Tie() {
using (var waitHandle = new AutoResetEvent(false)) {
// you can even pass the disposable to other methods
OtherMethod(waitHandle);
// or hold it in a field (but FxCop will complain that your class is not disposable),
// as long as you take control of its lifecycle
_waitHandle = waitHandle;
OtherMethodThatUsesTheWaitHandleFromTheField();
}
}
}
The scope of the wait handle is limited to the Tie method, and the class doesn't need to have a disposable field, and so won't need to be disposable itself.
Since the wait handle is an implementation detail inside of the Shoelace , it shouldn't change in any way its public interface, like adding a new interface in its declaration. What will happen then when you don't need a disposable field anymore, will you remove the IDisposable declaration? If you think about the Shoelace abstraction, you quickly realize that it shouldn't be polluted by infrastructure dependencies, like IDisposable . IDisposable should be reserved for classes whose abstraction encapsulate a resource that calls for deterministic clean up; ie, for classes where disposability is part of the abstraction.
上一篇: 托管和非托管代码，内存和大小有什么不同？