Skip to content

Dangerously misleading introduction to std::sync::atomic::Ordering #55196

Closed
@vorner

Description

@vorner
Contributor

When looking at https://doc.rust-lang.org/std/sync/atomic/enum.Ordering.html, this is in the intro:

At its most restrictive, "sequentially consistent" atomics allow neither reads nor writes to be moved either before or after the atomic operation;

This is not true (even the explanations for variants disprove this, but if someone reads only the intro, they could get to the conclusion that using SeqCst is good enough everywhere, which it actually isn't).

Eg, from the docs below:

SeqCst: Like AcqRel with the additional guarantee that all threads see all sequentially consistent operations in the same order.

AcqRel: For loads it uses Acquire ordering. For stores it uses the Release ordering.

So, atomic.load(Ordering::SeqCst) has Acquire semantics (+ participating in the global timeline of all SeqCst operations), which means reads and writes are allowed to be reordered after the operation. And even failed CAS operation is effectively just a read, so operations are allowed to be reordered after the operation.

I have the vague memory someone was updating it recently to actually point out these gotchas, but I can't find that now.

I can try rewording it, but I'm not really that good at writing good concise documentation.

Activity

nagisa

nagisa commented on Oct 19, 2018

@nagisa
Member

See also #54962

ehuss

ehuss commented on Oct 20, 2018

@ehuss
Contributor

The nightly docs has some updated wording from #53106.

vorner

vorner commented on Oct 20, 2018

@vorner
ContributorAuthor

It has some updated wording, but in the details below. But the intro wasn't changed there and it is still misleading. And if someone reads „SeqCst is allmighty“, they might just stop there and not read the rest.

But yes, that was the one I was referring to when I wrote about the vague memory.

added a commit that references this issue on Nov 28, 2018

Rollup merge of rust-lang#56023 - vorner:doc/atomic-ordering-strip, r=@…

c286be5
added a commit that references this issue on Nov 29, 2018

Rollup merge of rust-lang#56023 - vorner:doc/atomic-ordering-strip, r=@…

5d395ad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @ehuss@nagisa@vorner

        Issue actions

          Dangerously misleading introduction to std::sync::atomic::Ordering · Issue #55196 · rust-lang/rust