CA2022 - Object can be disposed more than once

I have started to get a series of CA2202 and CA2000 warnings in VS2010 which I cannot account for.  They seem to say that I am calling Dispose more than once on a series of statements.  Here is a sample block which demos the logic.

using (SqlConnection dCon = UTILITY.MakeSQLConnection(KNOWN_SQL_SERVERS.NEXGENAM))
{
     try
     {
               if (dCon != null) gaList = READDB(dCon);
    }
    catch (Exception)
     {
              throw;
     }
     finally
     {
              if ((dCon != null) &&
                   (dCon.State != ConnectionState.Closed)) dCon.Close();
    }
}

I do not see a single dispose in my code.  The warning/errors seem to imply that the Close() statement is a call to dispose.  I am under the impression that it marks the object available for disposal but does not actually call the dispose method.  While technically the Close is not required, I come from the school of "If I open it, I close it".  My understanding indicates that the using block will perform the actual disposal of the "dCon" object.

(The "MakeSQLConnection" creates a SQL connection to the specified server and either returns a open connection or NULL)

Anyone see something I am missing?  While I could supress these error/warning, I would prefer to repair the problem assuming it exists.

For many objects, including SqlConnection, a call to Close() is identical to calling Dispose. In fact, checking Reflector, all Dispose really does is call Close. So yeah, technically, you're disposing it more than once. The manual call to Close is unnecessary.

Off topic, but you don't need the

catch(Exception){throw;}

Herbie

Ion Todirel said:

Dr Herbie said:

*snip*

you don't need the try at all, if you are not handling the exception, also be aware of nested usings:

 

using (Stream stream...)

using (... = new StreamReader(stream))

Oh yes, missed that the whole finally clause is not needed is he doesn't need to close the connection ...

Herbie

I am under the impression that it marks the object available for disposal but does not actually call the dispose method.

So I assume that reading the help for SqlConnection.Close is a last resort option, after asking on forums. To quote MSDN:

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

"I am under the impression that it marks the object available for disposal but does not actually call the dispose method."

That doesn't even make sense. You can't "mark an object available for disposal". Disposal is never done implicitly. Garbage collection is done implicitly, and garbage collection will run the finalizer, which is explicitly tied to Dispose if you're following the disposal pattern correctly, but that's not the same thing. So maybe you meant it "marks the object available for garbage collection", but that's also incorrect. As long as there's a strong "root" reference to the object, it won't be garbage collected, and a method like Close() can't remove any such references.

You say you don't see any calls to Dispose in your code, but like the code analyzer, I see two. One is implicitly done through the use of the using statement, and the other is the call to Close(). Like everyone else has pointed out, it's clearly documented that these are synonymous operations.

Dexter said:

*snip*

I have read the documentation on SqlConnection.  The phrase "If the SqlConnection goes out of scope, it won't be closed" is huge in my eyes.  For that reason alone I explicitly close my SQL connections.  Testing has proven that client resources are eventually freed without a closure however server resources remain open until your server defined client timeout occurs.  Only by explicitly calling Close do both server and client resources get released.  If you skip the close the SQL Server has dozens of open processes for your program.  With a close you have only what you are actually using.

After having written a large number of long running applications (some with run durations above 3 weeks), I know that I cannot rely upon application termination to clean up my allocated objects.  As a result I police my own objects.

I do not like the phrase "functionally equivalent" as you could drive a tri-level bus through that loophole.

lensman said:

Dexter said:

*snip*

I have read the documentation on SqlConnection.  The phrase "If the SqlConnection goes out of scope, it won't be closed" is huge in my eyes.  For that reason alone I explicitly close my SQL connections.  Testing has proven that client resources are eventually freed without a closure however server resources remain open until your server defined client timeout occurs.  Only by explicitly calling Close do both server and client resources get released.  If you skip the close the SQL Server has dozens of open processes for your program.  With a close you have only what you are actually using.

 

After having written a large number of long running applications (some with run durations above 3 weeks), I know that I cannot rely upon application termination to clean up my allocated objects.  As a result I police my own objects.

 

I do not like the phrase "functionally equivalent" as you could drive a tri-level bus through that loophole.

 

The using statement will take care of cleaning up the connection by calling dispose on it once it's out of scope. Your try/catch/finally is not needed.

lensman said:

Dexter said:

*snip*

I have read the documentation on SqlConnection.  The phrase "If the SqlConnection goes out of scope, it won't be closed" is huge in my eyes.  For that reason alone I explicitly close my SQL connections.  Testing has proven that client resources are eventually freed without a closure however server resources remain open until your server defined client timeout occurs.  Only by explicitly calling Close do both server and client resources get released.  If you skip the close the SQL Server has dozens of open processes for your program.  With a close you have only what you are actually using.

 

After having written a large number of long running applications (some with run durations above 3 weeks), I know that I cannot rely upon application termination to clean up my allocated objects.  As a result I police my own objects.

 

I do not like the phrase "functionally equivalent" as you could drive a tri-level bus through that loophole.

 

After having written a large number of long running applications (some with run durations above 3 weeks), I know that I cannot rely upon application termination to clean up my allocated objects.  As a result I police my own objects.

You know, it's scary that you had to write a large number of long running applications to find out that particular detail. More exactly to find out that objects that implement IDispose must be disposed when they're no longer needed.

Dexter said:

lensman said:

*snip*

 

You know, it's scary that you had to write a large number of long running applications to find out that particular detail. More exactly to find out that objects that implement IDispose must be disposed when they're no longer needed.

 

Actually the quanity of long running programs is not the issue. 

When a person has to write a program which will have a run continiously for weeks you must be very careful you manage all objects you create.  Without this extra effort you will see memory consumption silently grow over time.  I do not subscribe to the some programmers seem to use:  Variable clean up occurs at program exit.

I have been programming long enough to realize that documentation is a vendors view of things as they should be.  Often reality is differant.  For example: In the early  80's I found a 256 byte allocation error in the standard libary, used by most C and Pascal compiliers of the day from multiple vendors, to open files.  When you open over 38000 files that error added up quickly.  Once found the bug was quickly patched.  Since then I have been vary wary of what is written versus what is in fact happening.

Now my original question was concerning two warnings I am getting in VS2010.  Their written text does not match the reality of what is occuring.  So far everyone has pointed out that based upon documentation my actions should not be necessary.   We can argue back and forth on these points however without the steps I noted the connection to the database will not be properly closed on both the client and server.  My concern over resources extends beyond the client.  As I do not see single dispose in my written code I am concerned over the text of the warnings. I strive for zero warnings in my compiled code.

Obviously I can ignore them however they are annoying. 

lensman said:

Dexter said:

*snip*

Actually the quanity of long running programs is not the issue. 

 

When a person has to write a program which will have a run continiously for weeks you must be very careful you manage all objects you create.  Without this extra effort you will see memory consumption silently grow over time.  I do not subscribe to the some programmers seem to use:  Variable clean up occurs at program exit.

 

I have been programming long enough to realize that documentation is a vendors view of things as they should be.  Often reality is differant.  For example: In the early  80's I found a 256 byte allocation error in the standard libary, used by most C and Pascal compiliers of the day from multiple vendors, to open files.  When you open over 38000 files that error added up quickly.  Once found the bug was quickly patched.  Since then I have been vary wary of what is written versus what is in fact happening.

 

Now my original question was concerning two warnings I am getting in VS2010.  Their written text does not match the reality of what is occuring.  So far everyone has pointed out that based upon documentation my actions should not be necessary.   We can argue back and forth on these points however without the steps I noted the connection to the database will not be properly closed on both the client and server.  My concern over resources extends beyond the client.  As I do not see single dispose in my written code I am concerned over the text of the warnings. I strive for zero warnings in my compiled code.

 

Obviously I can ignore them however they are annoying. 

 

 

I see no correlation between a memory allocation error and vendor documentation. Did they document that there were no bugs?

There's a difference between being wary and being paranoid. Your code shows signs of paranoia. It's also unfounded. The documentation is quite correct here, and is easily verifiable.

The warning text does match reality. You were Disposing the object twice. Once from the using statement and once from the call to Close. Several people, myself included, pointed this out. This isn't a comment about how your actions should not be necessary. That's a side comment that results from the main comment. And it's not "should not" be necessary, but "is not" necessary. Again, it's verifiable even if you don't want to trust the documentation (though how you can get by in this industry if you never trust the documentation is beyond me). BTW, warnings are just a form of documentation (especially those coming from Code Analysis), so maybe you shouldn't trust them either and forget about striving for zero warnings?

wkempf said:

lensman said:

*snip*

I see no correlation between a memory allocation error and vendor documentation. Did they document that there were no bugs?

 

There's a difference between being wary and being paranoid. Your code shows signs of paranoia. It's also unfounded. The documentation is quite correct here, and is easily verifiable.

 

The warning text does match reality. You were Disposing the object twice. Once from the using statement and once from the call to Close. Several people, myself included, pointed this out. This isn't a comment about how your actions should not be necessary. That's a side comment that results from the main comment. And it's not "should not" be necessary, but "is not" necessary. Again, it's verifiable even if you don't want to trust the documentation (though how you can get by in this industry if you never trust the documentation is beyond me). BTW, warnings are just a form of documentation (especially those coming from Code Analysis), so maybe you shouldn't trust them either and forget about striving for zero warnings?

I do find this warning (which is new in VS2010) a bit annoying. Personally, I like to dispose everything that's disposable, even in cases like this:

using( FileStream f = File.OpenRead("Foo.txt") )
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

In this case you get warning CA2022 that 'f' is disposed twice, which is true, because the BinaryReader.Dispose method will also dispose the underlying stream. But if I write the code like this:

FileStream f = File.OpenRead("Foo.txt");
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

Now you get warning CA2000 that you should call Dispose on 'f'! What are you supposed to do here!? It's a catch-22 if ever I saw one.

What's more, the warning text for CA2022 says that it's "to avoid generating a System.ObjectDisposedException". However, I've never seen an object that actually throws an ObjectDisposedException when you dispose it more than once. They'll throw that exception if you do anything else after disposing it, but not when disposing it twice. So I've got to wonder what the idea behind this warning was.

Sven Groot said:

wkempf said:

*snip*

I do find this warning (which is new in VS2010) a bit annoying. Personally, I like to dispose everything that's disposable, even in cases like this:

using( FileStream f = File.OpenRead("Foo.txt") )
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

 

In this case you get warning CA2022 that 'f' is disposed twice, which is true, because the BinaryReader.Dispose method will also dispose the underlying stream. But if I write the code like this:

 

FileStream f = File.OpenRead("Foo.txt");
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

 

Now you get warning CA2000 that you should call Dispose on 'f'! What are you supposed to do here!? It's a catch-22 if ever I saw one.

 

What's more, the warning text for CA2022 says that it's "to avoid generating a System.ObjectDisposedException". However, I've never seen an object that actually throws an ObjectDisposedException when you dispose it more than once. They'll throw that exception if you do anything else after disposing it, but not when disposing it twice. So I've got to wonder what the idea behind this warning was.

What's more, the warning text for CA2022 says that it's "to avoid generating a System.ObjectDisposedException".

Hmm, I'd say that the warning (or at least its message text) is a bit paranoid. The docs specifically say that Dispose should not throw exceptions and like you, I've never seen a case where Dispose throws.

But hey, most of the warnings raised by Code Analysis are a bit paranoid, nothing new here

Ion Todirel said:

Sven Groot said:

*snip*

That's true, there's some protection in the Dispose implementation of those, but hardly enough. What I usually do:

 

using (FileStream f = File.OpenRead("Foo.txt"))

{   

    BinaryReader reader = new BinaryReader(f);
}

But I need to flush the reader after I'm done with it. EDIT: if I'd use a BinaryWriter I mean.

 

Just don't fanatically rely on Code Analysis, there are some instances where it does make mistakes.

But then you're relying on the fact that the only thing BinaryReader.Dispose does is call the stream's Dispose method. If it does anything else, your code isn't correct.

Sven Groot said:

wkempf said:

*snip*

I do find this warning (which is new in VS2010) a bit annoying. Personally, I like to dispose everything that's disposable, even in cases like this:

using( FileStream f = File.OpenRead("Foo.txt") )
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

 

In this case you get warning CA2022 that 'f' is disposed twice, which is true, because the BinaryReader.Dispose method will also dispose the underlying stream. But if I write the code like this:

 

FileStream f = File.OpenRead("Foo.txt");
using( BinaryReader reader = new BinaryReader(f) )
{
     // Do stuff 
}

 

Now you get warning CA2000 that you should call Dispose on 'f'! What are you supposed to do here!? It's a catch-22 if ever I saw one.

 

What's more, the warning text for CA2022 says that it's "to avoid generating a System.ObjectDisposedException". However, I've never seen an object that actually throws an ObjectDisposedException when you dispose it more than once. They'll throw that exception if you do anything else after disposing it, but not when disposing it twice. So I've got to wonder what the idea behind this warning was.

In my fresh install of VS 2010 I don't get any warnings in either case (with "using" on the filestream or without).  tried it in both C# and VB.  Neither complained. hmm.....