Best practice with Disposable objects

  • Thread starter Thread starter proxyuser
  • Start date Start date
P

proxyuser

If I have a class that instantiates IDisposable objects for the lifetime of
my object, does that mean I always "have to" implement IDisposable as well
to dispose them? (Assuming I have no other reason to be disposable myself.)
If so, is there an easy way to know which objects are IDisposable without
having to go up the hierarchy chain of each included object? Not that this
is a major problem, I'm just askin'....
 
Nope, that's pretty much what you have to do in both cases. If the
object that you are using exists for the lifetime of your object, and it
implements IDisposable, then you pretty much have to implement IDisposable.
The thing is, you might want to consider why you are holding it for the
lifetime, and if you might want to hold it across method calls, if
applicable that is (like in the case of certain database operations, network
operations, etc, etc).

As for finding out whether or not something is IDisposable, I would
think that the documentation is usually the best resource (when it comes to
framework classes at least) with Reflector being a close second.
 
By the way, here is a disposal pattern from Troelsen's C# book. This is
just a nit, but I don't understand why the check for

if (!this.disposed)

is in the CleanUp method. Shouldn't it be in the Dispose method?

public class MyResourceWrapper : IDisposable
{
// Used to determine if Dispose()
// has already been called.
private bool disposed = false;

public void Dispose()
{
// Call our helper method.
// Specifying "true" signifies that
// the object user triggered the cleanup.
CleanUp(true); // Now suppress finalization.
GC.SuppressFinalize(this);
}

private void CleanUp(bool disposing)
{
// Be sure we have not already been disposed!
if (!this.disposed)
{
// If disposing equals true, dispose all
// If disposing equals true, dispose all
if (disposing)
{
// Dispose managed resources.
}
// Clean up unmanaged resources here.
}
disposed = true;
}

~MyResourceWrapper()
{
// Call our helper method.
// Specifying "false" signifies that
// the GC triggered the cleanup.
CleanUp(false);
}
}
 
This disposal pattern is ok if you are never going to derive from this
class (in which case, it should be marked as sealed), but in this case, I
would say that this pattern is lacking.

Basically, there is a guidline in the framework documentation that a
proper IDisposable implementation should be able to be called multiple times
without ill effects. In the author's opinion, a simple way to get around
this would be to just do it once, and then not do the same work again if it
has been done already.

Here is the documentation in MSDN on what the suggested way to implement
IDisposable is:

http://msdn.microsoft.com/en-us/library/b1yfkh5e(VS.71).aspx

Now personally, I can't say I agree completely with this, because I
don't think that the finalizer should call dispose. Rather, I think that
the finalizer should throw an exception, but that's just an overly
anal-retentive nature kicking in. The way I view it, if the class
implements IDisposable, and the finalizer is called (because Dispose was
not), then it means that you don't understand the lifetime requirements of
the class (as a consumer of it), and that is an implementation error.
Granted, throwing an exception in the finalizer isn't the solution, but it
at least tells you if your assumptions were wrong.
 
Nicholas Paldino said:
Basically, there is a guidline in the framework documentation that a
proper IDisposable implementation should be able to be called multiple
times without ill effects. In the author's opinion, a simple way to get
around this would be to just do it once, and then not do the same work
again if it has been done already.
Click to expand...

Thanks for the info. Yes, I just thought the check on the flag was in the
wrong place. Here's how I would have done his code:

public class MyResourceWrapper : IDisposable
{
// Used to determine if Dispose()
// has already been called.
private bool disposed = false;

public void Dispose()
{
if (!this.disposed)
{
// Call our helper method.
// Specifying "true" signifies that
// the object user triggered the cleanup.
CleanUp(true);
// Now suppress finalization.
GC.SuppressFinalize(this);
}
disposed = true;
}

private void CleanUp(bool disposing)
{
// If disposing equals true, dispose all
if (disposing)
{
// Dispose managed resources.
}
// Clean up unmanaged resources here.
}

~MyResourceWrapper()
{
// Call our helper method.
// Specifying "false" signifies that
// the GC triggered the cleanup.
CleanUp(false);
}
 
It's possible that will work, depending on the resources that are being
disposed of.

In your case, you are assuming that all implementations of IDispose that
your class instance is holding on to can be called multiple times with no
ill effect (as the framework guidelines suggest). The author assumes that
one of these could throw an exception if called twice (it would seem from
the code, at least), and just avoids calling any dispose code more than
once.

For the unmanaged resources, at least in Windows, most of the APIs that
handle the cleanup of unmanaged resources don't give you any problems if you
call them more than once with invalid handles. However, this is generally
bad practice, as there are always other errors that can be returned, and
those should be checked against and raised as exceptions if an unexpected
error occurs.

That being said, I would say the best way is to check the disposed flag
when trying to call the appropriate APIs to dispose of the unmanaged
resources. It wouldn't matter if Dispose is called multiple times on
references to classes that implement IDispose (ideally).

However, none of these address what you are to do in the event that
another class inherits from your class, in which case, you want to make
CleanUp (I would suggest renaming this to Dispose and having two versions) a
protected method.
 
proxyuser said:
public class MyResourceWrapper : IDisposable
{
// Used to determine if Dispose()
// has already been called.
private bool disposed = false;

public void Dispose()
{
if (!this.disposed)
{
// Call our helper method.
// Specifying "true" signifies that
// the object user triggered the cleanup.
CleanUp(true);
// Now suppress finalization.
GC.SuppressFinalize(this);
}
disposed = true;
}

private void CleanUp(bool disposing)
{
// If disposing equals true, dispose all
if (disposing)
{
// Dispose managed resources.
}
// Clean up unmanaged resources here.
}

~MyResourceWrapper()
{
// Call our helper method.
// Specifying "false" signifies that
// the GC triggered the cleanup.
CleanUp(false);
}
Click to expand...

Could you explain what's the point in defining a destructor, please ?
 
There are no destructors in C#, but there are finalizers. A finalizer
is code that should be executed in the event that the object is garbage
collected. In the case of implementing IDisposable, this is used as a kind
of backup, to dispose of unmanaged resources in the event that Dispose on
the IDisposable implementation is not called.
 
"Nicholas Paldino [.NET/C# MVP]" <[email protected]> a écrit
dans le message de groupe de discussion :
(e-mail address removed)...
There are no destructors in C#, but there are finalizers. A finalizer
is code that should be executed in the event that the object is garbage
collected. In the case of implementing IDisposable, this is used as a
kind of backup, to dispose of unmanaged resources in the event that
Dispose on the IDisposable implementation is not called.
Click to expand...

Do you mean that ~MyResourceWraper() is not called a destructor but a
finalizer in c# ?

In which cases could ~MyResourceWraper() be called and not Dispose ?
 
Christophe said:
"Nicholas Paldino [.NET/C# MVP]" <[email protected]> a
écrit dans le message de groupe de discussion :
(e-mail address removed)...
There are no destructors in C#, but there are finalizers. A
finalizer is code that should be executed in the event that the object
is garbage collected. In the case of implementing IDisposable, this
is used as a kind of backup, to dispose of unmanaged resources in the
event that Dispose on the IDisposable implementation is not called.
Click to expand...

Do you mean that ~MyResourceWraper() is not called a destructor but a
finalizer in c# ?
Click to expand...

Since C# 2.0 - in C# 1.x it was called a destructor - which was rather
confusing because it is a finalizer (aka Java finalizer) not a
destructor (aka C++ destructor).
In which cases could ~MyResourceWraper() be called and not Dispose ?
Click to expand...

If Dispose has not been called when the GC take it.

Arne
 
Christophe Lephay said:
Thanks. I must be confusing with using(...).
Click to expand...

using automatically calls the Dispose, which is one reason it's recommended
as good practice. It is always Disposed exactly when it's no longer needed
(i.e. no longer in scope.) This saves the developer from calling Dispose
explicitly, and at the right time.

By the way, normally you'd suppress the finalizer being called if Dispose
was already called (by putting GC.SuppressFinalize in the Dispose code.)
But that is completely up to the developer. It's entirely possible for the
finalizer to be called with Dispose having been called.
 
Back
Top