73

I have this legacy code :

 private void conecta()
 {  
     if (conexao.State == ConnectionState.Closed)
         conexao.Open();
 }

 public List<string[]> get_dados_historico_verificacao_email_WEB(string email)
 {
     List<string[]> historicos = new List<string[]>();
     conecta();

     sql = 
         @"SELECT * 
         FROM historico_verificacao_email 
         WHERE nm_email = '" + email + @"' 
         ORDER BY dt_verificacao_email DESC, hr_verificacao_email DESC";

     com = new SqlCommand(sql, conexao);
     SqlDataReader dr = com.ExecuteReader();

     if (dr.HasRows)
     {
         while (dr.Read())
         {
             string[] dados_historico = new string[6];
             dados_historico[0] = dr["nm_email"].ToString();
             dados_historico[1] = dr["dt_verificacao_email"].ToString();
             dados_historico[1] = dados_historico[1].Substring(0, 10);
             dados_historico[2] = dr["hr_verificacao_email"].ToString();
             dados_historico[3] = dr["ds_tipo_verificacao"].ToString();

             sql = 
                 @"SELECT COUNT(e.cd_historico_verificacao_email) QT 
                 FROM emails_lidos e 
                 WHERE e.cd_historico_verificacao_email = 
                     '" + dr["cd_historico_verificacao_email"].ToString() + "'";

             tipo_sql = "seleção";
             conecta();
             com2 = new SqlCommand(sql, conexao);

             SqlDataReader dr3 = com2.ExecuteReader();
             while (dr3.Read())
             {
                 //quantidade de emails lidos naquela verificação
                 dados_historico[4] = dr3["QT"].ToString(); 
             }
             dr3.Close();
             conexao.Close();

             //login
             dados_historico[5] = dr["cd_login_usuario"].ToString();
             historicos.Add(dados_historico);
         }
         dr.Close();
     }
     else
     { 
         dr.Close();
     }

     conexao.Close();
     return historicos;
 }


I have created two separates commands to correct the issue, but it still continues: "There is already an open DataReader associated with this Command which must be closed first".

An additional info: the same code is working in another app.

Zanon
  • 29,231
  • 20
  • 113
  • 126
alejandro carnero
  • 1,774
  • 7
  • 27
  • 43
  • 31
    You have a SQL injection vulnerability. – SLaks Aug 27 '13 at 20:50
  • possible duplicate of [There is already an open DataReader associated with this Command which must be closed first](http://stackoverflow.com/questions/6062192/there-is-already-an-open-datareader-associated-with-this-command-which-must-be-c) – Bob Vale Aug 27 '13 at 20:50
  • 4
    It's not just by command, it's by connection. If you're using the same connection for both commands, you'll get the error. – Michael Todd Aug 27 '13 at 20:52
  • Why don't you put the internal command into an extra function? It enhances readability. – sockfd2 Aug 27 '13 at 20:53

7 Answers7

215

Just add the following in your connection string:

MultipleActiveResultSets=True;
jgauffin
  • 99,844
  • 45
  • 235
  • 372
Ankit
  • 4,755
  • 2
  • 22
  • 14
21
  1. The optimal solution could be to try to transform your solution into a form where you don't need to have two readers open at a time. Ideally it could be a single query. I don't have time to do that now.

  2. If your problem is so special that you really need to have more readers open simultaneously, and your requirements allow not older than SQL Server 2005 DB backend, then the magic word is MARS (Multiple Active Result Sets). http://msdn.microsoft.com/en-us/library/ms345109%28v=SQL.90%29.aspx. Bob Vale's linked topic's solution shows how to enable it: specify MultipleActiveResultSets=true in your connection string. I just tell this as an interesting possibility, but you should rather transform your solution.

    • in order to avoid the mentioned SQL injection possibility, set the parameters to the SQLCommand itself instead of embedding them into the query string. The query string should only contain the references to the parameters what you pass into the SqlCommand.
Csaba Toth
  • 10,021
  • 5
  • 75
  • 121
13

You can get such a problem when you are two different commands on same connection - especially calling the second command in a loop. That is calling the second command for each record returned from the first command. If there are some 10,000 records returned by the first command, this issue will be more likely.

I used to avoid such a scenario by making it as a single command.. The first command returns all the required data and load it into a DataTable.

Note: MARS may be a solution - but it can be risky and many people dislike it.

Reference

  1. What does "A severe error occurred on the current command. The results, if any, should be discarded." SQL Azure error mean?
  2. Linq-To-Sql and MARS woes - A severe error occurred on the current command. The results, if any, should be discarded
  3. Complex GROUP BY on DataTable
Community
  • 1
  • 1
LCJ
  • 22,196
  • 67
  • 260
  • 418
  • 2
    key: if calling foreach() on an IQueryable (you did .Where() but did not follow it with .ToList()), and you then try to use the context within that loop, this can cause the problem. – Don Cheadle Jan 26 '16 at 15:31
  • 1
    Thanks @DonCheadle this was exactly the problem I was experiencing. The ToList() solved the problem! Thank you! – mack Sep 11 '19 at 02:02
8

I suggest creating an additional connection for the second command, would solve it. Try to combine both queries in one query. Create a subquery for the count.

while (dr3.Read())
{
    dados_historico[4] = dr3["QT"].ToString(); //quantidade de emails lidos naquela verificação
}

Why override the same value again and again?

if (dr3.Read())
{
    dados_historico[4] = dr3["QT"].ToString(); //quantidade de emails lidos naquela verificação
}

Would be enough.

jszigeti
  • 373
  • 4
  • 11
Jeroen van Langen
  • 21,446
  • 3
  • 42
  • 57
  • 9
    creating a second connection to same database maybe solves the problem but does not help code readability and maintenance – Mauricio Gracia Gutierrez Aug 27 '13 at 20:56
  • +1. This way he could be even Server vendor independent, although for this he has to use DbConnection instead of SqlConnection and Db* classes isnetad of any Sql* class. But the best is if he rewrites his queries, just as you (and any answer) say. – Csaba Toth Aug 27 '13 at 23:33
8

I bet the problem is being shown in this line

SqlDataReader dr3 = com2.ExecuteReader();

I suggest that you execute the first reader and do a dr.Close(); and the iterate historicos, with another loop, performing the com2.ExecuteReader().

public List<string[]> get_dados_historico_verificacao_email_WEB(string email)
    {

        List<string[]> historicos = new List<string[]>();
        conecta();
        sql = "SELECT * FROM historico_verificacao_email WHERE nm_email = '" + email + "' ORDER BY  dt_verificacao_email DESC, hr_verificacao_email DESC"; 
        com = new SqlCommand(sql, conexao);
        SqlDataReader dr = com.ExecuteReader();

        if (dr.HasRows)
        {
            while (dr.Read())
            {
                string[] dados_historico = new string[6];
                dados_historico[0] = dr["nm_email"].ToString();
                dados_historico[1] = dr["dt_verificacao_email"].ToString();
                dados_historico[1] = dados_historico[1].Substring(0, 10);
                //System.Windows.Forms.MessageBox.Show(dados_historico[1]);
                dados_historico[2] = dr["hr_verificacao_email"].ToString();
                dados_historico[3] = dr["ds_tipo_verificacao"].ToString();
                dados_historico[5] = dr["cd_login_usuario"].ToString();
                historicos.Add(dados_historico);
            }

            dr.Close();

            sql = "SELECT COUNT(e.cd_historico_verificacao_email) QT FROM emails_lidos e WHERE e.cd_historico_verificacao_email = '" + dr["cd_historico_verificacao_email"].ToString() + "'";
            tipo_sql = "seleção";
            com2 = new SqlCommand(sql, conexao);

            for(int i = 0 ; i < historicos.Count() ; i++)
            {
                SqlDataReader dr3 = com2.ExecuteReader();
                while (dr3.Read())
                {
                    historicos[i][4] = dr3["QT"].ToString(); //quantidade de emails lidos naquela verificação
                }
                dr3.Close();
            }

        }

        return historicos;
Mauricio Gracia Gutierrez
  • 10,288
  • 6
  • 68
  • 99
  • @CsabaToth two loops one for reading the data into dados_historico and close the reader and then another loop to execute the other reader – Mauricio Gracia Gutierrez Aug 27 '13 at 21:08
  • +1 I know. And seemingly he gathers the data into some array, so he can reuse it. Yours is the real solution, unless there is even better single query one. – Csaba Toth Aug 27 '13 at 22:29
6

Add MultipleActiveResultSets=true to the provider part of your connection string. See the example below:

<add name="DbContext" connectionString="Data Source=(LocalDb)\v11.0;Initial Catalog=dbName;Persist Security Info=True;User ID=userName;Password=password;MultipleActiveResultSets=True" providerName="System.Data.SqlClient" />
Rousonur Jaman
  • 1,183
  • 14
  • 19
3

Try to combine the query, it will run much faster than executing an additional query per row. Ik don't like the string[] you're using, i would create a class for holding the information.

    public List<string[]> get_dados_historico_verificacao_email_WEB(string email)
    {
        List<string[]> historicos = new List<string[]>();

        using (SqlConnection conexao = new SqlConnection("ConnectionString"))
        {
            string sql =
                @"SELECT    *, 
                            (   SELECT      COUNT(e.cd_historico_verificacao_email) 
                                FROM        emails_lidos e 
                                WHERE       e.cd_historico_verificacao_email = a.nm_email ) QT
                  FROM      historico_verificacao_email a
                  WHERE     nm_email = @email
                  ORDER BY  dt_verificacao_email DESC, 
                            hr_verificacao_email DESC";

            using (SqlCommand com = new SqlCommand(sql, conexao))
            {
                com.Parameters.Add("email", SqlDbType.VarChar).Value = email;

                SqlDataReader dr = com.ExecuteReader();

                while (dr.Read())
                {
                    string[] dados_historico = new string[6];
                    dados_historico[0] = dr["nm_email"].ToString();
                    dados_historico[1] = dr["dt_verificacao_email"].ToString();
                    dados_historico[1] = dados_historico[1].Substring(0, 10);
                    //System.Windows.Forms.MessageBox.Show(dados_historico[1]);
                    dados_historico[2] = dr["hr_verificacao_email"].ToString();
                    dados_historico[3] = dr["ds_tipo_verificacao"].ToString();
                    dados_historico[4] = dr["QT"].ToString();
                    dados_historico[5] = dr["cd_login_usuario"].ToString();

                    historicos.Add(dados_historico);
                }
            }
        }
        return historicos;
    }

Untested, but maybee gives some idea.

Jeroen van Langen
  • 21,446
  • 3
  • 42
  • 57