Code Analysis tool

Question

I am using Visual Studio's code analysis tool, and one of the warnings it gives me is "Do not dispose objects multiple times: Object 'conn' can be disposed more than once in method 'CycleMessages.discernScan_Reprint()'. To avoid generating a System.OjectDisposedEx you should not call Dispose more than one time on an object. Line:61

    private void discernScan_Reprint()
    {
        try
        {
            DataTable dt = new DataTable();

            SqlConnection conn = new SqlConnection("my constring");
            using (conn)
            {
                SqlCommand cmd = new SqlCommand("usp_myproc", conn);
                cmd.CommandType = CommandType.StoredProcedure;
                using(cmd)
                {
                    cmd.Parameters.Add(new SqlParameter("@param", param));
                    conn.Open();
                    SqlDataReader dr = cmd.ExecuteReader();
                    dt.Load(dr);
                    conn.Close();  // this is line 61
                }
            }

            switch(dt.Rows.Count)
            {
                default:
                    throw new Exception("message");
                case 1:
                    //do Stuff
                case 0:
                    //do stuff
                    break;
            }

        }
        catch (Exception ex) {throw;}
    }

I don't dispose the conn (explicitly via conn.Dispose();), I just close it and allow the using encapsulation to dipose of the conn object- I know I can allow it to be closed via the disposal, but why is it saying I'm disposing it twice? If anything it should warn me saying "You don't need to terminate connections on objects that will be disposed" or something like that. Am I missing something?

Edits: From ref link on close...

The Close method rolls back any pending transactions. It then releases the connection to the connection pool, or closes the connection if connection pooling is disabled.

and

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. If the connection pooling value Pooling is set to true or yes, the underlying connection is returned back to the connection pool. On the other hand, if Pooling is set to false or no, the underlying connection to the server is closed.

I know that functionally Close() is the same as dispose, but that is not literal from my understanding. When I close the object it is not disposed. It is either closed or returned to the connection pool, disposing (again from my understanding) internally calls the close() methods- so while redundant, I still am baffled as to why it is explicitly saying it is already disposed, when it's not.

As a side remark, I think you should better close your connection into a finally statement to be sure it will indeed be closed. — Laurent S.
I'm surprised the tool didn't also point out that completely unnecessary try/catch. — David
@Bartdude that is bad advice, and completely unnecessary; the using will do that all by itself; if anything: simply remove the .Close() — Marc Gravell
The Using statement disposes the object at the end, which calls this.Close() in your SqlConnection. Check that out for more info on the using statement (msdn.microsoft.com/en-us/library/yh598w02.aspx) — IEatBagels
@MarcGravell > My bad ! I had overlooked that the using statement will indeed be doing that... — Laurent S.

antiduh antiduh · Accepted Answer · 2014-07-02T13:25:13

The Dispose pattern suggests that implementors provide synonyms for Dispose that make sense in the context of the object. One such synonym on SqlConnection is Close():

Close and Dispose are functionally equivalent.

Since you're explicitly calling Close(), and the object's Dispose() method is being called when the connection's using statement ends, you're effectively calling Dispose() twice.

The best approach is to just let the using block handle it for you, since it guarantees that Dispose() is called even when an exception occurs from inside the using block. It also sets the variable to null so that it can be GC'd as soon as possible.

Edits to respond to @alykin's questions

The documentation says that the Close() and Dispose() methods are functionally equivalent, but @alykin has identified a scenario where they don't actually do the same thing. If I'm reading her comment correctly, it works something like this:

The following works:

SqlConnection conn = GetConnSomehow();
SqlCommand cmd = conn.CreateCommand(); 

// ...

conn.Open();

cmd.ExecuteSomething();

cmd.Close();

// ... time passes ...

conn.Open();

The following doesn't:

SqlConnection conn = GetConnSomehow();
SqlCommand cmd = conn.CreateCommand(); 

using ( conn ) {
    cmd.ExecuteSomething();
}

// ... time passes ...

// This won't compile, according to alykins.
conn.Open();

This shows that SqlConnection objects can be reused, at least when they've been only Close()'d.

Likely the reason why the second example with the using block doesn't compile is that the compiler knows that conn has been set to null when the using block ends, so it knows that you can't call methods on a null object reference.

I'm still not sure that this shows that a Dispose() is actually any different than a Close() though, since the incongruity arises due to the nulling semantic of the using block. It would be worthwhile testing whether or not an SqlConnection can be re-opened after it is Dispose()'d but not nulled. Even if it were, I wouldn't rely on that behavior since it is counter to Microsoft's own guidelines set in the Dispose Pattern documentation.

Additionally, I would not use the first block that doesn't use a using block - if an exception occurs, the connection may be leaked, or at least, held open for a non-deterministic amount of time until the GC sees that the object has been leaked and invokes its finalizer.

I would not rely on any difference in behavior between Close() and Dispose() - I would recommend against attempting to re-open a previously-closed SqlConnection object. Let the pooler handle actually keeping the connection alive even if you close or dispose the SqlConnection object you were handed.

A note on using statements.

Consider the following block of code:

IDisposable thing = GetThing();

using ( thing ) {
   thing.DoWork();
}

That block of code is exactly identical to this block:

IDisposable thing = GetThing();

try {
   thing.DoWork();
}
finally {
   thing.Dispose();
   thing = null;
}

And the following block, as considered by Microsoft, their documentation, and their analysis tools, counts as two disposes:

SqlConnection conn = GetConn();

using ( conn ) {
    DoWork(conn);
    conn.Close(); // code analysis tools count this as one Dispose().
} // implicit conn.Dispose() from the using block, so that's two.

Ignore the fact that Close and Dispose don't exactly do the same thing. They don't want you to rely on that, nor should you in case the behavior does actually get fixed.

Code Analysis tool

4 Answers