C# ExecuteReader requires an open and available Connection. The connection's current state is closed

Question

private void comT_SelectedIndexChanged(object sender, EventArgs e)
    {
        if (comT.SelectedIndex != -1)
        {
            SqlCommand cmd = new SqlCommand(@"SELECT ISNULL(substring(MAX(tCode),3,2),'00')
            FROM Teacher
            WHERE dCode = '" + comT.SelectedValue.ToString() + "'", MUControlClass.con);
            SqlDataReader dr = cmd.ExecuteReader();
            dr.Read();  <--HERE IS ERROR **"ExecuteReader requires an open and available Connection. The connection's current state is closed."**
            if (dr[0].ToString() != "00")
            {
                int dugaar = Convert.ToInt32(dr[0]) + 1;
                if (dugaar > 9)
                {
                    msk.Text = comT.SelectedValue.ToString() + dugaar.ToString();
                }
                else
                    msk.Text = comT.SelectedValue.ToString() + "0" + dugaar.ToString();
            }
            else
            {
                msk.Text = comT.SelectedValue.ToString() + "01";
            }
            dr.Close();
        }
    }

ExecuteReader requires an open and available Connection. The connection's current state is closed. Error

Error message is pretty clear. `MUControlClass.con` is not open. — LarsTech, May 14 '20 at 12:21
You should probably use `using {...}` blocks for your query calls, and always create a new connection when doing so. See [in a “using” block is a SqlConnection closed on return or exception?](https://stackoverflow.com/q/4717789/719186) for an example. — LarsTech, May 14 '20 at 12:25
This looks suspiciously like you're trying to share a single connection object across multiple methods. This is very often the wrong thing to do. Share the connection *string* but not the connection object. Create the object close to usage instead. — Damien_The_Unbeliever, May 14 '20 at 12:25

score 2 · Accepted Answer · answered May 14 '20 at 12:34

The immediate error is that the connection is not open, as it told you; so ... open it; however, there are a lot of other serious problems here - most notably "SQL injection", but also non-disposed objects. You can fix SQL injection by using parameters, and the non-disposed problem with lots of using, but: I strongly suggest you make use of tools that will help you get this right by default. For example:

private void comT_SelectedIndexChanged(object sender, EventArgs e)
{
    if (comT.SelectedIndex != -1)
    {
        using (var conn = new SqlConnection(yourConnectionString))
        {
            string max = conn.QuerySingle<string>(@"
SELECT ISNULL(substring(MAX(tCode),3,2),'00')
FROM Teacher
WHERE dCode = @dCode", new { dCode = comT.SelectedValue.ToString() });
            if (max != null)
            {
                // your parse/etc logic
            }
        }
    }
}

This is:

moving our connection lifetime to be local to this method, which will stop a lot of connection usage problems
using "Dapper" to provide the QuerySingle<T> code
which means you don't need to mess with commands or readers
adding parameter usage via @dCode and the new { dCode = ... } usage

Note it might look like we've still not opened the connection here, but Dapper is happy to do that for us; if it is given a closed connection, it opens the connection for the duration of the query only.

One of the reasons I use Stored Procedures. It _forces_ the developper to use parameters. — jscarle, May 14 '20 at 12:47
@jscarle I've seen people do `cmd.CommandText = "EXEC myProc @id=" + id + ", @name='" + name + "'";` - never under-estimate the ability to screw up; broadly (but not exactly) related: https://blog.marcgravell.com/2017/12/dapper-prepared-statements-and-car-tyres.html — Marc Gravell, May 14 '20 at 12:53
@jscarle Not to mention stored procedures that takes in parameters but use it inside to create dynamic SQL statements using string concatenations.... [Here's an example](https://stackoverflow.com/a/40603360/3094533) — Zohar Peled, May 14 '20 at 12:58

score 1 · Answer 2 · answered May 14 '20 at 12:22

1

Wherever the connection is created in MUControlClass you need to call con.Open().

answered May 14 '20 at 12:22

Neil

11,059
3
31
56

score 1 · Answer 3 · answered May 14 '20 at 12:31

Your connection is not opened.

Simplified flow of connection and interaction with database is like this:

using(SqlConnection con = new SqlConnection(yourConnectionString))
{
    con.Open() // You do not open this one so it returns that error
    using(SqlCommand cmd = new SqlCommand(yourSqlCommand, con))
    {
        using(SqlDataReader dr = cmd.ExecuteReader())
        {
            if(dr.Read())
            {
                // Do something
            }
        }
    }
}

We do not need to do close/destroy anything as long as it is wrapped in using since they all implements IDisposable

C# ExecuteReader requires an open and available Connection. The connection's current state is closed

3 Answers3