4

I tried to call this method from multiple threads trying to get the ID from the same string. I am always getting this exception at the line where I create the SqlDataReader:

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

I don't know where the problem is. I am using a lock() statement so I use the command only once, then I dispose it. Kinda new to database programming so I don't know where my error is.

Thanks!

public int UsernameGetID(string username)
{
    using (var command = new SqlCommand("SELECT user_id FROM " + ServerConstants.Database.TableUserInformation + " WHERE username = @Username", connection))
    {
        lock (command)
        {
            SqlParameter param = new SqlParameter("@Username", SqlDbType.VarChar, username.Length);
            param.Value = username;
            command.Parameters.Add(param);
            using (SqlDataReader reader = command.ExecuteReader())
            {
                if (reader.Read())
                {
                    return (int)reader[0];
                }
                else
                {
                    // username doesn't exists
                    return 0;
                }
            }
        }
    }
}
5
  • 2
    What if you do not share the connection variable, but create the connection in the UsernameGetID method itself? Commented Jul 31, 2012 at 22:40
  • You are using an opened connection from previous or some other SqlDataReader Commented Jul 31, 2012 at 22:42
  • wouldn't it hurt performance? I would like to use only one connection as I don't want to create a connection instance every time I fetch something from my databases. I have only one connection and all the commands are done on it. Is that a bad thing to do? Commented Jul 31, 2012 at 22:42
  • 1
    Are you using the same connection for other readers? If yes, you can try to add MultipleActiveResultSets=true; to the end of the connection string. Or just create a new SqlConnection Commented Jul 31, 2012 at 22:43
  • @Pacha: Yes, it is. It's error prone, as you've discovered. The connection pool will handle this for you - it won't really create a new network connection each time. Commented Jul 31, 2012 at 22:43

1 Answer 1

9

Locking on command is pointless, given that you're creating a new command within the method - no other code could lock on it.

However, you're sharing the connection between multiple commands. Don't do that - create a new SqlConnection (again, in a using statement) in each call. Don't worry about the efficiency aspect - the connection pool will take care of the "real" network connection.

So you want:

using (var connection = new SqlConnection(...))
using (var command = new SqlCommand(..., connection))
{
    connection.Open();
    ...
    using (var reader = command.ExecuteReader())
    {
        return reader.Read() ? (int) reader[0] : 0;
    }
}
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.