System.InvalidOperationException -> DataReader associated with this Command [Commit of Transaction Failed]

Question

I have such problem. I've added Transaction service to my SQL class. I get error, when I want to Commit query after its execution. I'll paste the code:

//SOME OF VARIABLES IN CLASS:
private SqlConnection connection;
private SqlCommand newQuery;
private SqlTransaction transaction;
private SqlDataReader result;

//BEGINNING TRANSACTION
public void BeginTransaction(string name)
{
    try
    {
        transaction = connection.BeginTransaction(name);
    }

    catch
    {
        MessageBox.Show("Error while beginning transaction " + name);
    }
}

//COMMIT TRANSACTION
public void Commit(string text)
{
    try
    {
        transaction.Commit();
    }

    catch (Exception e)
    {
        MessageBox.Show("Couldn't commit transaction " + text + "\n\n" + e.ToString());

        try
        {
            transaction.Rollback();
        }

        catch
        {
            MessageBox.Show("Couldn't Rollback transaction " + text);
        }
    }

        transaction = null;
}

//EXECUTE QUERY METHOD
private SqlDataReader ExecuteQuery(string query)
{
    try
    {
        if (connection.State == ConnectionState.Closed)
            connection.Open();

        if (result != null)
            result.Close();

        newQuery = connection.CreateCommand();
        newQuery.CommandText = query;
        newQuery.Transaction = transaction;

        result = newQuery.ExecuteReader();
    }

    catch (Exception e)
    {
        MessageBox.Show(e.ToString());
    }

    return result;
}

//EXAMPLE FUNCTION WITH TRANSACTION, WHICH OCCURS ERROR:
public bool DoesDatabaseExist(string dbName)
{
    BeginTransaction("DoesDatabaseExist");
    bool res = ExecuteQuery("SELECT * FROM master.dbo.sysdatabases WHERE name='" + dbName + "';").HasRows;
    Commit("Does DB Exist 211");

    return res;
}

Afrer running program, I get error, that Commit didn't pass. Like:

Couldn't commit transaction Does DB Exist 211

System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.

I'm studying programming in C# still, so probably it is easy to recognise error. But not for me. Please help.

Before I've added the transaction service, everything was ok, I didn't change or add any of queries or executions of queries. Please help.

Thanks, Mike.

If you have called ExecuteQuery before then you have an open DataReader. I really suggest you to use a real ORM instead of your home made database class — Steve, Jun 04 '18 at 12:33
You should also read https://stackoverflow.com/questions/14376473/what-are-good-ways-to-prevent-sql-injection as a matter of urgency. It is unrelated to your current issue, but critical for you to fix. — mjwills, Jun 04 '18 at 12:33
Ok, thank you. Answering your first comment, I want apply transaction to avoid of data loss. Here, it's true, it doesn't give me anything, but i have other method, where I have two queries, first, which add user to user table, and second, which creates user in sql server system. I wouldn't like to execute only one of them, because it would give me non consistent form — mpaw, Jun 04 '18 at 12:44

mjwills · Accepted Answer · 2018-06-04T13:21:28.277

The main issue is the way you are using your SqlDataReader instances.

Your first mistake is having it as a field of the class:

private SqlDataReader result;

Don't do that (please remove that line).

Then change the function to:

private SqlDataReader ExecuteQuery(string query)
{
    try
    {
        if (connection.State == ConnectionState.Closed)
            connection.Open();

        newQuery = connection.CreateCommand();
        newQuery.CommandText = query;
        newQuery.Transaction = transaction;

        var result = newQuery.ExecuteReader();
        return result;
    }

    catch (Exception e)
    {
        MessageBox.Show(e.ToString());
        throw;
    }
}

This ensures that every invocation of the above function returns its own instance of SqlDataReader.

OK, now the caller. You are currently using:

bool res = ExecuteQuery("SELECT * FROM master.dbo.sysdatabases WHERE name='" + dbName + "';").HasRows;
Commit("Does DB Exist 211");

This is incorrect, since ExecuteQuery is returning a SqlDataReader that you aren't cleaning up properly. Change it to:

var results = ExecuteQuery("SELECT * FROM master.dbo.sysdatabases WHERE name='" + dbName + "';");
using (results)
{
    bool res = results.HasRows;
}
Commit("Does DB Exist 211");

The using will ensure that the SqlDataReader is disposed (closed) properly.

Note there are still a number of other remaining issues in your code. For example:

Your use of an explicit transaction is pointless, since you are executing a single SELECT statement
Your other fields (for Command, Connection and Transaction) are also a bad idea (like result was)
Your code is open to SQL Injection
You may wish to read up on Dapper

System.InvalidOperationException -> DataReader associated with this Command [Commit of Transaction Failed]

1 Answers1