System.InvalidOperationException: The connection was not closed. The connection's current state is open

Question

I have searched on different sites but couldn't find the exact solution. Here is my code kindly help me how to fix that error I will be really thankful to you. I have close connection at the end of each method but I am having that error and when I comment it out another error come "invalid attempt to call read when reader is closed"

private void Count()
{
    int count;
    SqlDataReader reader = null;


    string selectedcmd = "SELECT COUNT(Review_rate) AS Review_count FROM Review WHERE (Paper_id = '" + Session["PaperID"] + "')";
    SqlCommand cmd = new SqlCommand(selectedcmd, dbcon);
    dbcon.Open();
    reader = cmd.ExecuteReader();
    while (reader.Read())
    {
        count = Convert.ToInt32(reader.GetInt32(0));


        if (count < 3)
        {
            Response.Redirect("Review_submit.aspx");
        }

        else
        {
            average();
        }
    }
    dbcon.Close();
}

private void average()
{
    int avg;
    SqlDataReader reader1 = null;


    string selectedValue = RadioButtonList1.SelectedValue;
    Response.Write(selectedValue);
    dbcon.Open();
    string selectedcmd = "SELECT AVG(Review_rate) AS AVG,COUNT(Review_rate) as Review_count  FROM  Review WHERE (Paper_id ='" + Session["PaperID"] + "') GROUP BY Paper_id HAVING (COUNT(Review_rate)>= 3)";
    SqlCommand cmd = new SqlCommand(selectedcmd, dbcon);

    reader1= cmd.ExecuteReader();
    while (reader1.Read())
    {
        avg = Convert.ToInt32(reader1.GetInt32(0));


        if (avg < 3)
        {
            if (reader1.HasRows)
            {
                SqlCommand cmd2 = new SqlCommand("SELECT [User].User_email FROM [User] INNER JOIN Paper ON [User].User_id = Paper.User_id WHERE ([User].User_id =Paper.User_id) AND (Paper.Paper_id ='" + Session["PaperID"] + "')", dbcon);
                // SqlCommand cmd1 = new SqlCommand("SELECT [User].User_email FROM [User] INNER JOIN Paper ON [User].User_id = Paper.User_id WHERE ([User].User_id ='2') AND (Paper.Paper_id ='2')", dbcon);
                string result = (string)cmd2.ExecuteScalar();

                //if (!string.IsNullOrEmpty(result))
                // {
                //   Label1.Text = result;
                // }

                StringBuilder bodyMsg = new StringBuilder();
                //email = Email.Text;
                bodyMsg.Append("Your paper is rejected");
                // bodyMsg.Append("<br /><br /><a href=http://localhost:7401/SendEmailConfirmationSample/Login.aspx?ID=" + email.ToString() + ">Activate Your Account</a>");
                bodyMsg.Append("<br />");
                bodyMsg.Append("<br />");
                bodyMsg.Append("<br />");
                // bodyMsg.AppendFormat("Registered Email: {0}", email);
                NetworkCredential loginInfo = new NetworkCredential("salmacms26@gmail.com", "salma@cms");
                MailMessage msg = new MailMessage();
                msg.From = new MailAddress("salmacms26@gmail.com");
                msg.To.Add(new MailAddress(result));
                msg.Subject = "Reviews Announcment";
                msg.Body = bodyMsg.ToString();
                msg.IsBodyHtml = true;

                SmtpClient client = new SmtpClient("smtp.gmail.com", 587);
                client.EnableSsl = true;
                client.UseDefaultCredentials = false;
                client.Credentials = loginInfo;
                client.Send(msg);


                //Response.Redirect("~/Login.aspx");
            }
            else
            {
                if(reader1.HasRows)
                {
                    SqlCommand cmd2 = new SqlCommand("SELECT [User].User_email FROM [User] INNER JOIN Paper ON [User].User_id = Paper.User_id WHERE ([User].User_id =Paper.User_id) AND (Paper.Paper_id ='" + Session["PaperID"] + "')", dbcon);
                    // SqlCommand cmd1 = new SqlCommand("SELECT [User].User_email FROM [User] INNER JOIN Paper ON [User].User_id = Paper.User_id WHERE ([User].User_id ='2') AND (Paper.Paper_id ='2')", dbcon);
                    string result = (string)cmd2.ExecuteScalar();

                    if (!string.IsNullOrEmpty(result))
                    {
                        Label1.Text = result;
                    }

                    StringBuilder bodyMsg = new StringBuilder();
                    //email = Email.Text;
                    bodyMsg.Append("Your paper is accepted");
                    // bodyMsg.Append("<br /><br /><a href=http://localhost:7401/SendEmailConfirmationSample/Login.aspx?ID=" + email.ToString() + ">Activate Your Account</a>");
                    bodyMsg.Append("<br />");
                    bodyMsg.Append("<br />");
                    bodyMsg.Append("<br />");
                    // bodyMsg.AppendFormat("Registered Email: {0}", email);
                    NetworkCredential loginInfo = new NetworkCredential("salmacms26@gmail.com", "salma@cms");
                    MailMessage msg = new MailMessage();
                    msg.From = new MailAddress("salmacms26@gmail.com");
                    msg.To.Add(new MailAddress(result));
                    msg.Subject = "Reviews Announcment";
                    msg.Body = bodyMsg.ToString();
                    msg.IsBodyHtml = true;

                    SmtpClient client = new SmtpClient("smtp.gmail.com", 587);
                    client.EnableSsl = true;
                    client.UseDefaultCredentials = false;
                    client.Credentials = loginInfo;
                    client.Send(msg);


                    //Response.Redirect("~/Login.aspx");
                }
            }
        }
        dbcon.Close();
    }
}

what should i do kindly help me i will be really thankful to you — kristen stewart, Jul 19 '14 at 09:45
These are the results that you would expect when using unnecessary global variables — Steve, Jul 19 '14 at 09:46
dbCon in this context is a global variable, defined at the class level and so it could be modified by any method in this class. The use of global variables is a well known problem for the maintenability and robustness of any kind of program — Steve, Jul 19 '14 at 09:55
@kristenstewart Steve is suggesting you make the `dbcon` variable a local variable (declared inside the `Count` method's body) instead of a field (declare as a member of the class). — dcastro, Jul 19 '14 at 09:55

dcastro · Answer 1 · 2014-07-19T09:59:03.673

1

Your average method tries to open a connection that has already been opened by Count.

First off, you have to be careful not to open the connection a second time without closing it first.

Also:

Instead of explicitly calling Close on the connection, you should be using the using statement, since the connection implements the IDisposable interface
I'd reduce the scope of the connection to be a local variable in Count, and pass it as an argument to average, like so:

private void Count()
{
    string selectedcmd = "SELECT COUNT(Review_rate) AS Review_count FROM Review WHERE (Paper_id = '" + Session["PaperID"] + "')";

    using(SqlConnection conn = new SqlConnection(connectionString) )
    using(SqlCommand command = new SqlCommand(selectedcmd, conn))
    {
        conn.Open();
        using(var reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                //more code
                average(conn);
            }
        }
    }
}

private void average(SqlConnection conn)
{
    //use the connection passed as an argument
}

The using statement will take care of disposing of the connection and the command, even if an exception occurs. Your current code won't close the connection if an excepion occurs.

edited Jul 19 '14 at 09:59

answered Jul 19 '14 at 09:44

dcastro

66,540
21
145
155

it means i should not open the connection again in average method right? – kristen stewart Jul 19 '14 at 09:47
@kristenstewart Yes. See my updated answer. More importantly, you should use a local variable for `conn`, instead of using a class-level field (like Steve suggested in the comments). – dcastro Jul 19 '14 at 09:51
where to close connection @dcastro – kristen stewart Jul 19 '14 at 10:17
@kristenstewart If you use the `using` statements like I suggested, the statement will automatically close the connection for you. – dcastro Jul 19 '14 at 10:18
i have used your code ..now i am facing this error ..ExecuteReader requires an open and available Connection. The connection's current state is closed. – kristen stewart Jul 19 '14 at 10:30
ahh...:( :( i am fed up now please what should i do now – kristen stewart Jul 19 '14 at 10:31
@kristenstewart You have to use the reader *inside* the brackets: `using(connection) { conn.Open(); //use reader here }` – dcastro Jul 19 '14 at 10:31
i have used it in brackets like that using (SqlCommand command = new SqlCommand(selectedcmd, dbcon)) { reader1 = command.ExecuteReader(); while (reader1.Read()) { avg = Convert.ToInt32(reader1.GetInt32(0)); – kristen stewart Jul 19 '14 at 10:36
Edit your question and add your new code at the bottom, and tell us where the exception happened. – dcastro Jul 19 '14 at 10:37
now i am pasting other string selectedcmd = "SELECT AVG(Review_rate) AS Expr1 FROM Review WHERE (Paper_id = '" + Session["PaperID"] + "')GROUP BY Paper_id HAVING (AVG(Review_rate) >= 3)";using (dbcon = new SqlConnection(conn))using (SqlCommand command = new SqlCommand(selectedcmd, dbcon)){reader1 = command.ExecuteReader();while (reader1.Read()){avg = Convert.ToInt32(reader1.GetInt32(0));if (avg < 3) – kristen stewart Jul 19 '14 at 10:41
@kristenstewart You're passing `dbconn` to the `SqlCommand` constructor, you should be passing `conn` instead. Delete all your connection, command, and reader fields, like `dbconn` and `reader1`, and make everything a local variable. – dcastro Jul 19 '14 at 10:51
conn is connection string ..dbcon is sql connection – kristen stewart Jul 19 '14 at 10:59

score 0 · Answer 2 · edited May 23 '17 at 10:28

Do not use global variables for keeping the Connection.
Start using the using statement to be sure to close correctly these preciuos resources..

For example

private void Count()
{
    .....

    using(SqlConnection dbCon = new SqlConnection(connectionString))
    using(SqlCommand cmd = new SqlCommand(selectedcmd, dbcon))
    {
         dbcon.Open();
         using(SqlDataReader reader = cmd.ExecuteReader())
         {
              while (reader.Read())
              {
                  ......
              }
         } // Here the reader goes out of scope and it is closed and disposed
    }// Here the connection is closed and together with the command is disposed
}

This will remove your problem because the Using Statement will close and dispose every disposable object referenced in the start of its block

As a side note, not related to your problem. String concatenation could be very dangerous for the security of your db (Sql Injection). So, the Session["ID"] value should be passed to your query using a parameterized approach

System.InvalidOperationException: The connection was not closed. The connection's current state is open

2 Answers2