Is it good practice / thread safe to negate the value of a bool directly after if-checking it in C#?

Question

I asked myself whether it is possible to write an if statement that checks for a bool variable and then directly changes its value before entering the statement's body.

So, instead of writing

if (getNextImage)
{
    getNextImage = false;
    // do some more stuff                     
}

I tried this

if (getNextImage ? !(getNextImage = false) : false) 
{
    // do some more stuff 
}

which worked.

The reason for this is that the code runs in several threads at the same time and I wanted to be sure that the if block is just executed once in the first thread coming to that line in the code.

My question is: Is this approach good practice and will it work the way I intended it to? If not, are there other approaches besides a lock?

No, it won't work the way you intend it to. `// do some more stuff` may occur more than once. — mjwills, Aug 02 '19 at 13:34
Generally for threading situations, `lock` or `Interlocked` should be your go to solutions. https://stackoverflow.com/questions/29411961/c-sharp-and-thread-safety-of-a-bool — mjwills, Aug 02 '19 at 13:35
Neither snippet is thread-safe. The value can easily change from expression to the next. Either wrap the block in a lock or use [Interlocked.CompareExchange](https://learn.microsoft.com/en-us/dotnet/api/system.threading.interlocked.compareexchange?view=netframework-4.8#System_Threading_Interlocked_CompareExchange__1___0____0___0_) to compare and replace the value in a single atomic operation — Panagiotis Kanavos, Aug 02 '19 at 13:38
https://stackoverflow.com/questions/6661055/using-interlocked-compareexchange-operation-on-a-bool-value/18027246 — Dmitry Bychenko, Aug 02 '19 at 13:40
What are you trying to do? There are **far** better ways to coordinate execution. For example, you could use an ActionBlock or Channel to post work to a worker function — Panagiotis Kanavos, Aug 02 '19 at 13:41
@PanagiotisKanavos In a `NewFrame` event handler of a video source, I want to check whether the user has clicked the "Take photo" button (sets `getNextImage` to true). If he has, I want to save the latest video source frame in a file, hence the bool variable. — dall, Aug 05 '19 at 12:01

score -1 · Answer 1 · answered Aug 02 '19 at 13:40

-1

I'm assuming that you intend to change the value of a global variable if it satisfies your condition. You gonna have problems for the reasons that you think, that would be confuse among many users. As a solution, you can use Lock (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement), or you can create an Async method (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/async), which is better (lock will make your application slow, likely).

answered Aug 02 '19 at 13:40

Eric

11
3

4

The async keyword doesn't make anything run asynchronously, nor does it coordinate asynchronous operations. It's just syntactic sugar that allows the *await* keyword to await already running asynchronous operations without blocking – Panagiotis Kanavos Aug 02 '19 at 13:42

score -1 · Answer 2 · edited Aug 05 '19 at 08:33

With Thread Safety, the question often comes down to avoiding Update Race Conditions. In that area, your code unfortunately offers not an iota of protection.

As another post said, the lock statement is one way to go. It is customary to create an object "mutex" for the sole purpose to lock onto.

object mutex = new object();

lock (mutex){
  //the code that can not run in parallel
}

There are two reasons for this in your case:

You can not lock on a value type. The value type will be boxed, but in a different instance every time you box it. And lock requires that the same instance is used during locking and unlocking.
You do not want to use the instance you return. Because someone else might have the fancy idea to lock on to that. And then any call to your locking call would cause a deadlock.

Hence that object with this single purpose.

Of course, in your case the locked area might be to - in lack of a better term - wide. After the read and write, you no longer need the lock on the value. The value itself will become the lockout, after all. So, maybe you should go to the underlying values.

Another option would be something like this:

bool LocalGetNextImage = false;

lock(mutex){
  if(GlobalGetNextImage){
     GlobalGetNextImage = false;
     LocalGetNextImage = true;
  }
}

This would quickly release the lock, while still giving the thread exclusive rights to read and write the value. And have a bool value to work with later.

Is it good practice / thread safe to negate the value of a bool directly after if-checking it in C#?

2 Answers2