CODEWITHSUNDEEP
c#multithreadingtask-parallel-library
Jussi Kosunen
Jussi Kosunen

Reputation: 8307

Is writing to a bool variable from multiple threads safe in C#?

I need to check if any iterations of a Parallel.ForEach reach a specific point. From my understanding the following would be safe if my bool was a volatile field, but it needs to be a variable in the enclosing method:

bool anySuccess = false;
Parallel.ForEach(things, thing =>
{
    // stuff happens...
    anySuccess = true;
});
if(!anySuccess)
    throw new Exception("No things succeeded :(");

Can I use this code as-is or should I use a lock or Interlocked function?

Upvotes: 4

Views: 2036

Answers (2)

ma11achy
ma11achy

Reputation: 132

I would lock the bool. Also, I would suggest moving your if statement into the Parallel.Foreach block. Reason:

This will evaluate after the parallel loop, thus reading only the last update of anySuccess

bool anySuccess = false;
Parallel.ForEach(things, thing =>
{
    // stuff happens...
    anySuccess = true;
});
if(!anySuccess)
    throw new Exception("No things succeeded :(");

This will evaluate all updates of the bool

Object myLock = new Object();

bool anySuccess = false;
Parallel.ForEach(things, thing =>
{
    // stuff happens...
    lock (myLock) 
    {
        anySuccess = true;
    }
    if(!anySuccess)
        throw new Exception("No things succeeded :(");
});

Upvotes: -2

Brondahl
Brondahl

Reputation: 8557

If that code is the ENTIRETY of the loops' access to that boolean, then yes that seems safe to me.

In general basic value types aren't thread safe in the sense that many of the operations performed on them aren't atomic.

But if the ONLY thing you're ever doing is assigning to that variable ... never reading it, never changing branch based on it, never writing to it based on it's current state ... and all the writes are the same (the ONLY modification that can happen is to set it to true) then I don't see any way for the non-atomicity to cause a problem.

==ADDITION==

The above comments on the correctness of the current code. It is also worth considering the long-term safeness of the code in it's context as part of your codebase. Leaving that code as it is, is setting up a trap for a future developer who doesn't know/understand/recognise what is going on, and why it's CURRENTLY safe.

If you leave it like this, it will be essential to put a CLEAR comment on both the declaration and the single usage explaining what's going on, why it's safe, and why they musn't start reading/using the variable in other ways.

The alternative (adding locking code) is long-term safer, but likely to also be very slightly less performant.

Upvotes: 4

Related Questions