C# - closing Sql objects best practice

Question

If you have a C# function with Sqlaccess, is it mandatory to close all objects/handles, or is everything cleaned up automatically once you exit the function

For example:

void DoSqlStuff()
{
    SqlConnection sqlConn = new SqlConnection(...);
    SqlCommand cmd = new SqlCommand(...);
    SqlDataReader sqlData= null;

    sqlConn,Open();
    sqlData = cmd.ExecutReader();


    while(sqlData.Read())
    {
         ...
    }
}

Is it optional, recommended or mandatory to close SqlConn and SqlData?

Thanks.

Answer 1

You should close the SqlConnection object as soon as you're done with it. If you don't then the connection will remain open, and will not be available to handle other requests.

The using statement is useful for this. It will call Dispose() on the object for you:

using (SqlConnection cn = new SqlConnection(connectionString))
{   
    SqlCommand cm = new SqlCommand(commandString, cn)
    cn.Open();
    cm.ExecuteNonQuery();       
}

Answer 2

All three classes have a Dispose() method. Mandatory is too strong, but definitely highly recommended you use the using keyword so Dispose() is automatically called. Failing to do so makes your program run "heavy", using more system resources than necessary. And outright failure when you don't use the "new" keyword enough to trigger the garbage collector.

Answer 3

Any class handling SQL stuff like Connections should implement the IDisposable interface as stated by Microsoft .NET coding guidelines.

Thus, you should probably close and dispose your connection in your Dispose method.

Answer 4

You should close everything before returning from the function. Open datareaders mean open cursors on the database, resulting in increased memory usage. Same goes for database connections.

Unused objects are not immediately freed in C#, but only when garbage collection is performed, which is not deterministic.

Answer 5

Calling Close on the SQL connection won't actually close it, but will return it to a connection pool to be reused, improving performance.

Additionally it is generally poor practice to not explicitly dispose of unmanaged resources when you are finished with them (asap).

Answer 6

You don't need to have a separate using statement for the SqlDataReader (as well as one using statement for the connection) unless you plan do perform other operations with the connection after the SqlDataReader has fully read through the row set.

If you are just opening a connection, reading some data using the reader, and then closing the connection, then one using statement for the entire block of code (surrounding the connection) will suffice as the garbage collector will clean up all resources tied to the connection that is disposed by the first using statement.

Anyway, here's a good article that describes it all...

Answer 7

Explicit closing and disposing in the finally statement is another way. It produces a bit more code, but demonstrates exactly what is being cleaned up and ensures everything is cleaned up even in the event of an exception.

More here... http://www.craigweston.ca/?p=9

SqlConnection sqlConn = null;
SqlCommand cmd = null;
SqlDataReader sqlData= null;

try
{
    //new'ing here
    //connection opening, reading ...etc
}
catch(Exception ex)
{
    throw ex;
}
finally
{
    if(cmd != null)
        cmd.Dispose();
    if(conn != null)
    {
        conn.Close();
        conn.Dispose();
    }
    if(sqlData != null)
    {        
        sqlData.Close();
        sqlData.Dispose();
    }
}

Answer 8

Be careful with absolutes here. A lot depends on what you are doing & where the inefficiencies may lie. In a Web Page where each user has a separate security context you may have no choice but to establish a new SQL connection with new security credentials with each page hit. Clearly nicer if you can use a pool of SQL connections with a shared security context & let the Web page filter the results but perhaps you can't.

In early versions of SQL Server ie (v6.5 or less) the Login Authentication was done by SQL Server. Also SQL was severely constrained by connection memory & the number of active connections it could handle. So it was a great idea to drop your connection when not in use. Post v6.5, most people use Windows Authentication to login to SQL. This causes a lot of network calls between servers & some latency. Kerberos Security is even more chatty, Thus establishing a SQL connection is expensive. For that reason you need to find a balance between Holding a connection open for the life of your WinForms application vs Opening & closing it within each method call.

As a rough guide, if you think your app is going to want to talk to SQL in the next, say 30 secs. Keep the established connection open. If they've minimised your app, not touched it within a timeout period, or you've got all the data in RAM & they are unlikely to need anything more from the SQL system. Close the connection.

Consider creating a Class with a System Timer to hold the connection. Your class will always provide a valid connection, but perhaps the class will choose to drop it & free the connection load on SQL when appropriate.

Unless you are also writing Server based code, a small amount of memory inefficiency might not even be noticed. But 2-10,000 clients all poorly using your Security & Data Servers is likely to bring your Data Centre to its knees.