Is it necessary to use a std::atomic to signal that a thread has finished execution?

Question

I would like to check if a std::thread has finished execution. Searching stackoverflow I found the following question which addresses this issue. The accepted answer proposes having the worker thread set a variable right before exiting and having the main thread check this variable. Here is a minimal working example of such a solution:

#include <unistd.h>
#include <thread>

void work( bool* signal_finished ) {
  sleep( 5 );
  *signal_finished = true;
}

int main()
{
  bool thread_finished = false;
  std::thread worker(work, &thread_finished);

  while ( !thread_finished ) {
    // do some own work until the thread has finished ...
  }

  worker.join();
}

Someone who commented on the accepted answer claims that one cannot use a simple bool variable as a signal, the code was broken without a memory barrier and using std::atomic<bool> would be correct. My initial guess is that this is wrong and a simple bool is sufficient, but I want to make sure I'm not missing something. Does the above code need a std::atomic<bool> in order to be correct?

Let's assume the main thread and the worker are running on different CPUs in different sockets. What I think would happen is, that the main thread reads thread_finished from its CPU's cache. When the worker updates it, the cache coherency protocol takes care of writing the workers change to global memory and invalidating the main thread's CPU's cache so it has to read the updated value from global memory. Isn't the whole point of cache coherence to make code like the above just work?

Why don't you use a condition variable or semaphore or autoresetevent to signal the thread? That's what these things are for. — Tony The Lion, Jan 16 '13 at 18:55
There could be a problem if the compiler did some optimization, based on the fact that you're testing a variable's value over and over again, and end up modifying the behaviour of the application. I haven't seen that happen ever, but I heard that could be a reason to use atomics rather than simple bools. — mfontanini, Jan 16 '13 at 18:56
Gosh, when we wrote the threading requirements for C++11 we **completely forgot** about the magic of cache coherency algorithms! — Pete Becker, Jan 16 '13 at 19:03
@TonyTheLion: Conditional variables, semaphores and events are for when you want to wait (suspend the thread) until something has occurred. He just wants to test if something has occured, so an atomic bool is more appropriate. — Andrew Tomazos, Jan 16 '13 at 19:14
for a related question: http://stackoverflow.com/q/12507705/819272 — TemplateRex, Jan 16 '13 at 20:10
and see also the comments to this answer: http://stackoverflow.com/a/12087141/819272 — TemplateRex, Jan 16 '13 at 20:12
http://stackoverflow.com/questions/9224542/is-a-memory-barrier-required-if-a-second-thread-waits-for-termination-of-the-fir — Zan Lynx, Jan 21 '16 at 06:02

score 23 · Accepted Answer · answered Jan 16 '13 at 19:04

Someone who commented on the accepted answer claims that one cannot use a simple bool variable as a signal, the code was broken without a memory barrier and using std::atomic would be correct.

The commenter is right: a simple bool is insufficient, because non-atomic writes from the thread that sets thread_finished to true can be re-ordered.

Consider a thread that sets a static variable x to some very important number, and then signals its exit, like this:

x = 42;
thread_finished = true;

When your main thread sees thread_finished set to true, it assumes that the worker thread has finished. However, when your main thread examines x, it may find it set to a wrong number, because the two writes above have been re-ordered.

Of course this is only a simplified example to illustrate the general problem. Using std::atomic for your thread_finished variable adds a memory barrier, making sure that all writes before it are done. This fixes the potential problem of out-of-order writes.

Another issue is that reads to non-volatile variables can be optimized out, so the main thread would never notice the change in the thread_finished flag.

Important note: making your thread_finished volatile is not going to fix the problem; in fact, volatile should not be used in conjunction with threading - it is intended for working with memory-mapped hardware.

+1 For mentioning volatile and memory mapped hardware. Not enough people understand this :) — Jesus Ramos, Jan 16 '13 at 19:15
`volatile` will stop the bool from being optimized out, but it wont provide the membar, or guarantee any timeframe for checking the cache coherency with main memory. atomic bool is the right move. — Andrew Tomazos, Jan 16 '13 at 19:19
"making sure that all writes *before it* are done" makes it sound like it's a case of either `x = 42; membar; thread_finished = true; ` or `x = 42; thread_finished = true; membar;` - the former wouldn't ensure timely visibility of `thread_finished`'s update, the latter may risk re-ordered exposure before the `membar` kicks in. membar may actually be a prefix/modifier per `x = 42; membar(thread_finished = true);`, ensuring relative "<=" visibility ordering *and* timely visibility of both updates, or need to be used twice per `x = 42; membar; thread_finished = true; membar`. Cheers. — Tony Delroy, Jul 16 '14 at 09:16

score 7 · Answer 2 · edited Jun 20 '20 at 09:12

Using a raw bool is not sufficient.

The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior. § 1.10 p21

Two expression evaluations conflict if one of them modifies a memory location (1.7) and the other one accesses or modifies the same memory location. § 1.10 p4

Your program contains a data race where the worker thread writes to the bool and the main thread reads from it, but there is no formal happens-before relation between the operations.

There are a number of different ways to avoid the data race, including using std::atomic<bool> with appropriate memory orderings, using a memory barrier, or replacing the bool with a condition variable.

score 3 · Answer 3 · answered Jan 16 '13 at 19:03

It's not ok. Optimizer can optimize

  while ( !thread_finished ) {
    // do some own work until the thread has finished ...
  }

to:

  if(!thread_finished)
    while (1) {
      // do some own work until the thread has finished ...
    }

assuming it can prove, that "some own work" doesn't change thread_finished.

score 2 · Answer 4 · answered Jan 16 '13 at 19:04

Cache coherency algorithms are not present everywhere, nor are they perfect. The issue surrounding thread_finished is that one thread tries to write a value to it while another thread tries to read it. This is a data race, and if the accesses are not sequenced, it results in undefined behavior.

Is it necessary to use a std::atomic to signal that a thread has finished execution?

4 Answers4

Linked