Is `std::atomic<>` necessary or overkill for `std::condition_variable`?

Question

Suppose there is a simple buffer which serves as the transfer point for some data between a producer and consumer thread. From reading around I have put together the following:

const std::chrono::time_point<std::chrono::steady_clock> TIMEOUT;
std::condition_variable waitForData;
std::mutex hasDataMutex;
std::atomic<bool> hasData;

void blocking_read(){
    std::unique_lock<std::mutex> lock(hasDataMutex);
    if (!hasData) {
        waitForData.wait_until(lock, TIMEOUT,
                           [&]() -> bool { return hasData; });
    }
}

void put_data(char* buf) {
    //...write data into local buffer
    std::unique_lock<std::mutex> lock(hasDataMutex);
    hasData = true;
    waitForData.notify_one();
}

Assuming the variables were properly initialized: is it necessary, or in the reverse, superfluous to have hasData declared atomic? Or is it even possible to get away without the mutex if I use atomic?

Is there a freely available "Multithreading in C++" write-up somewhere that you would recommend?

Answer 1

1. Using both atomic and mutex is overkill here. Worth noting that there are situations where it's useful, like Singleton double-checked locking.

2. If you have C++20, you can use atomic::wait() instead of mutex and conditional_variable.

Answer 2

The way you have written the code it is not necessary for _hasData to be atomic, but you could write the code like this, in which case it is necessary:

const std::chrono::time_point<std::chrono::steady_clock> _TIMEOUT;
std::condition_variable _waitForData;
std::mutex _hasDataMutex;
std::atomic<bool> _hasData;

void blocking_read(){
    if (!_hasData.load()) {
        std::unique_lock<std::mutex> lock(_hasDataMutex);
        _waitForData.wait_until(lock, _TIMEOUT,
                           [&]() -> bool { return _hasData.load(); });
    }
}

void put_data(char* buf) {
    //...write data into local buffer
    _hasData.store(true);
    _hasDataMutex.lock();
    _hasDataMutex.unlock();
    _waitForData.notify_all();
}

This has the advantage that checking _hasData in blocking_read does not have to acquire the mutex if it is already set. Note that we also don't have to hold the mutex when setting _hasData to true, and that it is also not necessary to hold the mutex when calling notify_all. However, we still need to acquire the mutex after we have set _hasData and before calling notify_all to prevent missed notifications. Otherwise a thread in blocking_read could check _hasData before we set it to true, but only enter wait_until after we called notify_all and thus wait forever.

Note that I adjusted the code to use explicit load/store calls - I prefer this to make it clear that a variable is atomic.

Whether this is actually worth it or not depends on your situation. If all you need is a simple atomic flag like in this example, I would also recommend to use atomic::wait, but this is only available from C++20 onward.

Update: Apparently it may still not be clear why this is sufficient, so let be try to elaborate.

Suppose we have two threads - thread A is calling put_data while thread B is calling blocking_read. The two functions have the following sequences of operations:

• blocking_read:
  _hasData.load() -> _hasDataMutex.lock() -> _waitForData.wait_until()
• put_data:
  _hasData.store() -> _hasDataMutex.lock() -> _hasDataMutex.unlock() -> _waitForData.nofify()

Note that in blocking_read the wait_until will check the predicate before going to sleep, and that it implicitly calls _hasDataMutex.unlock(). The operations on the mutex are serialized, so we have either: A:_hasDataMutex.unlock() -> B:_hasDataMutex.lock() or B:_hasDataMutex.unlock() -> A:_hasDataMutex.lock(), in other words, if A first acquires the mutex, it must unlock it before B can acquire it, or the other way round. Let's discuss these two cases in more detail.

Suppose we have A:_hasDataMutex.unlock() -> B:_hasDataMutex.lock(). Thread A is calling put_data, so we have the following happens before relation: A:_hasData.store() -> A:_hasDataMutex.unlock() -> B:_hasDataMutex.lock() -> B:_waitForData.wait_until() So the predicated in wait_until is guaranteed to see the updated value is _hasData.

Suppose we have B:_hasDataMutex.unlock() -> A:_hasDataMutex.lock(). Thread B is executing blocking_read, so this unlock happens implicitly in the wait_until, atomically with the thread being put to sleep. So we have the following relation: B:_waitForData.wait_until() -> A:_hasDataMutex.lock() -> A:_hasDataMutex.unlock() -> A:_waitForData.nofify() So in this situation it is guaranteed, that thread B is already asleep and will receive the notification from thread A.

As Peter Cordes has correctly pointed out, we can use more relaxed memory orders. Initially I omitted that for simplicity, but here is the complete example with relaxed memory orders:

const std::chrono::time_point<std::chrono::steady_clock> _TIMEOUT;
std::condition_variable _waitForData;
std::mutex _hasDataMutex;
std::atomic<bool> _hasData;

void blocking_read(){
    if (!_hasData.load(std::memory_order_acquire)) {
        std::unique_lock<std::mutex> lock(_hasDataMutex);
        _waitForData.wait_until(lock, _TIMEOUT,
                           [&]() -> bool { return _hasData.load(std::memory_order_relaxed); });
    }
}

void put_data(char* buf) {
    //...write data into local buffer
    _hasData.store(true, std::memory_order_release);
    _hasDataMutex.lock();
    _hasDataMutex.unlock();
    _waitForData.notify_all();
}

The first load in blocking_read has to use memory_order_acquire, so that if we already observe the value written by put_data, we establish a happens-before relation (and for that we also need the release-order for the store). The load in the wait_until predicate can be completely relaxed, because in that case the happens-before relation is established via the mutex as described above.