Having trouble with: The connection was not closed. The connection's current state is open. - SQL Server & C#

Question

I'm currently working on the login form of a school management system. The thing is that when I try to log in, I get an error:

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

It says that the error is on the 30th line of code but I can't seem to find a way to solve it.

Here's the code of the method in which the error occurs:

public void LoginTeacher()
{
        try
        {
            command = new SqlCommand("TeacherLogin", connection);
            command.CommandType = CommandType.StoredProcedure;

            connection.Open(); // This is the 30th line. 

            command.Parameters.AddWithValue("@username", Txt_User.Text);
            command.Parameters.AddWithValue("@password", Txt_Pass.Text);

            SqlDataReader dataReader = command.ExecuteReader();

            if (dataReader.Read())
            { 
                    TeacherDash teacherDash = new TeacherDash();
                    this.Hide();
                    teacherDash.lblusertype.Text = dataReader[1] + " " + dataReader[2].ToString();
                    teacherDash.ShowDialog();
                    this.Close();
                }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.ToString());
        }
        finally
        {
            connection.Close();
        }
}

Immediately after that error is shown there is another one that says:

System.InvalidOperationException: Invalid attempt to call CheckDataIsReady when reader is closed

and points to line 71 which is the following:

public void Login()
{
        try
        {
            command = new SqlCommand("SP_USER_LOGIN", connection);
            command.CommandType = CommandType.StoredProcedure;

            connection.Open();

            command.Parameters.AddWithValue("@user", Txt_User.Text);
            command.Parameters.AddWithValue("@pass", Txt_Pass.Text);

            SqlDataReader dataReader = command.ExecuteReader();

            if (dataReader.Read())
            {
                LoginTeacher();

                if (dataReader[10].Equals("Admin"))
                {
                    AdminDash adminDash = new AdminDash();
                    this.Hide();
                    adminDash.lblusertype.Text = dataReader[1] + " " + dataReader[2].ToString();
                    adminDash.ShowDialog();

                    this.Close();
                }

There's more code after that but I don't find it relevant since it's the same thing but with the different type of users.

Thanks in advance!

Answer 1

make it async method.It may fix.

Answer 2

Database object need to be closed and disposed. Keeping them local to the method where they are used lets you make sure this happens. using blocks take care of this for you.

I used a DataTable instead of testing with the reader because the connection must remain open as long as the reader is in use. Opening and closing the connection in the briefest possible time is important.

Please don't use .AddWithValue. See http://www.dbdelta.com/addwithvalue-is-evil/ and https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/ and another one: https://dba.stackexchange.com/questions/195937/addwithvalue-performance-and-plan-cache-implications Here is another https://andrevdm.blogspot.com/2010/12/parameterised-queriesdont-use.html Of course you will have to check your database for the real datatypes and field size to have the correct .Add method.

    public void LoginTeacher()
    {
        DataTable dt = new DataTable();
        using (SqlConnection cn = new SqlConnection("your connection string"))
        using (SqlCommand cmd = new SqlCommand("TeacherLogin", cn))
        { 
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.Add("@username",SqlDbType.VarChar,100 ).Value = Txt_User.Text;
            cmd.Parameters.Add("@password",SqlDbType.VarChar, 100 ).Value =Txt_Pass.Text;
            cn.Open();
            dt.Load(cmd.ExecuteReader());
        } //Your connection and command are both disposed
        if (dt.Rows.Count > 0)
        {
            TeacherDash teacherDash = new TeacherDash();
            teacherDash.lblusertype.Text = $"{dt.Rows[0][1]} {dt.Rows[0][2]}";
            teacherDash.ShowDialog();
            Close();
        }
        else
            MessageBox.Show("Sorry, login failed");
    }

Answer 3

You could try changing your TeacherLogin() method to something like the following:

public void TeacherLogin()
{
    try
    {
        using(SqlConnection con = new SqlConnection("connection string"))
        {
            using(SqlCommand cmd = new SqlCommand("TeacherLogin"))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.AddWithValue("@username", Txt_User.Text);
                cmd.Parameters.AddWithValue("@password", Txt_Pass.Text);
                cmd.Connection = con;
                con.Open();
                using(SqlDataReader dr = cmd.ExecuteReader())
                {
                    while(dr.Read())
                    {
                        TeacherDash teacherDash = new TeacherDash();
                        this.Hide();
                        teacherDash.lblusertype.Text = string.Format("{0} {1}", dr[1], dr[2]);
                        teacherDash.ShowDialog();
                    }
                }
            }
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

There's no need to use finally{} to close the connection as its all wrapped in a using() block, it will close and dispose on its own when the code leaves the block. I'd always recommend using SQL connections and commands in this way as not doing so can cause issues by leaving connections open.