Disposing SqlConnection

M

MuZZy

Hi,
Just wanted to make sure i get it right: consider this class:

// =========== START CODE =============
class Test
{
private SqlConnection con = null;
public void Connect()
{
con = new SqlConnection(SomeConnectionString);
<Do something with this connection here>
}
}
// =========== END CODE =============


If i understand right, i don't HAVE TO disconnect explicitly, if i use an object of this class as a
local reference, like here:

// =========== START CODE =============
void SomeFunc()
{
Test t = new Test();
t.Connect();
}
// =========== END CODE =============
"t" should be disposed by GC as the object gets dereferenced right after the line t.Connect()
So technically i don't need to implement Dispose or create/call t.Disconnect, right?

Suggestions would be highly appreciated!
Thank you,
Andrey
 
D

DalePres

Is there any reason you would not want to explicitly close your connection
and dispose of it in your class?

Not explicitly closing your connection can result in tying up connection
pool connections longer than is necessary while you wait for the GC to
dispose of your objects. Always explicitly close your connection. In
addition, to closing your connection, you should consider controlling the
lifetime and scope of your connections by creating them in a using block:

using (SqlConnection con = new SqlConnection("connection string"))
{
try
{
// do some stuff with the connection here
}
catch (Exception ex)
{
}
finally
{
con.Close();
}
}

DalePres
MCAD, MCDBA, MCSE
 
J

Jon Skeet [C# MVP]

DalePres said:
Is there any reason you would not want to explicitly close your connection
and dispose of it in your class?

Not explicitly closing your connection can result in tying up connection
pool connections longer than is necessary while you wait for the GC to
dispose of your objects. Always explicitly close your connection.

No - disposing the connection is good enough, if you don't want to
reopen it later.
In addition, to closing your connection, you should consider controlling the
lifetime and scope of your connections by creating them in a using block:

I'd use that in preference to manually calling Close.
 
C

Chris Lyon [MSFT]

Hi Audrey

I think you're confusing disposing with garbage collecting. The GC will
reclaim the managed memory after t is no longer referenced (t is garbage).
Disposing of an object must be done explicitly by calling Dispose. Inside
Dispose(), you should clean up your UNmanaged resources (like closing
database connections).

So in this case, Test should implement IDisposable, and inside Dispose(),
close the SqlConnection.

For more information on Dispose, see

http://weblogs.asp.net/clyon/archive/2004/09/21/232445.aspx
http://weblogs.asp.net/clyon/archive/2004/09/23/233464.aspx

Hope that helps
-Chris

--------------------
|
| Hi,
| Just wanted to make sure i get it right: consider this class:

<snip>

| "t" should be disposed by GC as the object gets dereferenced right after
the line t.Connect()
| So technically i don't need to implement Dispose or create/call
t.Disconnect, right?
 
M

MuZZy

DalePres said:
Is there any reason you would not want to explicitly close your connection
and dispose of it in your class?

Not explicitly closing your connection can result in tying up connection
pool connections longer than is necessary while you wait for the GC to
dispose of your objects. Always explicitly close your connection. In
addition, to closing your connection, you should consider controlling the
lifetime and scope of your connections by creating them in a using block:

using (SqlConnection con = new SqlConnection("connection string"))
{
try
{
// do some stuff with the connection here
}
catch (Exception ex)
{
}
finally
{
con.Close();
}
}

DalePres
MCAD, MCDBA, MCSE

Well, the story is that i took over a project which is already being developed/maintained/SOLD for
two and a half years. Right now besides doing hot fixes and some new development on the project, i
also investigate some memory problems.

And one thing that i see here is that multiple connections are being created in different classes.
To somehow compinsate this i move objects of tose classes to the local scope, but there is no way i
could be adding a Dispose for them - too many places to add it to, and it would take me about a year
to do that, which is obviousely not acceptable. I just need to make sure that i've done the best
possible.

So asking this question i wanted to make sure that there shouldn't be much problems with leaving the
connections on GC's will :)
I don't think it's big deal if th econnections pool will be tied, as i see no more than 5-10
concurrent connections from my machine to the SQL server at any point of time - i guess SQL server
can manage multiple connections fine.
 
M

MuZZy

Chris said:
Hi Audrey

I think you're confusing disposing with garbage collecting. The GC will
reclaim the managed memory after t is no longer referenced (t is garbage).
Disposing of an object must be done explicitly by calling Dispose. Inside
Dispose(), you should clean up your UNmanaged resources (like closing
database connections).

So in this case, Test should implement IDisposable, and inside Dispose(),
close the SqlConnection.

For more information on Dispose, see

http://weblogs.asp.net/clyon/archive/2004/09/21/232445.aspx
http://weblogs.asp.net/clyon/archive/2004/09/23/233464.aspx

Hope that helps
-Chris
Thanks for the reply Chris!
No, i'm not confusing dosposing with GC. My question actually was if i should implement and call
Dispose, which would explicitly disconnect from database server, or i could leave it to GC to collect.
 
S

Sami Vaaraniemi

MuZZy said:
Well, the story is that i took over a project which is already being
developed/maintained/SOLD for two and a half years. Right now besides
doing hot fixes and some new development on the project, i also
investigate some memory problems.

And one thing that i see here is that multiple connections are being
created in different classes. To somehow compinsate this i move objects of
tose classes to the local scope, but there is no way i could be adding a
Dispose for them - too many places to add it to, and it would take me
about a year to do that, which is obviousely not acceptable. I just need
to make sure that i've done the best possible.

So asking this question i wanted to make sure that there shouldn't be much
problems with leaving the connections on GC's will :)
I don't think it's big deal if th econnections pool will be tied, as i see
no more than 5-10 concurrent connections from my machine to the SQL server
at any point of time - i guess SQL server can manage multiple connections
fine.

In my experience, if you do not explicitely close or dispose every single
SqlConnection, you'll run into the 'connection pool exhausted' error sooner
or later, at which point the application becomes unusable for some time.
This might happen only under heavy load but then again I've never seen 'it
is acceptable for the application to fail under load' in a requirements
specification.

Also, there are circumstances in which you'll run into the problem a lot
sooner. If the application runs against MSDE and the connection pool is
limited to 5 connections (which is what you should do to avoid the workload
governor from kicking in), then you'll start seeing 'connection pool
exhausted' errors very quickly.

Regards,
Sami
 
C

Cor Ligthert

MuZZy

In the version Net 1.x is adviced for the best results to use the
conn.dispose because in that is done some extra work beneath the blankets
for connection pooling.

This is done in Net 2.x in another way and than you can use the close as
well.

I hope this helps?

Cor
 
J

Jeffrey Palermo, MCAD.Net

There are several items I would like to comment on. First, Cor
Ligthert, you are dead wrong about .Net 1.x. If you use Reflector to
look at the Dispose() method, here is the actual source:
protected override void Dispose(bool disposing)
{
if (disposing)
{
switch (this._objectState)
{
case ConnectionState.Open:
{
this.Close();
break;
}
}
this._constr = null;
}
base.Dispose(disposing);
}

So on SqlConnection, calling Dispose() calls close if the connection is
still open. For the most part in the .Net framework, the Dispose()
implementation also calls close for connections, files, etc, but it is
not guaranteed. Unless you know for _certain_ that Dispose() also
calls Close() (and a properly implemented Dispose() method _will_ close
all unmanaged resources), use caution. For code readability, you could
call Close() and then Dispose(), but if you prefer, it's real easy to
use Reflector to make sure of what the Dispose() is doing. Then you
can code with confidence.

It is very important _not_ to leave connection closing and disposing up
to the GC. Not only will you have connection pooling problems under
load, but your app will use more memory than necessary.

If a class has a finalizer, and GC.SuppressFinalization() isn't called
on it, it cannot be collected from Gen 0. It will always be promoted
to Gen 1 and then clean up from there later when a Gen 1 collection
runs. Yes, at this time the collection will run the Finalizer (which
_should_ call Dispose() if implemented correctly), and your connection
would be closed and memory reclaimed. While you are waiting for a Gen
1 collection, you could up all the connections in the pool. I have
seen this happen, and it's not pretty.

If you dispose an object as soon as you don't need it anymore, you
minimize memory and resource usage. I suggest this approach.

Muzzy, even if you only need 5-10 connections now, plan for more.
Dispose of your connections immediately after using them. Don't wait
for the GC to do it. You don't know how long that will be.

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

Cor Ligthert

Jeffrey,
There are several items I would like to comment on. First, Cor
Ligthert, you are dead wrong about .Net 1.x.

Can be, it is just as Angel Saenz-Badillos[MS] always is saying.

He sais always that they used the dispose for connection pooling and
therefore I call it beneath the blankets.

I hope you don't mind that I keep it a while with Angels replies, because
you cannot say in the Adonet newgroup not to use dispose for a connection on
Net 1.x and you will get a reply from Angel.

Beside that I believe him.

Cor
 
C

Cor Ligthert

Jeffrey Palermo, MCAD.Net

In fact have I seen the behaviour that Angel told why dispose with the
connection should be used with framework 1.x.

So please when you next time write that somebody is "dead wrong" with his
answer than try it or investigate first why somebody wrote that. It gives
in my idea now a kind of just own opinion in your answers based on a very
small investigation just in top code.

While I have seen from your other answers that your knowledge is enough to
prevent that.

You need at least more than 100 simultanious connections to test this
behaviour.

As addition to my previous answer.

Cor
 
J

Jeffrey Palermo, MCAD.Net

Cor,
Sorry for the confusion. Let me seperate the message:

"In the version Net 1.x is adviced for the best results to use the
conn.dispose. . ."
Yes, this is correct. I agree with you about this.

". . . because in that is done some extra work beneath the blankets
for connection pooling."
This isn't correct. The only thing the Dispose() method does is to
close the connection. It doesn't do any extra work for connection
pooling. This can be seen by the code I posted.

Cor, I apologize for not being clear.

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

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