Search code examples
c++multithreadingatomicmemory-barriersstdatomic

Mixing memory orders with/out conditionals

Consider the following:

std::atomic_bool disabled = true;
int counter1 = 0, counter2 = 1;

[Thread 1]
while (...)
{
    counter2 = ...;
    if (disabled.load(std::memory_order::acquire) && counter2 > counter1) //#1
    {
        disabled.store(false, std::memory_order::relaxed);
        disabled.notify_one();
    }
}

[Thread 2]
while (...)
{
    disabled.wait(true, std::memory_order::relaxed);
    //Do stuff
    counter1 = ...;
    disabled.store(true, std::memory_order::release); //#2
}

Is this code guaranteed to behave correctly, i.e. will the load in #1 synchronize with the store in #2 so that the later condition will see the correct value of counter1? Can I get away with using relaxed order for the wait and store(false) or will I have to change them to acquire/release as well?

Related scenario:

std::atomic_int a = 0;
x = 0;

[Thread 1]
x = 1;
a.store(1, std::memory_order::release);
a.store(2, std::memory_order::relaxed);

[Thread 2]
assert(a.load(std::memory_order::acquire) != 2 || x == 1);

In this case, can the assert fail? There are two possibilities I can think of:

  1. The 2nd store is able to float above the 1st
  2. No reordering but the store(relaxed) happening after the 1st breaks the release/acquire relationship in the standard. Or in other words: will the last happened-before release synchronize with an acquire (on the same variable of course), given other load/stores with weaker order in between?

For example, for seq_cst cppreference.com says:

as soon as atomic operations that are not tagged memory_order_seq_cst enter the picture, the sequential consistency guarantee for the program is lost

The context of this question is the standard/memory model and not specific architecture.

Solution

  • (2) is UB since C++20

    The 2nd store is able to float above the 1st

    No, a thread isn't allowed to reorder access to an atomic variable relative to other accesses to the same variable. Only (in some cases) relative to other variables.

    No reordering but the store(relaxed) happening after the 1st nullifies the release/acquire relationship in the standard. Or in other words: will the last happened-before release synchronize with an acquire (on the same variable of course), given other load/stores with weaker order in between?

    Since C++20 there's no synchronization, so this is a data race. Pre-C++20 this was legal, but apparently didn't actually work on some hardware (in more complex scenarios than this), hence the change.

    (1) looks fine. But if you put the contents of the second thread in a loop (as implied by otherwise unused wait()), then I believe you get UB.

    counter1 = ...; happens before counter2 > counter1, so no race here.

    But if the second thread is in a loop, when counter1 = ...; happens the second time, that's a data race with preceding counter2 > counter1. Replacing relaxed with acquire/release fixes that.