Method local variable being modified inside a Parallel.For. How thread safe is this?

I have this code snippet:

int totalData = result.Data.Count;
int count = 0;
Parallel.ForEach(result.Data, data =>
{
    try
    {
        EventRange importedEntity = ImportEntity(auxResult.EntityName, data);
        count++;
        EntityImported(importedEntity, count, totalData);
    }
    catch (Exception e)
    {
        exceptions.Enqueue(e);
    }
});

EntityImported is an event that should say how many entities have been processed already, and how much entities I should process. My worries lie around the thread safety of incrementing count inside the lambda, and what steps you would recommend towards ensuring that the event is always fired with the correct value of the count variable.

Jon Skeet
people
quotationmark

It's not at all thread-safe at the moment.

You can use Interlocked.Increment(ref count) instead, but it's generally better to have a "local" value for each thread, and sum them at the end. That way you don't need to have any sort of cross-thread data flow, other than allocating the items to be processed.

There's an overload of Parallel.ForEach designed for just this purpose - look at the example near the bottom of the documentation, as it's doing something very similar to your code. (It maintains a local count, and then sums the counts at the end.) Unfortunately that doesn't help in your particular case due to the way you need to raise an event on each iteration - but usually that's a better approach.

In this case, you should use the result of Interlocked.Increment in your event raising code:

Parallel.ForEach(result.Data, data =>
{
    try
    {
        EventRange importedEntity = ImportEntity(auxResult.EntityName, data);
        int newCount = Interlocked.Increment(ref count);
        EntityImported(importedEntity, newCount, totalData);
    }
    catch (Exception e)
    {
        exceptions.Enqueue(e);
    }
});

That way there will be exactly one event raised for each count (so one with 0, one with 1, one with 2 etc).

people

See more on this question at Stackoverflow