Is SqlCommand.Dispose() required if associated SqlConnection will be disposed?

Question

I usually use code like this:

using (var connection = new SqlConnection(ConfigurationManager.ConnectionStrings["MyConn"].ConnectionString))
{
   var command = connection.CreateCommand();
   command.CommandText = "...";
   connection.Open();
   command.ExecuteNonQuery();
}

Will my command automatically disposed? Or not and I have to wrap it into using block? Is it required to dispose SqlCommand?

Solution 1

Just do this:

using(var connection = new SqlConnection(ConfigurationManager.ConnectionStrings["MyConn"].ConnectionString))
using(var command = connection.CreateCommand())
{
   command.CommandText = "...";
   connection.Open();
   command.ExecuteNonQuery();
}

Not calling dispose on the command won't do anything too bad. However, calling Dispose on it will suppress the call to the finalizer, making calling dispose a performance enhancement.

Solution 2

The safest policy is to always call Dispose() on an object if it implements IDisposable, either explicitly or via a using block. There may be cases where it is not required but calling it anyway should never cause problems (if the class is written correctly). Also, you never know when an implementation may change meaning that where the call was previously not required it is now definitely required.

In the example you've given, you can add an extra inner using block for the command, as well as maintaining the outer using block for the connection.

Solution 3

Yes, you should, even if it the implementation is currently not doing much, you don't know how it is going to be changed in the future (newer framework versions for instance). In general, you should dispose all objects which implement IDisposable to be on the safe side.

However, if the operation is deferred and you don't control the full scope (for instance when working asynchroneously, or when returning an SqlDataReader or so), you can set the CommandBehavior to CloseConnection so that as soon as the reader is done, the connection is properly closed/disposed for you.

Solution 4

In practice, you can skip Dispose. It doesn't free any resources. It doesn't even suppress finalization since the SQLCommand constructor does that.

In theory, Microsoft could change the implementation to hold an unmanaged resource, but I would hope they'd come out with an API that gets rid of the Component base class long before they'd do that.

Solution 5

You can find out this kind of stuff using Reflector or dotPeek or https://referencesource.microsoft.com/.

I had a small dig (I would suggest that you dig yourself though to be fully sure of the rest of this though as I didn't try that hard) and it looks like when you kill a connection there is no disposal of any children associated with that connection. Furthermore it doesn't actually look like the disposal of a command actually does that much. It will set a field to null, detach itself from a container (this may prevent a managed memory leak) and raise an event (this might be important but I can't see who is listening to this event).

Either way it's good practice to use this stuff in a using block or to ensure you dispose of it using a dispose pattern in the object that holds the connection (if you intend to hold onto the command for a while).

View more solutions

Comments

[Can Sahin]

"Not calling dispose on the command won't do anything too bad." True, but don't get used to it; it's only true for SqlCommand s. On the other hand, not disposing a SqlCeCommand, for example, will cause your mobile device to run out of memory quite fast. (Just been there, done that...)

[The Dag]

Yes, the safest policy is to always dispose disposable objects - notably if you created it! It's not a good idea for example to write a method that accepts a stream, and dispose the stream inside that method. Another caveat is that you can't always dispose via the using statement with impunity. WCF proxies are the only practical example I know of. If something goes wrong at the remote end and you get an exception the channel closes and Dispose then throws a new exception, replacing the original exception, which can be a serious problem.

[Edward Brey]

Dispose doesn't suppress finalization since the constructor does that.

[abatishchev]

You don't need any of this code if you have using: compilter will generate the try/catch/dispose() for you.

[Tahir Alvi]

@abatishchev, yes you are right, but it is always a good practice to catch any exception and logged it properly.

[Serg]

@EdwardBrey which constructor?

[Edward Brey]

@Sergey The SQLConnection constructor.

[Serg]

@EdwardBrey it's done only in the private constructor SqlConnection(SqlConnection connection) which can be called only from the Clone() method. But none of the two normal public constructors does call GC.SuppressFinalize, only the Close() method does. So I guess you're wrong.

[Edward Brey]

@Sergey Sorry. I linked to the wrong constructor. It's the SQLCommand constructor that matters, and it does always call GC.SuppressFinalize.

[Serg]

@EdwardBrey right, and I should have been more careful, too. Ok, in this version it suppresses the finalizer in advance (which is quite unusual for the IDisposable pattern) but they can change it in future versions, so, we'd better follow the protocol either way, especially since SqlCommand has the Dispose method that does some work.

[Edward Brey]

Maybe SqlCommand.Dispose contains a bug.