Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

atomic::Ordering docs provide fewer guarantees than C++20 reference #104814

Open
JanBeh opened this issue Nov 24, 2022 · 7 comments
Open

atomic::Ordering docs provide fewer guarantees than C++20 reference #104814

JanBeh opened this issue Nov 24, 2022 · 7 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools

Comments

@JanBeh
Copy link
Contributor

JanBeh commented Nov 24, 2022

Problem description

The documentation in std::sync::atomic and std::sync::atomic::Ordering (source in Rust 1.65) describe that the Orderings are equal to those defined in C++20:

In std::sync::atomic:

Each method takes an Ordering which represents the strength of the memory barrier for that operation. These orderings are the same as the C++20 atomic orderings. For more information see the nomicon.

In std::sync::atomic::Ordering:

Rust’s memory orderings are the same as those of C++20.
[…]
Relaxed: […] Corresponds to memory_order_relaxed in C++20.
Release: […] Corresponds to memory_order_release in C++20.
Acquire: Corresponds to memory_order_acquire in C++20.
AcqRel […] Corresponds to memory_order_acq_rel in C++20.
SeqCst […] Corresponds to memory_order_seq_cst in C++20.

However, at the same time, the documentation of Ordering::Release and Ordering::Acquire (source in Rust 1.6.5) lists the guarantees made by these Orderings. In particular:

Release:

When coupled with a store, all previous operations become ordered before any load of this value with Acquire (or stronger) ordering. In particular, all previous writes become visible to all threads that perform an Acquire (or stronger) load of this value.

Acquire:

When coupled with a load, if the loaded value was written by a store operation with Release (or stronger) ordering, then all subsequent operations become ordered after that store. In particular, all subsequent loads will see data written before the store.

These guarantees seem to be less than what the C++20 standard guarantees. Thus the explanation of these orderings differs from the C++20 standard.

Consider the following example, where a: AtomicUsize, for example:

One thread does:

a.store(10, Release);
a.fetch_add(1, Relaxed);

A second thread does:

if a.load(Acquire) == 11 {
    /* … */
}

Suppose the second thread reads the value 11 written by the fetch_add operation in the first thread. As this value was not written by a store operation with Release or stronger ordering (but with Relaxed ordering), there are no guarantees that thread two will see data written before any store in thread one.

Looking into the C++20 reference, however, reveals a concept named "Release sequences":

After a release operation A is performed on an atomic object M, the longest continuous subsequence of the modification order of M that consists of […] atomic read-modify-write operations made to M by any thread is known as release sequence headed by A.

To my understanding, the fetch_add in thread one is part of the release sequence headed by the store.

Looking into the Working Draft N4861 of the C++ Standard (the final revision isn't available for free), we see that on page 1525:

An atomic operation A that performs a release operation on an atomic object M synchronizes with an atomic operation B that performs an acquire operation on M and takes its value from any side effect in the release sequence headed by A.

Thus a.store in thread one would synchronize with the a.load in thread two, even though the rules described in Rust's documentation of std::sync::atomic::Ordering do not allow this reasoning; i.e. following solely the rules in Rust's documenation, one would (wrongly) assume that there is no synchronization between those two threads if thread two reads the value of 11 written by the fetch_add operation.

Additional issues with accessibility

Overall, it's very difficult for someone who starts with the Rust documentation to get an overview on what the Rust atomics really do. The C++20 standard isn't available for free and the linked C++ reference (C++20 atomic orderings) uses but does not define the "synchronizes-with" relationship (which is vital for following/understanding the atomic orderings).

Intent of the Rust documentation

Before making any fixes to the documentation, it should be clarified whether the difference between the Rust documentation and the C++20 reference exists intentionally, i.e. is Rust deliberately giving less guarantees to the programmer than the C++20 standard does? (Even though still following the C++20 memory model in practice "as of now".) Or is the Rust documentation incomplete here?

Possible improvements

Understanding atomics seems to be very complex, and it might be difficult to fully cover all details in the documentation of std. However, the description of the Orderings in Rust's documentation should not differ from those given in the C++ reference.

Possible solutions could be:

  • Explicitly state that the guarantees listed under each enum variant of std::sync::atomic::Ordering are non-exhaustive (not to be confused with the non_exhaustive property of the enum itself) and that the C++20 standard is the normative source.
  • Correctly explain the behavior in regard to release sequences.
@JanBeh JanBeh added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Nov 24, 2022
@JanBeh
Copy link
Contributor Author

JanBeh commented Nov 24, 2022

See also thread "Better understanding atomics" on URLO.

@JanBeh
Copy link
Contributor Author

JanBeh commented Nov 25, 2022

Also see PR #378 for the Nomicon for an exhaustive proposal to provide better documentation for atomics.

@cbeuw
Copy link
Contributor

cbeuw commented Nov 25, 2022

Rust does have release sequences, and all other rules in C++20 (except for the ones relating to consume). I think it's intentional that we don't have an exhaustive and stand-alone documentation on the atomic memory model, otherwise we'd be just copying C++ docs. The nomicon is meant to be (more) exhaustive but it's written as a guide not a reference.

Although the std doc isn't exhaustive, for the things it does contain, they don't conflict with C++20. For release sequences specifically

Acquire: When coupled with a load, if the loaded value was written by a store operation with Release (or stronger) ordering, then all subsequent operations become ordered after that store.

This only talks about when an Acquiring load sees a value written by a Releasing store. It says nothing about what does or doesn't happen if the Acquiring load sees a value not written by a Releasing store, so it doesn't exclude release sequences.

That said English isn't first-order logic so we shouldn't expect readers to think about vacuous truths in implications all the time... I agree we should explicitly state that there are more guarantees to be found in the C++ reference than the ones documented by us.

@ibraheemdev
Copy link
Member

ibraheemdev commented Nov 28, 2022

The C++20 standard isn't available for free

The draft (still not the ISO standard, but useful for reference) is available here.

the linked C++ reference (C++20 atomic orderings) uses but does not define the "synchronizes-with" relationship

cppreference does define synchronizes-with:

image

@JanBeh
Copy link
Contributor Author

JanBeh commented Nov 28, 2022

cppreference does define synchronizes-with

Isn't that (an early) working draft of the C++ standard (which you linked) instead of the (publicly available) C++ reference that is referenced by the Rust docs? The latter doesn't seem to be authoritative (i.e. not being the standard), but aims to be a "good specification" (see FAQ). The former, instead, specificially is issued with a warning (see table of contents):

It says: "Note: this is an early draft. It's known to be incomplet and incorrekt, and it has lots of bad formatting."

In either way, it was very difficult for me to find a defintion for "synchronizes-with", starting from the Rust std documentation and the links to cppreference.com. In particular, I don't see where I can get from this page (edit: old revision where it was missing) to an authoritative definition of "synchronizes-with", or where I can get easily to any definition of "synchronizes-with" from that page. This makes understanding that page pretty difficult.

Maybe this could be improved by editing the en.cppreference.com Wiki.

But I still think Rust's docs should either

  • specify the atomic orderings correctly, matching the same definition as found in the C++20 standard,
  • explicitly note that the guarantees given in the Rust docs are non-exhaustive, and that the C++20 standard is authoritative in that matter and that any guarantees given in the C++20 standard are guaranteed by Rust as well, or
  • note that there are deliberately less guarantees given than those given in the C++20 standard in order to be able to make future changes without breaking downward compatibility.

I don't think that the latter is the case, but given the Rust docs, I cannot be entirely sure as a reader.

@ibraheemdev
Copy link
Member

I don't see where I can get from this page to an authoritative definition of "synchronizes-with", or where I can get easily to any definition of "synchronizes-with" from that page

Screenshot_2022-11-28-10-49-41-44_3aea4af51f236e4932235fdada7d1643.jpg

@JanBeh
Copy link
Contributor Author

JanBeh commented Nov 28, 2022

@ibraheemdev: Ooops, I totally missed that. I was sure I had checked it several times, but I guess I got confused and probably only searched for "synchronizes-with" with a minus-hyphen insead of a space. Sorry for the confusion.


I re-checked, and the section actually wasn't there when I started writing this issue (or got added at around the same time). It was introduced here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools
Projects
None yet
Development

No branches or pull requests

3 participants