.NET Framework - Why call Connection.Close() in using block?

Asked By csharper on 23-Aug-11 04:47 PM
I see some people explicitly call the Close method of the
SqlConnection object in a using block. See below:

using (SqlConnection connection = new SqlConnection())
{
connection.ConnectionString = myConnectionString;
connection.Open();
// Do something;
connection.Close(); // <-- Is this necessary?
}

I do not quite understand the explicit call of connection.Close().  I
am sure it is not necessary, but does it do anything in particular?

Thank you.




Big Steel replied to csharper on 23-Aug-11 05:28 PM
I myself do it this way because I have been burnt by the using statement
leaving connections open particularly when things abort, and you keep
coming back and executing the statements opening, it blows up within the
using statement, and it leaves the connection open.

This can happen on an application that is waking up on a timer and keeps
going back and going back throughout a nightly during a nightly process,
blowing up,  leaving connections open and sucking up all the
connections on SQL server.

public DTOArticle GetArticle(int id)
{
var dto = new DTOArticle();
using (var conn = new EntityConnection(pcdb))
using (var db = new PublishingCompanyEntities(conn))
{
try
{
var article = (from a in db.Articles.Where(a =>
a.ArticleID == id)select a).First();

dto.ArticleID = article.ArticleID;
dto.AuthorID = (int) article.AuthorID;
dto.Body = article.Body;
dto.Tille = article.Title;

}
finally
{
conn.Close();
}
}

return dto;
}


You are going to get some people in here that will say that it cannot
happen.

You cannot convince me that other conditions or situations cannot cause
this issue with the using statement

http://msdn.microsoft.com/en-us/library/aa355056.aspx
Peter Duniho replied to csharper on 23-Aug-11 05:45 PM
For SqlConnection, no.  It is not necessary at all.  See
http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlconnection.close.aspx:

If the SqlConnection goes out of scope, it will not be closed.
Therefore, you must explicitly close the connection by calling
Close or Dispose. Close and Dispose are functionally equivalent.

Note the last sentence.

This is, in fact, .NET convention.  .NET classes with a Close() method
are _supposed_ to do exactly the same thing when calling Close() as when
calling Dispose().  Implementation is generally simply a matter of
calling one method from the other (either Dispose() calls Close() or
vice a versa).

For other DbConnection() implementations, it is _possible_ that the
provider has failed to write a correct .NET-compatible class and that
the two methods do something different.  In that case, you might have to
call both Close() and Dispose() (as an example, I have used a ZIP library
that has this requirement???it is awful, but it does happen).

Finally, note that all that a "using" statement does is insert a
try/finally block on your behalf.  There certainly is no point in adding
yet another try/finally block for the same object just to call
Dispose(), or Close() for an object like SqlConnection that does follow
the .NET convention.  That would be redundant.

However, for objects that do not follow the convention, writing a
try/finally to call both Close() and Dispose() would be necessary
(there'd be no point in having a "using" statement in that case, as it
adds very little convenience at that point).

Again, SqlConnection does not have this problem.  But other classes
might, especially those found in third-party libraries (I am not aware of
any .NET classes that do???but .NET is a huge library, and I suppose it is
possible at least one exists :) ).

Pete
Gene Wirchenko replied to csharper on 23-Aug-11 05:47 PM
From the general point of view, I prefer to handle things.  It
would make it clear to me that I was thinking of being done with the
connection.  (This could help in debugging.)  It is also very clear
where the connection is no longer needed, much more so than some "}"
ending a block.  (Think blocking error.)

And then there is Big Steel's reason.  I do not know enough to
say that he is right or wrong, but I have run into can-not-happen
situations before, too.  I agree with his caution, and even if he is
mistaken, the extra statement does not cost much.  If he is right, not
having the statement might be very costly.

Sincerely,

Gene Wirchenko
Peter Duniho replied to Gene Wirchenko on 23-Aug-11 06:04 PM
Then you should not be bothering with C#.  Just write your code in MSIL.
C# handles all sorts of things for you, just like any other high-level
language.  It is silly to eschew various language features on the basis
that you "prefer to handle things".  Handling things on your behalf is
the _whole point_ of using a high-level language.


For those who know the language, a "using" statement is actually _more_
clear than an explicit try/finally block.  It has a specific semantic
value, one that try/finally does not alone have.


One should try to avoid using poorly written libraries (e.g. those that
fail to follow the .NET conventions regarding Close() and Dispose()).
If the author does not care to understand the environment well enough to
get that right, there is no telling what other bugs and other hardships
are lying in wait in the library.

If one must use a third-party library with a class that requires calling
both Close() and Dispose(), then yes, you should not bother using the
this is not generally the case with Microsoft-provided classes that
exist in the .NET Framework itself (and is definitely not the case for
SqlConnection???there is no point at all in adding a call to Close() for
that class).

Note that at least according to the documentation, the
System.Data.EntityClient.EntityConnection class is a potential "gotcha",
because it does not in fact override the Dispose() method (so obviously
calling Dispose() for EntityConnection does not really do anything),
_and_ the documentation specifically says that its Close() method also
does nothing "If the underlying data provider is not known"
(http://msdn.microsoft.com/en-us/library/system.data.entityclient.entityconnection.close.aspx).
So depending on what provider you are using Close() may or may not be
needed, but in any case is not the same as calling Dispose().

Thus, while the example posted by the person posting as "Big Steel" is
not relevant to the OP's question, it appears to me from the
documentation that (assuming his "EntityConnection" is in fact the
EntityClient class in .NET) for EntityConnection you do need an explicit
try/finally with a call to Close().

The bottom line: it pays to read the documentation.  That is almost
always where you will find answers to questions like "do I need to call
Close() in addition to or instead of Dispose()?"

Pete
Big Steel replied to Peter Duniho on 23-Aug-11 06:59 PM
I say that it is relevant, because I was not using EF at the time when
the using statement issue stated leaving ADO.NET connections open. I was
using ADO.NET, Oracle Command Objects, T-SQL.  in a nightly process that
ran all night  based on xml files it saw in a staging area when files
were received to be processed and inserted into database tables
off-hours. It sucked up all the connections to the database with the DBA
in my face the next morning about it.

And here again is your misinterpretation of what I am saying, as usual.
You and a couple other people in this NG have a very bad habit of doing
this.

The example I posted is how I handle the close of the connection when
using a 'using statement' period, because I have been burnt by the using
statement expecting it to close connections on an abort. It did not close
anything leaving the connection open.

It does not matter if it is EF, ADO.NET,  database command objects or
connections from a client to a service are being used, I always always
take the control and close the connection myself using a try/finally and
not depending upon the using statement to do it, because I got burnt.

The OP wanted to know why people were doing an explicit close within a
using statement, and I tried to show him why they do it. And then I
showed an example how code in a using statement can be short-circuited,
blow out of the using statement and leave a connection open, because it
has happened.
Peter Duniho replied to Big Steel on 24-Aug-11 02:14 AM
original question.  Just because you have some _other_ experience that
you believe to me relevant, that does not mean the code you posted was
relevant.

The OP did not use EF in the question, so a code example that uses EF
is not relevant.


Right.  Like the last time, when you claimed to not have said _exactly_
the same words you had posted earlier?  And stood by that claim, even
when your previous post was quoted?

Forgive me if I have no sympathy for your concern of being misunderstood.


For classes that follow the .NET convention of making Dispose() and
Close() equivalent (and this includes SqlConnection, which is the class
that the OP is asking about), adding a Close() statement, with or
without a try/finally block, to code that already has a "using"
statement is pointless.  There is no possible way it would improve anything.

You might as well double up on your assignments, because you are afraid
that the first one might not do the job:

int i, j;

i = 5;
i = 5;
j = i + 10;
j = i + 10;

Or more to the point, double up on all your try/catch/finally blocks,
because you are afraid that one is not enough:

try
{
try
{
SomeMethod(foo, bar);
}
catch (Exception e)
{
// Handle exception. Belt goes here
}
}
catch (Exception e)
{
// Handle exception. Suspenders go here
}


I do not doubt that you had some problem, and I do not doubt that you
think that what fixed it was adding a redundant Close() statement in a
try/finally block to an object that was already being handled by a
latter is not at all.


Then show him code that is relevant to his question.  The
EntityConnection class is very different from the SqlConnection class,
in that it does not follow the standard .NET convention of making
Close() and Dispose() equivalent.  As such, it is not a suitable example
when discussing the SqlConnection class, especially not for a question
that is specifically about the very methods in which those two classes
are specifically different.


it is simply not true that "a using statement can be short-circuited" in
a way that adding another try/finally block would correct.

The only way for a "using" statement to fail to do its work is if the
entire thread is killed, and even then only if it is killed in a way that
prevents the normal ThreadAbortException from occurring in the thread
(e.g. the entire process is forcibly killed, or the thread itself is
killed from unmanaged code).  And thread death in those ways cannot be
handled by an explicit try/finally block any more than it can be by the
implicit try/finally block a "using" statement creates.

Pete

(and no, I doubt you will make it this far before you get upset and refuse
to read any further; and even if you do, I am sure your response will
involve some childish, profane, non-technical exclamation on your part,
rather than directly addressing the issues above.  You appear to have no
other means of handling a disagreement)
Big Steel replied to Peter Duniho on 24-Aug-11 06:19 AM
I am tossing your junk lip-service in the trash along with you.
Peter Duniho replied to Big Steel on 24-Aug-11 09:59 AM
Right on cue.
Big Steel replied to Peter Duniho on 24-Aug-11 12:05 PM
Read my other post......
Gene Wirchenko replied to Peter Duniho on 24-Aug-11 02:32 PM
No kidding.


Nothing about levels of abstraction?


And it ends at a difficult to see point: a brace.  Is this the
brace for the using or is it the next one?  I do indent, but misreads
can happen.


One should handle the situation either way.  "Well, the
documentation says it should work!" is often no taken well by the end
user.

[snip]


The pay is not as good as it should be.


I really would like it if documentation were that good, but all
too often, it is incomplete.  Then, there are the errors.

Once^WOften bitten, often careful.

Sincerely,

Gene Wirchenko