CODEWITHSUNDEEP
c#concurrencyparallel.foreachatomic
user588582
user588582

Reputation: 31

Why does simple Parallel.ForEach sometimes fail to add value to a string variable?

I am trying to speed up a foreach loop which appends a string result from a method to a variable. It doesn't matter what order the result strings get appended to the variable so I thought about changing the foreach to a Parallel.ForEach. Running a simple test I found that this may cause one or more of the result strings to not get appended to the variable.

Here is the test I created.

private string GetMessageString()
{
    string message = string.Empty;
    List<int> listOfInts = new List<int>();
    listOfInts.Add(1);
    listOfInts.Add(2);
    listOfInts.Add(3);
    listOfInts.Add(4);
    listOfInts.Add(5);
    listOfInts.Add(6);
    listOfInts.Add(7);
    listOfInts.Add(8);
    listOfInts.Add(9);
    Parallel.ForEach(listOfInts, currentInt =>
    {
        message += currentInt.ToString();
    });
    return message;
}

Now if I run this multiple times the message variable will have all 9 numbers in the string most of the time, but occasionally the message variable will be missing one or more of the numbers.

For example message = "12436789" where the 5 is missing.

Is this a race condition? What is the best way to assure all numbers get appended before the Parallel.ForEach loop exits?

Upvotes: 3

Views: 705

Answers (1)

usr
usr

Reputation: 171226

Is this a race condition?

Yes, this line is not atomic. By default pretty much nothing is atomic or thread-safe. It is important to understand the rules before you start playing with fire (threads).

This particular code is not going to receive any speedup due to per-iteration overhead. Even if that overhead was zero this would not speed up because we would need to synchronize on message. So this is pointless and there is no simple way to make this go faster than serial.

Maybe you could make it faster by running some kind of aggregation tree... This is a bad example for you to start learning threads basically.

So a fix would be to use a lock:

object @lock = new object();
...
lock (@lock)
      message += currentInt.ToString();

Upvotes: 1

Related Questions