SQL Server connection timeouts and using statements with command and connection objects

Question

Please read entire question before responding. And I apologize, I never seem to write short questions...

I am supporting a C# internal web app that hits SQL Server 2008 R2 running on a Windows Small Business Server 2011 SP1 box.

We have been getting a lot of SQL timeouts lately, here is an example exception:

System.Web.HttpUnhandledException: Exception of type 'System.Web.HttpUnhandledException' was thrown. ---> System.InvalidOperationException: 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. at System.Data.ProviderBase.DbConnectionFactory.GetConnection(DbConnection owningConnection) at System.Data.ProviderBase.DbConnectionClosed.OpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory) at System.Data.SqlClient.SqlConnection.Open()

I have checked a few things, one of them being how the code handles connections and closing of connections. I have read in other threads that using a Using statement with your connection is adequate as it "...wraps the connection create in a try .. finally and places the connection disposal call inside the finally". The connection is closed even in the event of an exception.

So, I agree with and have used that method for years. Others have recommended explicitly closing connections even when using a Using statement with your connection. I think that would be redundant...

My question, however, is regarding the command object. Someone else wrote a large library of db methods for this app and they have (in all of the db methods) declared the SqlCommand object BEFORE the SqlConnection object using statement. They have also assigned the connection object to the command object before the connection using statement.

Is it better practice to declare and use the command object inside the connection using statement, and could doing it the other way cause sql connection timeouts (barring other causes of sql connection timeouts)? Take this code for example:

    public Musician GetMusician(int recordId)
    {
        Musician objMusician = null;
        SqlConnection con = new SqlConnection(_connectionString);
        SqlCommand cmd = new SqlCommand();
        cmd.Connection = con;
        cmd.CommandText = "selectMusician";
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.AddWithValue("@id", recordId);

        using (con)
        {
            con.Open();
            SqlDataReader reader = cmd.ExecuteReader();
            if (reader.HasRows)
            {
                reader.Read();
                objMusician = new Musician((int)reader["id"]);
                objMusician.Name = (string)reader["name"];
            }
        }

        if objMusician != null)
        {
            objMusician.Albums = Albums.GetAlbums((int)objMusician.ID);
            objMusician.Tours = Tours.GetTours((int)objMusician.ID);
            objMusician.Interviews = Interviews.GetInterviews((int)objMusician.ID);
        }
        return objMusician;
    }

Also know that the calling pages have try catches in them and it is the page that logs the error to our logging db. We let the exception bubble up to the calling method on the page, and it gets handled there.

mjw · Accepted Answer · 2016-02-29 20:46:06Z

0

You should explicitly close the connection when you're finished with it. You're never closing any connections so after you hit the connection pool limit you're going to get errors until you manually recycle the pool or it cycles on its own. Move the property assignment block inside the using block and do a con.Close(); cmd.Dispose(); before returning your objMusician:

using (con)
{
    con.Open();
    SqlDataReader reader = cmd.ExecuteReader();
    if (reader.HasRows)
    {
        reader.Read();
        objMusician = new Musician((int)reader["id"]);
        objMusician.Name = (string)reader["name"];
    }

    if objMusician != null)
    {
        objMusician.Albums = Albums.GetAlbums((int)objMusician.ID);
        objMusician.Tours = Tours.GetTours((int)objMusician.ID);
        objMusician.Interviews = Interviews.GetInterviews((int)objMusician.ID);
    }

    con.Close();
    cmd.Dispose();        

    return objMusician;
}

answered Feb 29, 2016 at 20:46

mjw

1,2061 gold badge13 silver badges20 bronze badges

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

4 Comments

Kershaw Over a year ago

The Using statement is supposed to close the connection. Isn't that correct?

Kershaw Over a year ago

Also, the main question is, should the command object be declared and used inside the connection using block? Could not doing that be problematic?

Chris Dunaway Over a year ago

Dispose closes the connection and returns it to the pool.

mjw Over a year ago

No, the command object can be declared inside or outside of the connection block... your problem is you aren't disposing of the objects so you're just leaving them out there in memory and hoping the GC comes along to resolve things before you run in to timeouts. Based on your described behavior, the GC isn't handling things before this occurs.

Kevin · Accepted Answer · 2016-02-29 20:57:41Z

0

Don't know if it will help your timeout problem, but I've always structured my code like the following and not had that problem:

using(var cmd = new SqlCommand())
{
    using(var con = new SqlConnection(ConnectionString))
    {
        con.Open();
        cmd.Connection = con;
        cmd.CommandText = "selectMusician";
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.AddWithValue("@id", recordId);
        ...
    }
}

Was just reading on MSDN, it said "Call Dispose when you are finished using the Component. The Dispose method leaves the Component in an unusable state. After calling Dispose, you must release all references to the Component so the garbage collector can reclaim the memory that the Component was occupying." This means in order for the GC to immediately collect the connection, you must dispose the connection before disposing the command, otherwise the connection hangs around until the GC gets around to calling the Finalize on it.

edited Feb 29, 2016 at 20:57

answered Feb 29, 2016 at 20:47

Kevin

1,4729 silver badges9 bronze badges

2 Comments

Kershaw Over a year ago

But isn't that what the Using statements do? They are supposed to handle disposing for you. Some say explicitly call Dispose anyways, even if you are using a Using statement with your connection? Just hoping to get a definitive answer.

Kevin Over a year ago

Yes, the using statement calls Dispose. My point above is to get them in the right order so that when the connection's using statement disposes the connection, the command (which has a reference to the connection) is disposed after. That way the GC will collect the connection right away rather than waiting until it gets around to calling Finalize on it (which can take a while).

Mike Hofer · Accepted Answer · 2016-02-29 21:05:35Z

0

Refactor your method as follows. You are likely running into a situation where a data reader has a reference to a connection, and it has not yet been disposed of.

public Musician GetMusician(int recordId)
{
    Musician objMusician = null;

    using(SqlConnection con = new SqlConnection(_connectionString))
    {
        con.Open();
        using (SqlCommand cmd = new SqlCommand())
        {
            cmd.Connection = con;
            cmd.CommandText = "selectMusician";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@id", recordId);

            using (SqlDataReader reader = cmd.ExecuteReader(CommandBehavior.CloseConnection))
            {
                if (reader.HasRows)
                {
                    reader.Read();
                    objMusician = new Musician((int) reader["id"]);
                    objMusician.Name = (string) reader["name"];
                }

                if objMusician != null)
                {
                    objMusician.Albums = Albums.GetAlbums((int)objMusician.ID);
                    objMusician.Tours = Tours.GetTours((int)objMusician.ID);
                    objMusician.Interviews = Interviews.GetInterviews((int)objMusician.ID);
                }
            }
        }

        return objMusician;
    }
}

answered Feb 29, 2016 at 21:05

Mike Hofer

17.1k11 gold badges80 silver badges117 bronze badges

3 Comments

Kershaw Over a year ago

I think this is the best structure I've seen yet, Using statements for connection, then command, then reader, and no explicit dispose or close. What does the CommandBehavior.CloseConnection on the ExecuteReader do? Why is it necessary? If good practice then I will use it, but haven't seen that before.

Mike Hofer Over a year ago

It ensures that the connection is closed as soon as you are finished with the reader (that is, when it goes it is closed/disposed of). See here: msdn.microsoft.com/en-us/library/…

Kershaw Over a year ago

Looking at some of my recent code in other projects, I have been using the same structure as above, but without the CommandBehavior.CloseConnection on the ExecuteReader. I think I'll update my structure with it, though, as a new refinement...

Collectives™ on Stack Overflow

SQL Server connection timeouts and using statements with command and connection objects

3 Answers 3

4 Comments

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related