CODEWITHSUNDEEP
c++multithreadingc++17
C.M.
C.M.

Reputation: 3383

waiting on worker thread using std::atomic flag and std::condition_variable

Here is a C++17 snippet where one thread waits for another to reach certain stage:

std::condition_variable  cv;
std::atomic<bool>        ready_flag{false};
std::mutex               m;


// thread 1
... // start a thread, then wait for it to reach certain stage
auto lock = std::unique_lock(m);
cv.wait(lock, [&]{ return ready_flag.load(std::memory_order_acquire); });


// thread 2
... // modify state, etc
ready_flag.store(true, std::memory_order_release);
std::lock_guard{m};   // NOTE: this is lock immediately followed by unlock
cv.notify_all();

As I understand this is a valid way to use atomic flag and condition variable to achieve the goal. For example there is no need to use std::memory_order_seq_cst here.

Is it possible to relax this code even further? For example:

Upvotes: 6

Views: 2026

Answers (2)

Anthony Williams
Anthony Williams

Reputation: 68631

Firstly: this code is indeed valid. The lock_guard prior to the notify_one call ensures that the waiting thread will see the correct value of ready_flag when it wakes, whether that is due to a spurious wake, or due to the call to notify_one.

Secondly: if the only accesses to the ready_flag are those shown here, then the use of atomic is overkill. Move the write to ready_flag inside the scope of the lock_guard on the writer thread and use a simpler, more conventional pattern.

If you stick with this pattern, then whether or not you can use memory_order_relaxed depends on the ordering semantics you require.

If the thread that sets the ready_flag also writes to other objects which will be read by the reader thread, then you need the acquire/release semantics in order to ensure that the data is correctly visible: the reader thread may lock the mutex and see the new value of ready_flag before the writer thread has locked the mutex, in which case the mutex itself would provide no ordering guarantees.

If there is no other data touched by the thread that sets the ready_flag, or that data is protected by another mutex or other synchronization mechanism, then you can use memory_order_relaxed everywhere, as it is only the value of ready_flag itself that you care about, and not the ordering of any other writes.

atomic_thread_fence doesn't help with this code under any circumstances. If you are using a condition variable, then the lock_guard{m} is required.

Upvotes: 3

LWimsey
LWimsey

Reputation: 6677

The combined use of a std:atomic and std:condition_variable is unconventional and should be avoided, but it can be interesting to analyse the behavior if you come across this in a code review and need to decide if a patch is required.

I believe there are 2 problems:

  1. Since ready_flag is not protected by the std:mutex, you cannot rely on the guarantee that thread 1 will observe the updated value once wait wakes up from notify_one. If the store to ready_flag in thread 2 is delayed by the platform, thread 1 may see the old value (false) and enter wait again (possibly causing a deadlock).
    Whether a delayed store is possible depends on your platform. On a strongly ordered platform such as X86, you are probably safe, but again, no guarantees from the C++ standard.
    Also note that using a stronger memory ordering does not help here.

  2. let's say, the store is not delayed and once wait wakes up, ready_flag loads true.
    This time, based on the memory ordering you are using, the store to ready_flag in thread 2, synchronizes with the load in thread 1 which can now safely access the modified state written by thread 2.

    But, this only works one time. You cannot reset ready_flag and write to the shared state again. That would introduce a data race since the shared state can now be accessed unsynchronized by both threads

Is it possible to relax this code even further

Because you are modifying the shared state outside the lock, release/acquire ordering on ready_flag is necessary for synchronization.

To make this a portable solution, access both the shared state and ready_flag while protected by the mutex (ready_flag can be a plain bool). This is how the mechanism is designed to be used.

std::condition_variable  cv;
bool                     ready_flag{false}; // not atomic
std::mutex               m;


// thread 1
... // start a thread, then wait for it to reach certain stage
auto lock = std::unique_lock(m);
cv.wait(lock, [&] { return ready_flag; });
ready_flag = false;
// access shared state


// thread 2
auto lock = std::unique_lock(m);
... // modify state, etc
ready_flag = true;
lock.unlock();  // optimization
cv.notify_one();

Unlocking the mutex before the call to notify_one is an optimization. See this question for more details.

Upvotes: 1

Related Questions