Why is this causing an ArgumentOutOfRangeException

Question

I have had a go at writing something to hash numbers and check them against a list to see if a matching hash exists or not.

I got this working fine using a for loop, I then decided to try speed things up by using Parallel.For - unfortunately this causes an ArgumentOutOfRangeException that i'm having trouble debugging.

public class HashCompare
{
    private string encryptedCardNumber;
    private Encrypter encrypter;
    private BlockingCollection<string> paraCardNumberList;

    public string Compare(string hash)
    {
        bool exists = Lookup.hashExists(hash);

        if (exists)
        {
            string unencryptedCardNumber = Lookup.GetUnencryptedCardNumber(hash);
            return unencryptedCardNumber;
        }
        return null;
    }

        public BlockingCollection<string> PLCompareAll()
    {

        paraCardNumberList = new BlockingCollection<string>();

        Parallel.For(100000, 999999, i =>               
        {
            encrypter = new Encrypter();

            encryptedCardNumber = encrypter.EncryptCardNumber(i.ToString());
            var result = Compare(encryptedCardNumber);

            if (result != null)
            {
                paraCardNumberList.Add(result);
            }
        });
        paraCardNumberList.CompleteAdding();

        return paraCardNumberList;
    }
}

The error occurs randomly when calling encrypter.EncryptCardNumber (seemingly on the returnValue.ToString())

private StringBuilder returnValue

public string EncryptCardNumber(string str)
{
    try
    {
        var sha1 = SHA1.Create();
        byte[] hashData = sha1.ComputeHash(Encoding.Default.GetBytes(str));
        returnValue = new StringBuilder();

        for (int i = 0; i < hashData.Length; i++)
        {
            returnValue.Append(hashData[i].ToString("x2"));
        }
    }
    catch (Exception ex)
    {
        string strerr = "Error in hash code: " + ex.Message;
    }
    return returnValue.ToString();
}

I have 2 questions:

1. Why am i getting an exception?
2. Am i right to use a BlockingCollection for what i'm trying to achieve?

Answer 1

StringBuilder is not thread safe:

Any instance members are not guaranteed to be thread safe.

But it appears you're using the same instance of StringBuilder for all of your EncryptCardNumber calls. It's not surprising that you'd encounter a race condition when one thread is doing a .ToString() while another is trying to append more characters.

I'm guessing that you don't actually intend to have all of these threads overwriting and appending to each other's instances of StringBuilder. Try declaring the object as a local variable, rather than a field on that class. The same principle probably also applies to other variables, like encrypter.

As for your BlockingCollection<>, that's probably a fine approach, but I'd personally go with something a little more functional:

return Enumerable.Range(100000, 999999)
    .AsParallel() // One line to make this parallel
    .Select(i => new Encrypter().EncryptCardNumber(i.ToString())
    .Select(Compare)
    .Where(hash => hash != null)
    .ToList();