There is already an open DataReader associated with this Command which must be closed first - AGAIN

Question

I have a client running a web site portal which uses some legacy code that is periodically causing an issue. The system will work for days even weeks and then the worker process will die and no longer serve any data and you have to perform an IISRESET to get it working again

I have found numerous postings about this error and in my mind none of the solutions or explanations fit my code.

Here is the method in question that causes my error

    /// <summary>
    /// Returns Data from Database using Table Name using a field list (if supplied)
    /// Otherwise will return all fields.
    /// </summary>
    /// <param name="TableName">The TableName rquired</param>
    /// <param name="WHERE">Where clause if required</param>
    /// <param name="FieldNames">String array of required field names (if any)</param>
    /// <returns>Dictionary List of results.</returns>
    public List<Dictionary<string, object>> Select(string TableName, string WHERE, string[] FieldNames, int TopRecords = -1, string OrderBy = null)
    {
        string query = string.Empty;
        string sFieldNames = string.Empty;
        if (FieldNames.Length > 0)
        {
            sFieldNames = string.Join(", ", FieldNames);
            query = string.Format("SELECT {2}{0} FROM {1} ", sFieldNames, TableName, TopRecords > -1 ? "TOP (" + TopRecords + ") " : "");
            if (!string.IsNullOrEmpty(WHERE))
            {
                query += string.Format(" WHERE {0}", WHERE);
            }
        }
        else
        {
            // Select ALL fields
            query = string.Format("SELECT {1}* FROM {0} ", TableName, TopRecords > -1 ? "TOP (" + TopRecords + ") " : "");
            if (!string.IsNullOrEmpty(WHERE))
            {
                query += string.Format(" WHERE {0}", WHERE);
            }
        }
        if (!string.IsNullOrEmpty(OrderBy))
        {
            query += " ORDER BY " + OrderBy;
        }

        //Open connection
        if (this.OpenConnection() == true)
        {
            //System.Diagnostics.Debug.WriteLine( "SQL : " + query );
            //Create Command
            using (SqlCommand cmd = new SqlCommand(query, DBConnection))
            {
                //Create a data reader and Execute the command                    
                //Read the data and store them in the list 
                List<Dictionary<string, object>> ResultsSet = null;//Create a list to store the result
                using (SqlDataReader dataReader = cmd.ExecuteReader())
                {
                    ResultsSet = new List<Dictionary<string, object>>();
                    while (dataReader.Read())
                    {
                        Dictionary<string, object> ROW = new Dictionary<string, object>();
                        for (int i = 0; i < dataReader.FieldCount; i++)
                        {
                            if (dataReader[i].GetType().ToString() == "System.Byte[]")
                            {
                                ROW.Add(dataReader.GetName(i), (byte[])dataReader[i]);
                            }
                            else
                            {
                                ROW.Add(dataReader.GetName(i), dataReader[i] + string.Empty);
                            }
                        }
                        ResultsSet.Add(ROW);
                    }
                    dataReader.Close(); //close Data Reader
                    cmd.Dispose(); // Only added today - have to wait for some time to see if it fails
                }                  
                return ResultsSet;
            }
        }
        else
        {
            return null;
        }
    }

Many solutions state you cannot re-use a connection to perform updates but this method does not. I am pretty sure its obvious and it is only fetching the data from the database and no updates are performed.

I don't want to use MARS unless I have absolutely no choice.

Looking for pointers as to what I might have missed

Connection string

<add name="APP.Properties.Settings.DB" connectionString="server=trs-app;User Id=username;password=xxx;Persist Security Info=False;database=TRailS;Pooling=True" providerName="System.Data.SqlClient"/>

OpenConnection Method

        //open connection to database
    public bool OpenConnection()
    {
        try
        {                
            if (DBConnection.State != ConnectionState.Open)
            {
                while (DBConnection.State == ConnectionState.Connecting)
                {
                    // Do Nothing
                }
                DBConnection.Open();
                System.Diagnostics.Debug.WriteLine("SQL Connection Opened");
            }
            return true;
        }
        catch (SqlException ex)
        {
            switch (ex.Number)
            {
                case 0:
                    LastError = "Cannot connect to server.  Contact administrator";
                    return false;
                case 1045:
                    LastError = "Invalid username/password, please try again";
                    return false;
            }
            LastError = "Unknown Error : " + ex.Message;
            return false;
        }
    }

Just spotted this in the DAL Class - is this the cause!!!

     private static SqlConnection DBConnection;

Solution might be to remove static from Sqlconnection variable (DBConnection) and implement the IDisposable pattern in the DAL class as suggested

    protected virtual void Dispose(bool disposing)
    {
        if (disposing == true)
        {
            DBConnection.Close(); // call close here to close connection
        }
    }

    ~MSSQLConnector()
    {
        Dispose(false);
    }

Answer 1

After reading all the comments and reviewing the code in light of the information provided I have refactored the DAL class to ensure every method used in the class is now set to a using statement to create the connection. I understand IIS will handle the connection pool for this

I am also closing the db connection in code too (I know its not required with a using statement but its just for neatness.

I have a small monitoring application that will periodically refresh the login page of the portal to watch for an outage and log then perform an IISReset so I can monitor if the problem goes away after all.

Thanks for all the input.

         //Open connection
        using (SqlConnection DBConnection = new SqlConnection(connectionString))
        {
            DBConnection.Open();
            //System.Diagnostics.Debug.WriteLine( "SQL : " + query );
            //Create Command
            using (SqlCommand cmd = new SqlCommand(query, DBConnection))
            {
                //Create a data reader and Execute the command                    
                //Read the data and store them in the list 
                List<Dictionary<string, object>> ResultsSet = null;//Create a list to store the result
                using (SqlDataReader dataReader = cmd.ExecuteReader())
                {
                    ResultsSet = new List<Dictionary<string, object>>();
                    while (dataReader.Read())
                    {
                        Dictionary<string, object> ROW = new Dictionary<string, object>();
                        for (int i = 0; i < dataReader.FieldCount; i++)
                        {
                            if (dataReader[i].GetType().ToString() == "System.Byte[]")
                            {
                                ROW.Add(dataReader.GetName(i), (byte[])dataReader[i]);
                            }
                            else
                            {
                                ROW.Add(dataReader.GetName(i), dataReader[i] + string.Empty);
                            }
                        }
                        ResultsSet.Add(ROW);
                    }

                }
                DBConnection.Close();
                return ResultsSet;
            }
        }           
    }

Answer 2

The code you have here ... while not perfect (see Zohar's comment - it is actively dangerous, in fact), does treat the SqlDataReader correctly - it is making use of using, etc. So: if this is throwing this error, there are three possibilities:

1. of the code we can see, one of the operations inside the while (dataReader.Read()) has side-effects that is causing another SQL operation to be executed on the same connection; frankly I suspect this is unlikely based on the code shown
2. there is code that we can't see that already has an open reader before this method is called
   • this could be because some code higher in the call-stack is doing another similar while (dataReader.Read()) {...} - typical in "N+1" scenarios
   • or it could be because something that happened earlier (but no-longer in the same call-stack) executed a query, and left the reader dangling
3. your this.OpenConnection() is sharing a connection between different call contexts without any consideration of what is going on (a textbook example of this would be a static connection, or a connection on some kind of "provider" that is shared between multiple call contexts)

2 and 3 are the most likely options. Unfortunately, diagnosing and fixing that requires a lot of code that we can't see.