Is branching on a std::atomic variable a possible source of bugs?

Question

I heard that using if or switch on a std::atomic is a source of bugs, due to some arguments to do with memory ordering.

Specifically, code like:

std::atomic<bool> branchVar = false;
void secondThreadFunc() {
  puts("Starting 2nd thread");
  branchVar = true;
}

int main() {

  std::thread t(secondThreadFunc);
  Sleep(100);
  
  // Branching on a std::atomic considered dangerous due to memory ordering issues?
  if( branchVar ) {
    puts("Branch true");
  }
  else {
    puts("Branch false");
  }

  t.join();
}

Plants a race condition at the if(branchVar) call and is considered unsafe programming. A std::mutex should always be used to lock access of the branch variable instead

Is this true? Could someone explain to me why that is so? What about memory ordering would cause the std::atomic branch to cause a race condition?

Alex Guteniev · Accepted Answer · 2024-04-09 12:59:57Z

There's nothing wrong with branching on atomic variables.

It is common to have a loop that reads atomic variable, and exits if it reads as some value. It is called spinning. Spinning is basically a branch backwards on an atomic variable.

Spinning is often used in implementation of synchronization primitives, including std::mutex or std::call_once. (You may not see it directly in the STL implementation, as this is usually done in a lower level).

When you use atomic variable directly in if or switch expression, implicit conversion happens, which is equivalent to .load() without parameters, and the memory order is implicitly memory_order_seq_cst. It is the safest memory order, so will not introduce any new issues. The use of atomic may still be problematic, as there may be data races with other data, but it depends on your code, and doesn't have anything to do with the atomic itself.

The advice to use std::mutex, etc, may come form the fact that std::atomic on its own provides atomicity only for itself. So if you modify atomic and some other data in a thread, and then branch on atomic, there may be a data race related to the other data modification. Or may be no data race, if you do everything right.

Peter Cordes · Accepted Answer · 2024-04-24 01:10:37Z

The whole point of writing lock-free code with atomics is that your code has to work for whatever order of operations actually happens on any given run, preferably doing whatever is useful given the current state you see rather than sitting there waiting. (Sitting there waiting is what locks do.)

For example https://preshing.com/20130605/the-worlds-simplest-lock-free-hash-table/ - insert attempts might have to retry a different location if another thread won the race to claim that slot, but it can try again right away instead of waiting for the other writer to finish (like if you'd protected a std::unordered_map with a std::mutex).

It's up to you to make sure that your code does what you want for all allowed orderings.

Just like for a single-threaded program, branching on user input is "dangerous" because your program has to correctly handle every possible combination of user input. It's not inherently dangerous, it only increases the surface area for bugs.

It is harder to test lock-free code, because compiling for some specific ISA often makes some reorderings impossible. Especially x86, where the hardware memory model is basically acq_rel so some kinds of reorderings can only happen at compile time.

See also https://preshing.com/20120612/an-introduction-to-lock-free-programming/

Collectives™ on Stack Overflow

Is branching on a std::atomic variable a possible source of bugs?

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related