std::atomic_bool for cancellation flag: is std::memory_order_relaxed the correct memory order?

I have a thread that reads from a socket and generates data. After every operation, the thread checks a std::atomic_bool flag to see if it must exit early.

In order to cancel the operation, I set the cancellation flag to true, then call join() on the worker thread object.

The code of the thread and the cancellation function looks something like this:

std::thread work_thread;
std::atomic_bool cancel_requested{false};

void thread_func()
{
   while(! cancel_requested.load(std::memory_order_relaxed))
      process_next_element();

}

void cancel()
{
    cancel_requested.store(true, std::memory_order_relaxed);
    work_thread.join();
}

Is std::memory_order_relaxed the correct memory order for this use of an atomic variable?

As long as there is no dependency between cancel_requested flag and anything else, you should be safe.

The code as shown looks OK, assuming you use cancel_requested only to expedite the shutdown, but also have a provision for an orderly shutdown, such as a sentinel entry in the queue (and of course that the queue itself is synchronized).

Which means your code actually looks like this:

std::thread work_thread;
std::atomic_bool cancel_requested{false};
std::mutex work_queue_mutex;
std::condition_variable work_queue_filled_cond;
std::queue work_queue;

void thread_func()
{
    while(! cancel_requested.load(std::memory_order_relaxed))
    {
        std::unique_lock<std::mutex> lock(work_queue_mutex);
        work_queue_filled_cond.wait(lock, []{ return !work_queue.empty(); });
        auto element = work_queue.front();
        work_queue.pop();
        lock.unlock();
        if (element == exit_sentinel)
            break;
        process_next_element(element);
    }
}

void cancel()
{
    std::unique_lock<std::mutex> lock(work_queue_mutex);
    work_queue.push_back(exit_sentinel);
    work_queue_filled_cond.notify_one();
    lock.unlock();
    cancel_requested.store(true, std::memory_order_relaxed);
    work_thread.join();
}

And if we're that far, then cancel_requested may just as well become a regular variable, the code even becomes simpler.

std::thread work_thread;
bool cancel_requested = false;
std::mutex work_queue_mutex;
std::condition_variable work_queue_filled_cond;
std::queue work_queue;

void thread_func()
{
    while(true)
    {
        std::unique_lock<std::mutex> lock(work_queue_mutex);
        work_queue_filled_cond.wait(lock, []{ return cancel_requested || !work_queue.empty(); });
        if (cancel_requested)
            break;
        auto element = work_queue.front();
        work_queue.pop();
        lock.unlock();
        process_next_element(element);
    }
}

void cancel()
{
    std::unique_lock<std::mutex> lock(work_queue_mutex);
    cancel_requested = true;
    work_queue_filled_cond.notify_one();
    lock.unlock();
    work_thread.join();
}

memory_order_relaxed is generally hard to reason about, because it blurs the general notion of sequentially executing code. So the usefulness of it is very, very limited as Herb explains in his atomic weapons talk.

Note std::thread::join() by itself acts as a memory barrier between the two threads.