Connection to SQL is not closing after call to DB in ASP.NET

Question

I have a generic method to call a stored Procedure in ASP.NET:

public SqlDataReader ExecuteStoredProc(string sprocName, SqlParameter[] SqlP)
        {
            SqlDataReader iReader;
            SqlCommand sql = new SqlCommand();

            sql.CommandText = sprocName;
            sql.CommandType = CommandType.StoredProcedure;
            sql.Connection = ConnStr;
            if (SqlP != null)
            {
                foreach (SqlParameter p in SqlP)
                {
                    sql.Parameters.Add(p);
                }

            }
            sql.Connection.Open();
            iReader = sql.ExecuteReader(CommandBehavior.CloseConnection);
            sql.Dispose();

            return iReader;
        }

Even though I am calling CommandBehavior.CloseConnection the connection is not closing. I can get the data fine the first time I request a page. On reload I get the following error:

The connection was not closed. The connection's current state is open. Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.InvalidOperationException: The connection was not closed. The connection's current state is open.

Source Error:

Line 35: Line 36: } Line 37: sql.Connection.Open(); Line 38: iReader = sql.ExecuteReader(CommandBehavior.CloseConnection); Line 39: sql.Dispose();

Finally if I put sql.Connection.Close(); before sql.Dispose(); I get an error that iReader is not readable because it's been closed already.

Obviously I am closing my connection incorrectly, can someone point me in the right direction?

Answer 1

I would suggest wrap the sql connection with a "using" statement, and that will take care of most sql connection issue.

using (var conn = new SqlConnection("..."))
{
    conn.Open();
    using (var cmd = conn.CreateCommand())
    {
        cmd.CommandText = "...";
        using (var reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                // ...
            }
        }
    }

}

Answer 2

When you return a DataReader, the underlying connection must remain open. It's the consumer's responsibility to properly clean up resources.

public SqlDataReader ExecuteStoredProc(string sprocName, SqlParameter[] SqlP)
{
    SqlCommand sql = new SqlCommand();

    sql.CommandText = sprocName;
    sql.CommandType = CommandType.StoredProcedure;
    sql.Connection = ConnStr;
    if (SqlP != null)
    {
        foreach (SqlParameter p in SqlP)
        {
            sql.Parameters.Add(p);
        }

    }
    sql.Connection.Open();
    return sql.ExecuteReader(CommandBehavior.CloseConnection);          
}

public void ConsumingMethod()
{
    using(SqlDataReader reader = ExecuteStoredProc("MyProc", params))
    {
        while(reader.Read())
        {
            //work with your reader
        }
    }
}

Answer 3

The idea is to do a Connection.Close(); after you've finished with the SqlReader, so basically instead of placing the close() statement before the SqlReader.Dispose() command, you should place it below.

Answer 4

This is my preferred way to process IDataReader. Let the caller creates SqlConnection instance and passes to methods.

It's expensive to create SqlConnection instance. And you’ll end up code call the same ExecuteStoredProc method multiple times in different situation.

Thus, I refactor ExecuteStoredProc method by adding SqlConnection instance as part of the parameter.

using (SqlConnection conn = new SqlConnection())
{
    conn.ConnectionString = // Connection String;
    conn.Open();

    using (IDataReader reader = foo.ExecuteStoredProc(conn, sprocName, SqlP))
    {
     // Process IDataReader
    }
}