Application Crash on SQL Connection.Close method

Question

I have a windows services which is performing lot of Data processing. At some point, my service is crashed on closing a SQLConnection. When I comment the Close connection method call, Service is working consistently without crashing.

What could be the problem ? Below is a code snippet

private void DeleteTempTable()
    {
        _logger.Info("DeleteTempTable");

        try
        {

            foreach (KeyValuePair<string, string> item in _stables)
            {                 
                string dropSql = string.Empty;
                dropSql = string.Format("DROP TABLE [{0}];", item.Value);                 


                    SqlConnection oConnDropTables = new SqlConnection(connectionString);
                    oConnDropTables.Open();

                    if (!string.IsNullOrEmpty(dropSql))
                    {
                        using (SqlCommand cmd = new SqlCommand(dropSql, oConnDropTables))
                        {                              
                            cmd.ExecuteNonQuery();                             
                        }
                    }

              if (oConnDropTables != null && oConnDropTables.State == ConnectionState.Open)
                oConnDropTables.Close();                    
                oConnDropTables = null;



            }


        }
        catch (Exception ex)
        {
            _logger.Error("Error " + ex.Message);
            throw ex;
        }

    }

When I comment the Close connection, service is working without crashing. Also it is not caught in the catch block. Also Connection is not Null and connectionstate is open only..

What I have tried: 1) Put "using" construct for connection - Didn't help 2) catching SQLException to check if anything I get- Didn't help

Answer 1

The issue is the creation of a new connection object each time the loop runs. When you close a SQL Connection, it is not actually closed but its returned to the app pool ready to be re-used. There is a limited number of connections you can open in SQL at once.

Try moving the SQLConnection object out of the loop and just execute commands in the loop and close the connection after the loop finishes.

private void DeleteTempTable()
{
    _logger.Info("DeleteTempTable");

    try
    {
        using(SqlConnection oConnDropTables = new SqlConnection(connectionString))
        {
            oConnDropTables.Open();

            foreach (KeyValuePair<string, string> item in _stables)
            {                 
                string dropSql = string.Empty;
                dropSql = string.Format("DROP TABLE [{0}];", item.Value);                 

                if (!string.IsNullOrEmpty(dropSql))
                {
                    using (SqlCommand cmd = new SqlCommand(dropSql, oConnDropTables))
                    {                              
                        cmd.ExecuteNonQuery();                             
                    }
                }
            }
        }

    }
    catch (Exception ex)
    {
        _logger.Error("Error " + ex.Message);
        throw ex;
    }

}

Answer 2

Removing that Close() should not make any problems go away, and frankly I don't believe it has. Since you don't yet understand the problem, it is premature to assume that a random code change has fixed it. Specifically:

• with the Close(), it is returning the connection to the pool each time; when you call Open(), it will get back the same connection from the pool (cleansed, except for a few minor things)
• without the Close(), the previous connection will be left to be garbage collected, which can cause either the connection-pool to become saturated, or the database-server's connection count to saturate; basically - bad things

What I suspect is happening is that you had some random error, that you now aren't seeing, by random. For example, network connectivity, or the unpredictable ordering of a Dictionary<,> (which means you don't know what order the tables are being DROPped, which is very important if there are foreign keys between them).

The only major problem with the current code is that it isn't using using. There are some redundant lines, though. This would be better:

foreach (var item in _stables)
{                 
    var dropSql = string.Format("DROP TABLE [{0}];", item.Value);
    using(var oConnDropTables = new SqlConnection(connectionString))
    using (var cmd = new SqlCommand(dropSql, oConnDropTables))
    {
        oConnDropTables.Open();
        cmd.ExecuteNonQuery();
    }
}

or (preferable):

using(var oConnDropTables = new SqlConnection(connectionString))
{
    oConnDropTables.Open();
    foreach (var item in _stables)
    {                 
        var dropSql = string.Format("DROP TABLE [{0}];", item.Value);
        using (var cmd = new SqlCommand(dropSql, oConnDropTables))
        {
            cmd.ExecuteNonQuery();
        }
    }
}