Parallel.ForEach() not getting expected results

Question

Inside my web api controller I'm looping over a list with Parallel.ForEach(). I have a counter that I increment in the Parallel.ForEach code. I'm noticing that counter is a variable number each time I run it and it's never as high as the list I'm looping over with Parallel.ForEach(). It seems like Parallel.ForEach() isn't waiting to come back before it's finished with looping over all elements.

// get all the new records from the csv
var newData = csv.GetRecords<MyEFTable>().ToArray();
int count = 0;
Parallel.ForEach(newData, (d) => {
  count++});

newData has 6588 items and count generally is around 3400 or so items but again it's variable each time. This is very strange.

count++ is not thread safe. Replace it with Interlocked.Incremenet(ref count) — MarcinJuraszek
– MarcinJuraszek, Commented Mar 31, 2017 at 17:36

user47589 · Accepted Answer · 2017-03-31 17:36:42Z

6

You're getting into a race condition. You need to use var newCount = Interlocked.Increment(ref count); to safely increment a variable in a multithreaded environment. The newCount variable is the incremented counter.

The reason this happens is because count++ is not atomic. It's actually three operations: getting the value, adding 1, then storing it back. If you have those three things happening concurrently, stuff gets out of order and wonky.

When dealing with threads, its vital to ensure each thread isn't manipulating the same data, or they will squash each other.

answered Mar 31, 2017 at 17:36

user47589

Sign up to request clarification or add additional context in comments.

5 Comments

user441521 Over a year ago

So is adding to a list not thread safe too? The reason I was using the counter was to see what was happening inside this. I have more code than this but was breaking it up to see what was going on because adding to a list wasn't giving correct results either.

user47589 Over a year ago

Probably. You can check the MSDN documentation for the List class to see (hint: list methods are not thread-safe). So if you're manipulating a list in more than one thread, you will either a) need to switch to a thread-safe collection type such as ConcurrentBag, or b) use a lock (SyncLock) around your code that accesses shared resources.

user47589 Over a year ago

Again, what's happening is one thread adds an item, and another thread does the same at the same time. The first thread gets the current items, so does the other thread. Then the first thread adds an item, and stores the result. Well if the second thread does the same, it's still operating on the original 'version' of its items list. So you can add items, but have those adds get 'overwritten'.

user441521 Over a year ago

OK, that's what is happening then. I see there are concurrent collections to prevent this. That's what I need. Will accept your answer when it lets me. Thanks for pointing me in the right direction.

William Xifaras Over a year ago

Adding to a list in a Parallel.ForEach is not thread safe. Use a thread safe collection or lock before the write.

Collectives™ on Stack Overflow

Parallel.ForEach() not getting expected results

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related