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?

Corbin March · Accepted Answer · 2009-06-03 20:13:06Z

4

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
        }
    }
}

answered Jun 3, 2009 at 20:13

Corbin March

25.8k6 gold badges77 silver badges100 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

J.W. Over a year ago

I agree with your approach "the consumer's responsibility to properly clean up resources.", but it's generally not to rely on that, the consumer is too lazy to do this.

RedWolves Over a year ago

Thanks this helped. I added a iReader.Close() to my consumingmethod after my while loop.

Roman Starkov Over a year ago

Don't you need to close and dispose of that connection you're creating in ExecuteStoredProc?

Corbin March Over a year ago

@romkyns: Since the Reader was created with CommandBehavior.CloseConnection, the Connection will be closed when the ConsumingMethod's Reader is closed by its using statement: msdn.microsoft.com/en-us/library/…. It feels dangerous since we have to trust a consumer to do the right thing, but it's the only solution if a Reader leaves method scope.

J.W. · Accepted Answer · 2009-06-03 20:11:54Z

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())
            {
                // ...
            }
        }
    }

}

answered Jun 3, 2009 at 20:11

J.W.

18.2k7 gold badges47 silver badges76 bronze badges

1 Comment

J.W. Over a year ago

Also, I won't suggest return SqlDataReader , it's best to initialize your custom object, and then return your object.

mtranda · Accepted Answer · 2009-06-03 20:28:03Z

0

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.

answered Jun 3, 2009 at 20:28

mtranda

1911 gold badge3 silver badges11 bronze badges

Comments

canonw · Accepted Answer · 2009-06-03 21:00:11Z

0

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
    }
}

answered Jun 3, 2009 at 21:00

canonw

Collectives™ on Stack Overflow

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

4 Answers 4

4 Comments

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related