0

I have the following script to fetch me some data from a database and then remove it:

public void checkDB()
{
    string query = "SELECT * FROM dbt";

    SqlCommand sqlCommand = new SqlCommand(query, conn);

    SqlDataReader reader;
    int id = -1;

    using (reader = sqlCommand.ExecuteReader())
    {
            if (reader.Read())
            {
                String data= reader["sdata"].ToString();
                Order o = new Order(reader["sdata"].ToString());
                o.prepareForScript();
                id = reader.GetSqlInt32(1).Value;
            }

            reader.Close();
    }

    if (id != -1)
    {
        string removeQuery = "DELETE FROM data WHERE ID=" + id;

        SqlCommand removeCMD = new SqlCommand(removeQuery, conn);

        removeCMD.ExecuteNonQuery();
    }
}

This code results in an exception

unhandled exception of type 'System.InvalidOperationException' occurred in System.Data.dll

with the aditional information that a reader is already associated with this connection. However as you can see the reader is both closed and inside a using loop meaning that it should definitly be closed. Anybody know how to fix this?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Thijser
  • 2,625
  • 1
  • 36
  • 71
  • http://stackoverflow.com/questions/5440168/exception-there-is-already-an-open-datareader-associated-with-this-connection-w – splattne Aug 05 '15 at 08:44
  • Try wrapping your SQL commands in a using also. see http://stackoverflow.com/questions/16985876/sqlconnection-sqlcommand-sqldatareader-idisposable – Mark Aug 05 '15 at 08:46
  • May be the `conn` problem. Have you tried to close the conn after SELECT query & open for DELETE query. – Harshit Aug 05 '15 at 08:49
  • [SQL Injection alert](http://msdn.microsoft.com/en-us/library/ms161953%28v=sql.105%29.aspx) - you should **not** concatenate together your SQL statements - use **parametrized queries** instead to avoid SQL injection – marc_s Aug 05 '15 at 09:05
  • @Marc_s I'm aware of the risk however I'm pulling an int from a database and then sending it back for deletion that shouldn't be a risk here right? – Thijser Aug 05 '15 at 09:15
  • Also take a look at the OUTPUT keyword. You can do your delete and return the deleted rows all at the same time. – Mark Aug 05 '15 at 09:20
  • @Thijser: just get in the habit of doing it *the right way*! Don't do it with parameters once, and without another time - just use parametrized queries **always** to be on the safe side! – marc_s Aug 05 '15 at 09:23
  • @Thijser: [see what Jon Skeet has to say on the topic](http://codeblog.jonskeet.uk/2014/08/08/the-bobbytables-culture/) - and takes his words to heart ! – marc_s Aug 05 '15 at 11:53

1 Answers1

4

You need to dispose first SqlCommand as below :

using (SqlCommand sqlCommand = new SqlCommand(query, conn))
{
        SqlDataReader reader;
        int id = -1;

        using (reader = sqlCommand.ExecuteReader())
        {
            if (reader.Read())
            {


                String data= reader["sdata"].ToString();
                Order o = new Order(reader["sdata"].ToString());
                o.prepareForScript();
                id = reader.GetSqlInt32(1).Value;
            }
            reader.Close();
        }
}

Or

enable MultipleActiveResultSets. This way SQL Server allows several open data readers on a single connection.

But i would suggest not to use your SQL statements together to avoid Sql Injection.

Neel
  • 11,625
  • 3
  • 43
  • 61