Is it correct to not dispose of SqlCommand like the example in Microsoft documentation?

Question

The Microsoft Documentation for the SqlCommand class gives the following example code which does not dispose of the SqlCommand.

SqlCommand inherits from DbCommand which implements IDisposable.

Common knowledge would suggest it's advisable to dispose of the SqlCommand, however, the example does not.

Is the example providing a best practice?

private static void ReadOrderData(string connectionString)
{
    string queryString =
        "SELECT OrderID, CustomerID FROM dbo.Orders;";
    using (SqlConnection connection = new SqlConnection(
               connectionString))
    {
        SqlCommand command = new SqlCommand(
            queryString, connection);
        connection.Open();
        using(SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                Console.WriteLine(String.Format("{0}, {1}",
                    reader[0], reader[1]));
            }
        }
    }
}

*"Is there any downside to this approach?"* Probably. There is an upside and downside to just about any design decision. I suggest asking a more specific question — Nigel, Jun 09 '22 at 22:54
Downside is that they decide to add something internal that *needs* disposing (although unlikely given the amount of code without `using` in the wild. Upside is a tiny performance boost. — Charlieface, Jun 09 '22 at 23:19
As you can see from here https://github.com/dotnet/SqlClient/blob/966fc6eab94e193aa893d960dfc8dc0dd2236029/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs#L1061 it doesn't do much, although it used to not do anything at all — Charlieface, Jun 09 '22 at 23:26
@Charlieface The [Microsoft version](https://referencesource.microsoft.com/System.Data/R/53ad9885e5a8fc48.html) hints at older versions needing it. — LarsTech, Jun 09 '22 at 23:31
Thinking about the tiny perf boost, you actually lose out because `SuppressFinalize` is not called. @LarsTech That's interesting, and basically proves my point: they could put that in again or similar — Charlieface, Jun 09 '22 at 23:31
Does this answer your question? [Is SqlCommand.Dispose() required if associated SqlConnection will be disposed?](https://stackoverflow.com/questions/1808036/is-sqlcommand-dispose-required-if-associated-sqlconnection-will-be-disposed) — Charlieface, Jun 09 '22 at 23:32

T.S. · Accepted Answer · 2022-06-10T04:12:26.527

See my comments in your code first

private static void ReadOrderData(string connectionString)
{
    string queryString = "SELECT OrderID, CustomerID FROM dbo.Orders;";
    using (SqlConnection connection = new SqlConnection(connectionString))
    {
        SqlCommand command = new SqlCommand(queryString, connection);
        connection.Open();
        using(SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                Console.WriteLine(String.Format("{0}, {1}", reader[0], reader[1]));
            }
        } // -- disposes reader and closes connection
    } // -- disposes connection and closes it 
}

command was declared in the scope of connection using, i.e. try/catch/finally Command is not necessarily holding any resources outside of connection. And you disposing connection, which already closed when reader is disposed. (see CommandBerhavior). So, in the end, command is the object that holds references to disposed resources.

To know better, use some reflection tool and look inside.

protected override void Dispose(bool disposing)
{
    if (disposing)
        this._cachedMetaData = (_SqlMetaDataSet) null;
    base.Dispose(disposing);
}

So, what is this metadata? Probably some Parameter info. But if you don't add parameters then you might have nothing to dispose. Microsoft knows this and found unnecessary to include dispose on command. This is to answer, why Microsoft did not include using for command.

Look at the constructor

public SqlCommand()
{
    GC.SuppressFinalize((object) this);
}

Looks like there is nothing to dispose in the first place. And all those "cached items" will be garbage collected normally. I did not find there any IO or other unmanaged resources anywhere there.

One thing however, to remember, is that implementation might change from version to version and you don't want to change the code. So, add another layer to command

private static void ReadOrderData(string connectionString)
{
    string queryString = "SELECT OrderID, CustomerID FROM dbo.Orders;";

    using (SqlConnection connection = new SqlConnection(connectionString))
    using (SqlCommand command = new SqlCommand(queryString, connection))
    { 
       . . . . . . 
    }   
}

This will potentially save some hustle.

Fun fact: When working with MySql provider which implements same interfaces and base classes as SqlClient, there was a bug (still is probably) in the MySqlCommand.Dispose which closed the reader always. And this same code with using would work with SqlClient and Oracle ODP but not MySql. I had to wrap MySqlCommand and use my custom class (think of Decorator pattern) in the provider-agnostic code to overwrite the behavior.

Is it correct to not dispose of SqlCommand like the example in Microsoft documentation?

1 Answers1