cplusplus / CWG

Core Working Group
23 stars 7 forks source link

LWG3980 [atomics.ref.ops] p19 Where is the wording that forbids the read operation to read the same initial value? #423

Closed xmh0511 closed 9 months ago

xmh0511 commented 10 months ago

Full name of submitter (unless configured in github; will be published with the issue): Jim X

[atomics.types.operations] p23 says:

bool compare_exchange_weak(T& expected, T desired, memory_order success, memory_order failure) noexcept;

Effects: Retrieves the value in expected. It then atomically compares the value representation of the value pointed to by this for equality with that previously retrieved from expected, and if true, replaces the value pointed to by this with that in desired. If and only if the comparison is true, memory is affected according to the value of success, and if the comparison is false, memory is affected according to the value of failure. When only one memory_order argument is supplied, the value of success is order, and the value of failure is order except that a value of memory_order​::​acq_rel shall be replaced by the value memory_order​::​acquire and a value of memory_order​::​release shall be replaced by the value memory_order​::​relaxed. If and only if the comparison is false then, after the atomic operation, the value in expected is replaced by the value pointed to by this during the atomic comparison. If the operation returns true, these operations are atomic read-modify-write operations ([intro.multithread]) on the memory pointed to by this. Otherwise, these operations are atomic load operations on that memory.

Consider this example:

#include <iostream>
#include <atomic>
#include <thread>
struct SpinLock{
    std::atomic<bool> atomic_;
    void lock(){
       bool expected = false;
           while (!atomic_.compare_exchange_strong(expected,true,std::memory_order_release,std::memory_order_relaxed)){

       }
    }
    void unlock(){
      atomic_.store(false, std::memory_order_release);
    }
};
int main(){
    SpinLock spin{false};
        auto t1 = std::thread([&](){
        spin.lock();
        spin.unlock();
    });
    auto t2 = std::thread([&](){
        spin.lock();
        spin.unlock();
    });
    t1.join();
    t2.join();
}

This is the essence of implementing the "spinlock". However, in this example, spin is initialized with the value false, So, can the comparison in the two threads both read this initial value false and compare equally with the expected value false? IMO, there is no formal wording in the standard that forbids this possible. [intro.races] p14 just says:

The value of an atomic object M, as determined by evaluation B, shall be the value stored by some side effect A that modifies M, where B does not happen before A.

If they cannot, where does the relevant wording specify that this is impossible?

xmh0511 commented 10 months ago

[atomics.order] p10 maybe a relevant rule

Atomic read-modify-write operations shall always read the last value (in the modification order) written before the write associated with the read-modify-write operation.

However, this wording can have two interpretations

  1. The check of the violation is done before the side effect of the RMW is in the modification order
  2. The check of the violation is done after the side effect of the RMW is in the modification order

With the first interpretation, the initial value false is indeed the last value before the point when the two RMW operations' write are added to the modification order. Instead, with the second interpretation, the last RMW operation(in the modification order) reads the initial value that won't be the last value at the point when the side effect of the RMW is in the modification order.

The second interpretation presumably is the intent of the wording. It may be clearer if we chang it to the following

Suggest Resolution

Atomic read-modify-write operations shall always read the value from a side effect X, where X immediately precedes the side effect of the read-modify-write operation in the modification order.

Eisenwave commented 10 months ago

Related: Stack Overflow / Can the read operations in compare_exchange_strong in different two thread read the same value?

xmh0511 commented 10 months ago

Related: Stack Overflow / Can the read operations in compare_exchange_strong in different two thread read the same value?

The wording needs to be improved to make it readable. In the current wording, "before" has some ambiguities as said in the issue.

jensmaurer commented 10 months ago

Note "atomically" in [atomics.types.operations] p23. The read-compare-write is a single, indivisible operation ("atomically"). That's inconsistent with the idea that a second read-compare-write somehow would partially overlap (on the same atomic object).

xmh0511 commented 10 months ago

"atomically" does not have a formal interpretation in the current draft. Moreover, based on your interpretation of "atomically", the "atomically" seems to only apply to comparison, in which the read operation is unavoidable, not sure whether the "atomically" ultimately applies to that read. The "atomically" even needs to have meaning "non-interrupt" in this case.

However, the load operation is also marked with "atomically"

Returns: Atomically returns the value pointed to by this.

IMO, there can be more than one load operation to concurrently read the atomic object, "atomically" seems to not prevent one operation from excluding others.

As @Eisenwave linked the related question, the answers seem to agree [atomics.order] p10 is that formal interpretation of the original issue, however, this wording is quite subtle and needs some decoration.


The read-compare-write is a single

As exposed in Stackover flow / For purposes of ordering, is atomic read-modify-write one operation or two? and the answer in the question linked by @Eisenwave

People have less or more confusion in this aspect, for example, the opinion in the answer

The current wording avoids treating the atomic RMW as a single operation, because it's not when we consider operations on other objects

In certain circumstances, the read and write operations in a CAS_strong are separated.

t3nsor commented 9 months ago

This issue is now https://cplusplus.github.io/LWG/issue3980 so I think it could be closed here.