do sequentially-consistent atomic loads (load-load pair) form an inter-thread synchronisation point?

Question

I am trying to understand what does sequentially-consistent ordering mean for loads. Consider this artificial example:

#include <atomic>
#include <thread>
#include <cassert>

static std::atomic<bool> preStop {false};
static std::atomic<bool> stop {false};
static std::atomic<int> counter{0};

void waiter() {
    preStop.store(true, std::memory_order_relaxed);
    while (counter.load() > 0);
    stop.store(true, std::memory_order_relaxed);
}

void performer() {
    while (true) {
        counter.fetch_add(1);
        const bool visiblePreStop = preStop.load();
        if (stop.load()) {
            assert(visiblePreStop);
            return;
        }
        counter.fetch_sub(1);
        std::this_thread::yield();
    }
}

int main() {
    std::thread performerThread(performer);
    std::thread waiterThread(waiter);
    waiterThread.join();
    performerThread.join();
}

Can assert fail? Or does counter.fetch_add() synchronise with counter.load()?

It is my understanding that had operations on counter have std::memory_order_relaxed or std::memory_order_acq_rel, the load-load pair would not create a synchronisation point. Does std::memory_order_seq_cst make any difference for load-load pairs?

Answer 1

The assert can fail. The sequential consistency of counter.load() implies that it is acquire, but that does not prevent the relaxed preStop.store(true) from being reordered after it. Then preStop.store(true) may also be reordered after stop.store(true). If we have a weakly ordered machine with a store buffer, then nothing in waiter() ever drains the store buffer, so preStop.store() may languish there for an arbitrarily long time.

If so, then it is entirely possible that the code does something like

waiter
======
preStop.store(true, relaxed); // suppose not globally visible for a while
while (counter.load() > 0);   // terminates immediately
stop.store(true, relaxed);    // suppose globally visible right away  

performer
=========
counter.fetch_add(1);
preStop.load(); // == false
stop.load(); // == true

I don't quite understand the rest of the question, though. Synchronization is established by a release store and an acquire load that reads the stored value (or another value later in the release sequence); when this occurs, it proves that the store happened before the load. Two loads cannot synchronize with each other, not even if they are sequentially consistent.

But counter.fetch_add(1) is a read-modify-write; it consists of a load and a store. Since it is seq_cst, the load is acquire and the store is release. And counter.load() is likewise acquire, so if it returns 1, it does synchronize with counter.fetch_add(1), proving that counter.fetch_add(1) happens before counter.load().

But that doesn't really help. The problem is that waiter doesn't do any release stores at all, so nothing in performer can synchronize with it. Therefore neither of the relaxed stores in waiter can be proved to happen before the corresponding loads in performer, and so we cannot be assured that either load will return true. In particular it is quite possible that preStop.load() returns false and stop.load() returns true.

Answer 2

You have a problem where you're reading two different atomic variables but expect their state to be inter-dependent. This is similar, but not the same, as Time-of-check to time-of-use bugs.

Here's a valid interleaving where the assert fires:

1. PerformerThread (PT) created
2. WaiterThread (WT) created
3. PT executes the following:

    while (true) {
        counter.fetch_add(1);
        const bool visiblePreStop = preStop.load();

4. PT sees that visiblePreStop is false
5. PT is suspended
6. WT executes the following:

    preStop.store(true, std::memory_order_relaxed);
    while (counter.load() > 0);
    stop.store(true, std::memory_order_relaxed);

6. WT is suspended
7. PT executes the following:

        if (stop.load()) {

8. PT sees that stop is true
9. PT hits the assert because visiblePreStop is false and stop is `true