Why call SqlClient.SqlDataReader Close() method anyway?

Question

Is the SqlClient.SqlDataReader a .NET managed object or not? Why do we have to call the Close() method explicitly close an open connection? Shouldn't running out of scope for such an object automatically close this? Shouldn't garbage collector clean it up anyway?

Please help me understand what is the best practise here.

I have seen a related question here and it further illustrates the issue I have with a web application. The issue is that we were running out of connections. The detailed error is here: Exception: System.InvalidOperationException Message: Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached. Source: System.Data

at  System.Data.SqlClient.SqlConnectionPoolManager.GetPooledConnection(SqlConnectionString options, Boolean& isInTransaction)
   at System.Data.SqlClient.SqlConnection.Open()

To fix this, I had to explicitly close all the SQLDataReader objects.

I am using .NET Framework 3.5

Answer 1

Sure, it will be collected when it goes out of scope (if their are no other references to it). When it is collected, it will be closed through its Dispose() method. However, you never really know when the GC is going to deallocate things; if you don't close your readers, you very quickly run out of available connections to the database.

Further Reading

• O'Reilly's ADO.NET Connection Pool Explained
• Microsoft's Retrieving Data using a DataReader has a general overview of DataReaders.

~ William Riley-Land

Answer 2

The 'managed' resource referred to by the term 'managed code' is memory. That's it. Any other scarce resource needs to be wrapped with the disposable pattern, including database connections.

The reason this is a problem for you is that the garbage collector doesn't run for every object the moment it goes out of scope. It's much more efficient to collect more items less frequently. So if you wait for the collector to dispose your objects (and yes, if you implement idisposable it eventually will) you maybe be holding a number of database connections open much longer than you realize.

Answer 3

Also take into consideration what happens when an exception gets thrown - you never know if the connection will be closed if you suddenly are forced out of the executing code.

As a rule in our shop, we explicitly wrap all database calls in a Try...Finally block, with the finally section catching and closing the data connections. It's worth the tiny bit of effort to save yourself a major troubleshooting headache.

Answer 4

@Lieutenant Frost

As a rule in our shop, we explicitly wrap all database calls in a Try...Finally block, with the finally section catching and closing the data connections. It's worth the tiny bit of effort to save yourself a major troubleshooting headache.

I have a similar rule, but I require that objects implementing IDisposable use the 'using' block.

using (SqlConnection conn = new SqlConnection(conStr))
{
     using (SqlCommand command = new SqlCommand())
     {
        // ETC
     } 
}

The using block calls Dispose immediately when leaving the scope, even with an exception.

Answer 5

It's not the Connection that's the problem, but the SQL Cursor being held by the SqlDataReader. If you try to open a second without closing the first, it will throw an exception.

Answer 6

One good practice (as long as you aren't re-using connections) is to add the Command Behavior to the SqlDataReader to close the connection when it gets disposed:

SqlDataReader rdr = cmd.ExecuteReader( CommandBehavior.CloseConnection );

Adding this will ensure that the connection to the database is closed when the SqlDataReader object is closed (or garbage collected).

As I stated before, though, you don't want to do this if you are planning on re-using the database connection for another operation within the same method.

Answer 7

If you don't explicitly close it, then it sits there waiting for the garbage collector to "collect" it... When that happens it gets released back to the ADO.Net Connection pool to be reused by another Connection.Open request, so during all the intervening time, any other requests for a connection will have to create a brand new one, even though there is a perfectly good one sitting there unused that coul;d be used...

Depending on how long it is before the GC runs, and how many Database requests ar ebeing executed, you could run out of available connections to the database.

But there is a optional parameter on the Command.ExecuteReader() method, called CommandBehavior. This is an enumeration, with values: CommandBehavior.Default, CommandBehavior.SingleResult, CommandBehavior.SchemaOnly, CommandBehavior.KeyInfo, CommandBehavior.SingleRow, CommandBehavior.SequentialAccess, and CommandBehavior.CloseConnection

This enumeration has a FlagsAttribute attribute that allows a bitwise combination of its member values. It is the last value, (CommandBehavior.CloseConnection) that is relevant here. It tells the Command object to close the connection when the data reader is closed. http://msdn.microsoft.com/en-us/library/system.data.commandbehavior.aspx

Unfortunately the default is NOT to close the connection when the data reader is closed, but you can, (and should) pass this parameter as CommandBehavior.CloseConnection when you want your method to release the connection back to the pool immediately when you are done with it...

Answer 8

I think everybody else said this but I wanted it to be clear:

Out of scope doesn't mean immediate garbage collection.

Your app needs to 'play nice' on a number of levels. Closing the connections helps you do that. Let's examine a few of those levels.

1: You aren't relying on garbage collection. Ideally garbage collection shouldn't need to exist. But it does. But most assuredly you should not rely on it.

2: You aren't holding your database connections. While connections are usually pooled, as you've found there is a limit. Holding this longer than necessary makes your app the bad apple.

3: You aren't generating network traffic. Every database connection is essentially a TCP connection. Keeping it open probably generates network traffic along the lines of Are you still there? yes. Small traffic yes, but on a crowded network this is bad practice. And SQL Server itself is using resources to keep your connection alive. Resources other people trying to get to that sql server could make better use of.

When thinking about database access you also have to think about network resources. Some ways to get the data are bad because they can bring unnecessary stuff along for the ride. The earlier versions of ADO were kind of notorious for this type of stuff. Bringing back the schema info when you just want the data. Sure, with only a few connections this isn't a problem. But since when does any database have only a few connections. So think about this stuff and try not to abuse the resources.

When your web app only has 100 users you may not care. But what about 100,000? Always consider what happens when you scale.