closing SQLDataReaders - how to tell if they are c

问题:

I'm finding that I'm having some website connection pool issues and I'm in the process of tracking them down. I know one thing to look for is to make sure that any SQLDataReaders get closed and I've gone though and made sure they are. One question that popped up in my head was about methods that return SQLDataReaders and how they get closed (or not).

So here's how I have things setup and some example methods:

public static SqlDataReader ExecuteReader(SqlCommand cmd)
{
    SqlConnection c = new SqlConnection(Properties.Settings.Default.DatabaseConn);
    cmd.Connection = c;
    c.Open();
    return cmd.ExecuteReader(System.Data.CommandBehavior.CloseConnection);
}

then I have a method that uses the 'ExecuteReader()'

public static SqlDataReader GetData()
{
  SqlCommand Query = new SqlCommand("select * from SomeTable");
  return ExecuteReader(Query);
}

Now say I have another method that calls 'GetData'. I simplified things obviously.

public static SqlDataReader GetMoreData()
{
  return GetData;
}

So my question is, when I call 'GetMoreData' like this

SqlDataReader dr = GetMoreData();
//do some stuff with 'dr'
dr.close();

Are all my SqlDataReaders and connections being closed properly?

Thanks!

回答1:

Description

The SqlDataReader implements the IDisposable interface. On every class that implements IDisposable you should call Dispose or use using in order to free resources, in this case close the reader and the underlying connection.

IDisposable Interface Defines a method to release allocated resources.

Sample

using(SqlDataReader dr = GetMoreData()) 
{
    try
    {   
       // do your stuff
    }
    catch(Exception ex)
    {
       // handle the exception
    }
} // the reader will get closed here

or

SqlDataReader dr;
try
{   
    dr = GetMoreData();
    // do your stuff
}
catch(Exception ex)
{
   // handle the exception
}
finally
{
   // close the reader
   dr.Dispose();
}

Edit

Nice comment by JotaBe

but if he implements a method that returns a DataReader, the using should be used int the method's caller. So there's no way to warranty that the DataReader is closed.

I dont recommend to return a SqlDataReader but if you want to do this you need to do this

SqlDataReader reader;
try
{
   reader = methodThatReturnsAReader();
}
catch(Exception ex)
{
   // handle the exception
}
finally
{
   // close the reader
   reader.Dispose();
}

More Information

• MSDN - IDisposable Interface
• MSDN - SqlDataReader Class

回答2:

As long as you're sure that dr.Close() is being called every time (even in the case of an exception being thrown), then yes your connections will also close. However, it's generally good practice to wrap that type of code into a try/finally block, where the finally block contains the dr.Close() statement.

The other alternative is a using statement, which takes advantage of the IDisposable interface implemented by SqlDataReader.

回答3:

I'd recommend you to never return a DataReader from a method. You give the responsibility of closing the DataReader to the method caller. If the method caller doesn't assure that the DataReader is closed, even if an exception happens, you'll be in serious trouble.

Definitely, you should not do it this way.

What is worst, on some circumstances, an open DataReader can create locks in the database.

The only exception would be if the method is private, and you assure that all of the methods callers are closing the DataReader. But it's still quite error-prone.

回答4:

The using statement, wrapped around SqlDataReader dr = GetMoreData() will protect you, as long as it's used in every place where GetMoreData() is called. That's hard to manage, so you can better protect yourself by changing the design.

From Microsoft Patterns and Practices:

"Use a DataSet when the following conditions are true:

-You have to cache or pass the data between layers."

compared with:

"Use a DataReader when the following conditions are true:

-you have a data container such as a business component that you can put the data in."

I would say your application has layers and doesn't appear to use business components. While it's true that DataSets have a much larger overhead than DataReaders, consider this:

• cost of leaked connections (high, unpredictable) vs cost of using DataSets (measurable)
• how much data do you need - can you return DataTable or DataRow instead of a DataSet?

DataReaders are great for low level code, such as Data Access components, but should not be passed around between different parts of your application.