Simple Dispose Question

J

Jeff B.

If an object implements the IDisposable interface, should I always call the
Dispose method or is just setting it to null and letting the GC handle it
sufficient? Here is the pattern I've been using but wasn't sure if it was
necessary:

DataAdapter da = null;

try {
// Some logic here...
} catch (Exception ex) {
// Some exception handling logic here...
} finally {
// Clean up...
if (da != null) {
da.Dispose();
da = null;
}

Is the above necessary or could I just set da = null in the "finally" clause
and be good?

--- Thanks, Jeff

--

Jeff Bramwell
Digerati Technologies, LLC
www.digeratitech.com

Manage Multiple Network Configurations with Select-a-Net
www.select-a-net.com
 
T

Telmo Sampaio

Hi Jeff,

If a class has a Dispose method the best practice is to call it. the reason
behind that is that Dispose runs when called, whereas setting the object to
null simply adds a entry to the Finalize queue in GC, and we cannot
determine when GC will run.

HTH,
Telmo Sampaio
MCT
 
S

sadhu

Setting da to null isn't really necessary, the GC is intelligent enough
to know that there are no more references. Calling Dispose() has
nothing to do with GC either.

Calling Dispose() is recommended for the simple reason that the
resources held by that class get released earlier (it does *not* mean
the object will get GCed immediately). It also prevents the object from
being added to the finalization queue and getting finalized, which can
delay the GCing of the object.

Regards
Senthil
 
K

Kevin Yu [MSFT]

Hi Jeff,

I agree with Senthil. The Dispose method is often used to dispose unmanaged
resources that the object uses. What I recommend is not to call it, but
just leave it to GC. HTH.

Kevin Yu
=======
"This posting is provided "AS IS" with no warranties, and confers no
rights."
 
J

Jon Skeet [C# MVP]

Jeff B. said:
If an object implements the IDisposable interface, should I always call the
Dispose method or is just setting it to null and letting the GC handle it
sufficient? Here is the pattern I've been using but wasn't sure if it was
necessary:

DataAdapter da = null;

try {
// Some logic here...
} catch (Exception ex) {
// Some exception handling logic here...
} finally {
// Clean up...
if (da != null) {
da.Dispose();
da = null;
}

Is the above necessary or could I just set da = null in the "finally" clause
and be good?

An equivalent but simpler solution would be to use the using statement:

using (DataAdapter da = ...)
{
....
}

(You can put a try/catch inside the using statement if you really want
- most of the time you should let exceptions propagate up.)

Relying on the garbage collector to release unmanaged resources is a
very bad idea - there's no guarantee about when it will finalize the
object.
 
J

Jeffrey Palermo, MCAD.Net

Jeff,
I'd like to add something that hasn't come out in this thread yet. The
use of .Dispose() has been thoroughly discussed, but I'd like to answer
your question regarding the benefit of additionally setting the
variable to null.

If I assume that you have a method containing only that code:
void Foo(){
DataAdapter da = null;
try {
// Some logic here...
} catch (Exception ex) {
// Some exception handling logic here...
} finally {
// Clean up...
if (da != null) {
da.Dispose();
da = null;
}
}
you are setting da to null one statement before it would naturally be
set to null (or go out of scope). At the end of every method, every
variable declared in the method is set to null and goes out of scope,
so there is no benefit of explicitly setting it to null because it will
automatically happen one statement later.

On the other hand, there are situations where if may be beneficial to
set this to null. For instance:
void Foo(){
DataAdapter da = new DataAdapter(. . .);
try{
// use it somehow
}finally{
da.Dispose();
}
// now call another method that may take a while to return
}
Here, we would wait for another long-running method to return and the
da variable would still be in scope. Depending on what the
long-running method does, the GC may run several times. IN THIS CASE,
if you set da to null just before calling the long-running method, any
garbase collections would be able to reclaim that memory, and you would
have a benefit.

Normally, my methods are small enough that my variables naturally go
out of scope quickly, but in cases where they might not, set it to
null.

Best regards,
Jeffrey Palermo
Blog: http://www.jeffreypalermo.com
 
J

Jon Skeet [C# MVP]

On the other hand, there are situations where if may be beneficial to
set this to null. For instance:
void Foo(){
DataAdapter da = new DataAdapter(. . .);
try{
// use it somehow
}finally{
da.Dispose();
}
// now call another method that may take a while to return
}
Here, we would wait for another long-running method to return and the
da variable would still be in scope. Depending on what the
long-running method does, the GC may run several times. IN THIS CASE,
if you set da to null just before calling the long-running method, any
garbase collections would be able to reclaim that memory, and you would
have a benefit.

Actually, in most cases there's still no benefit - in release mode, the
JIT notices that the variable is never read again, and makes it
eligible for collection even before it goes out of scope.
 
J

Jon Skeet [C# MVP]

Kevin Yu said:
I agree with Senthil. The Dispose method is often used to dispose unmanaged
resources that the object uses. What I recommend is not to call it, but
just leave it to GC. HTH.

That way you end up with hard-to-diagnose problems where, say, one
stream has finished writing to a file, but the stream hasn't been
finalized. An attempt to open the file for reading would then fail -
but on another run it might succeed, because the garbage collector
might have finalized the writing stream earlier.

In another situation, with a databasse connection, you might end up
running out of pooled connections even though they're not actually
being used by anything any more.

In short, Dispose is there for a reason - call it, or face nasty bugs
which can be a real pain to fix (partly because the behaviour will
change between debug and release).
 
J

Jeff B.

Thanks everybody for your feedback. Pretty much what I get out of this is
that I should be calling Dispose if it's available and don't worry too much
about setting the variable to null. So I can pretty much follow either of
these patterns:

DataAdapter da = null;
try {
// Some logic here...
} catch (Exception ex) {
// Some exception handling logic here - only if I need an exception
handler at this level
} finally {
// Clean up...
if (da != null) {
da.Dispose();
}

--- or ---

DataAdapter da = null;
try {
using { da = new DataAdapter(...)
// Some logic here...
}
} catch (Exception ex) {
// Some exception handling logic here - only if I need an exception
handler at this level
}

--

Jeff Bramwell
Digerati Technologies, LLC
www.digeratitech.com

Manage Multiple Network Configurations with Select-a-Net
www.select-a-net.com
 
K

Kevin Yu [MSFT]

Yes, Jon. You're right. Thanks for sharing your idea. What I meant to most
objects that doesn't consume unmanaged resources, we needn't worry about
disposing the object, but GC will take care of it.

Kevin Yu
=======
"This posting is provided "AS IS" with no warranties, and confers no
rights."
 
J

Jon Skeet [C# MVP]

Kevin Yu said:
Yes, Jon. You're right. Thanks for sharing your idea. What I meant to most
objects that doesn't consume unmanaged resources, we needn't worry about
disposing the object, but GC will take care of it.

Yes - but in those cases you *can't* dispose of the object because they
won't implement IDisposable :)

(The exceptions are things which inherit from MarshalByValueComponent,
Stream or whatever without really needing to dispose of anything. Even
so, it's often worth calling Dispose in those cases as there may well
be a finalizer - while many classes in that situation tell the GC to
suppress finalization, others may not.)
 
J

Jon Skeet [C# MVP]

Jeff B. said:
Thanks everybody for your feedback. Pretty much what I get out of this is
that I should be calling Dispose if it's available and don't worry too much
about setting the variable to null. So I can pretty much follow either of
these patterns:

DataAdapter da = null;
try {
// Some logic here...
} catch (Exception ex) {
// Some exception handling logic here - only if I need an exception
handler at this level
} finally {
// Clean up...
if (da != null) {
da.Dispose();
}

--- or ---

DataAdapter da = null;
try {
using { da = new DataAdapter(...)
// Some logic here...
}
} catch (Exception ex) {
// Some exception handling logic here - only if I need an exception
handler at this level
}

I'd actually go for:

using (DataAdapter da = new DataAdapter(...))
{
try
{
...
}
catch
{
...
}
}

That way it keeps the scope of da tighter.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top