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

compiler_fence example is misleading #133014

Open
lukas-code opened this issue Nov 13, 2024 · 0 comments
Open

compiler_fence example is misleading #133014

lukas-code opened this issue Nov 13, 2024 · 0 comments
Labels
A-atomic Area: Atomics, barriers, and sync primitives A-concurrency Area: Concurrency A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team

Comments

@lukas-code
Copy link
Member

lukas-code commented Nov 13, 2024

Location

https://doc.rust-lang.org/nightly/core/sync/atomic/fn.compiler_fence.html#examples

Summary

use std::sync::atomic::{AtomicBool, AtomicUsize};
use std::sync::atomic::Ordering;
use std::sync::atomic::compiler_fence;

static IMPORTANT_VARIABLE: AtomicUsize = AtomicUsize::new(0);
static IS_READY: AtomicBool = AtomicBool::new(false);

fn main() {
    IMPORTANT_VARIABLE.store(42, Ordering::Relaxed);
    // prevent earlier writes from being moved beyond this point
    compiler_fence(Ordering::Release);
    IS_READY.store(true, Ordering::Relaxed);
}

fn signal_handler() {
    if IS_READY.load(Ordering::Relaxed) {
        assert_eq!(IMPORTANT_VARIABLE.load(Ordering::Relaxed), 42);
    }
}

This example makes it look like there is some kind of guarantee for a release fence that isn't paired with an acquire operation or fence, which as far as I can tell, is not the case. (See the C++ docs for regular (thread) fences that do not guarantee anything for an unpaired release fence and compiler (signal) fences that are described as strictly weaker than thread fences.)

So in this example there is no happens-before relation between the store of IMPORTANT_VARIABLE in main and the load thereof in the signal handler and therefore the signal handler may actually observe IS_READY == true and IMPORTANT_VARIABLE == 0.

Presumably the example should have a load fence in the signal handler and explain in the prose text above that the loads can be reordered, too:

fn signal_handler() {
    if IS_READY.load(Ordering::Relaxed) {
+       // prevent later reads from being moved beyond this point
+       compiler_fence(Ordering::Acquire);
        assert_eq!(IMPORTANT_VARIABLE.load(Ordering::Relaxed), 42);
    }
}

related: #129189

@lukas-code lukas-code added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Nov 13, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 13, 2024
@lukas-code lukas-code added A-concurrency Area: Concurrency T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-atomic Area: Atomics, barriers, and sync primitives T-opsem Relevant to the opsem team and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: Atomics, barriers, and sync primitives A-concurrency Area: Concurrency A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team
Projects
None yet
Development

No branches or pull requests

2 participants