Parallel.ForEach Misbehaviour

Question

Possible Duplicate:
C# Value storage during Parallel Processing

I was running some performance tests in my console application today and I stumbled across something really unexpected. My code:

int iterations = 1000000;

var mainList = new List<string>();

for (int i = 0; i < iterations; i++)
{
    mainList.Add(i.ToString());
}

var listA = new List<string>();

Parallel.ForEach(mainList, (listItem) =>
                           {
                               if (Int32.Parse(listItem)%2 == 0)
                               {
                                   listA.Add(listItem);
                               }
                           });

Console.WriteLine("Parallel Count: {0}", listA.Count);

var listB = new List<string>();
foreach (var listItem in mainList)
{
    if (Int32.Parse(listItem) % 2 == 0)
    {
        listB.Add(listItem);
    }
}

Console.WriteLine("Sequential Count: {0}", listB.Count);

Which resulted in an output:

Parallel Count: 495939

Sequential Count: 500000

I ran it several times and the parallel loop never seems to get executed at a proper amount of times. Can anyone explain this "misbehaviour"? Are the parallel loops trustworthy?

P.S. I know there is a lot of nonsense going on in the provided example of code like ToString() calls on an integer and than parsing them back but that was just a random code I came up with while testing. Thanks in advance.

`List` is not threadsafe - you're adding to a collection which is not threadsafe from multiple threads, so you can't rely on it working correctly. — Lee, Dec 29 '12 at 18:28

score 15 · Accepted Answer · edited Dec 29 '12 at 20:02

15

Your problem is not with Parallel.ForEach. Your problem is with the List<int> - the class is not thread safe. My guess is that you are running into thread safety issues with the list object. Try using a ConcurrentBag<int> instead, and the problem will likely vanish.

From Microsoft regarding the thread safety of List<T>:

To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

edited Dec 29 '12 at 20:02

Adam

15,537
2
42
63

answered Dec 29 '12 at 18:31

theMayer

15,456
7
58
90

1

Please do not recommend ConcurrentBag since it is flawed in many ways (http://stackoverflow.com/questions/10850443/is-there-a-memory-leak-in-the-concurrentbagt-implementation). You trade thread safety vs memory leak or a much higher memory consumption. A simple lock and List would be sufficient as well. – Alois Kraus Dec 29 '12 at 20:05
1

@AloisKraus, that is another relatively simple option; I was just trying to provide the simplest way of making the example work properly. – theMayer Dec 29 '12 at 20:11

Parallel.ForEach Misbehaviour

1 Answers1

Linked