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

Question

Why does this code throw an Invalid Operation Exception?

private SqlCommand cmd; // initialized in the class constructor

public void End(string spSendEventNotificationEmail) {
  try {
    cmd.CommandText = spSendEventNotificationEmail;
    cmd.Parameters.Clear();
    cmd.Parameters.Add("@packetID", SqlDbType.Int).Value = _packetID;
    cmd.Parameters.Add("@statusID", SqlDbType.Int).Value = _statusID;
    cmd.Parameters.Add("@website", SqlDbType.NVarChar, 100).Value = Tools.NextStep;
    cmd.Connection.Open();
    cmd.ExecuteNonQuery();
  } finally {
    cmd.Connection.Close();
    cmd.Parameters.Clear();
    cmd.Dispose();
  }
  endCall = true;
}

InvalidOperationException

I think that the root of the problem is in the instance of SqlCommand initialized at the class constructor. Having this var around in all of your code could be easily misused and leading to nasty bugs in other parts of your code — Steve, Jun 15 '12 at 15:35

Sergey Kudriavtsev · Accepted Answer · 2012-06-15T15:35:58.727

7

You're trying to open a connection which is already open, this results in exception.

Solution 1 (recommended):

Inspect your code, check all the parts where cmd.Connection connection is opened and ensure that it's always closed properly.

Solution 2 (quick'n'dirty fix):

before line

cmd.Connection.Open();

add the following check/cleanup code:

if (cmd.Connection.State == ConnectionState.Open)
{
    cmd.Connection.Close();
}

edited Jun 15 '12 at 15:35

answered Jun 15 '12 at 15:30

Sergey Kudriavtsev

10,328
4
43
68

1

Bleh! Freakin' noobie mistakes. – Jun 15 '12 at 15:35
Well, this could do the job, however the OP code hightlights the ExecuteNonQuery line, not the Connection.Open – Steve Jun 15 '12 at 15:38

score 6 · Answer 2 · answered Jun 15 '12 at 17:22

There's very little need for keeping the Sql* objects at the class level, especially based on what you're showing. You'll also lose the benefits of connection pooling by attempting to do it yourself.

With this method, you remove the possibility of your error because you're not sharing any objects

private readonly _connectionString = "...";

public void End(string spSendEventNotificationEmail) {
  using(var conn = new SqlConnection(_connectionString))
  using(var cmd = conn.CreateCommand())
  {
    cmd.CommandText = spSendEventNotificationEmail;
    cmd.Parameters.Add("@packetID", SqlDbType.Int).Value = _packetID;
    cmd.Parameters.Add("@statusID", SqlDbType.Int).Value = _statusID;
    cmd.Parameters.Add("@website", SqlDbType.NVarChar, 100).Value = Tools.NextStep;
    conn.Open();
    cmd.ExecuteNonQuery();
  }
  endCall = true;
}

The only reason I initially created it this way was so that several part numbers from a single request could share the same transaction. I've found a better way to do that, though, so I could (probably should) ditch this old method anyway. Thanks. — , Jun 15 '12 at 19:32

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

2 Answers2

Linked

Related