Warn about safety of fetch_update

[Riolku]

Specifically as it relates to the ABA problem.

fetch_update is a useful function, and one that isn't provided by, say, C++. However, this does not mean the function is magic. It is implemented in terms of compare_exchange_weak, and in particular, suffers from the ABA problem. See the following code, which is a naive implementation of pop in a lock-free queue:

fn pop(&self) -> Option<i32> {
    self.front.fetch_update(Ordering::Relaxed, Ordering::Acquire, |front| {
        if front == ptr::null_mut() {
            None
        }
        else {
            Some(unsafe { (*front).next })
        }
    }.ok()
}

This code is unsound if called from multiple threads because of the ABA problem. Specifically, suppose nodes are allocated with Box. Suppose the following sequence happens:

Initial: Queue is X -> Y.

Thread A: Starts popping, is pre-empted.
Thread B: Pops successfully, twice, leaving the queue empty.
Thread C: Pushes, and `Box` returns X (very common for allocators)
Thread A: Wakes up, sees the head is still X, and stores Y as the new head.

But Y is deallocated. This is undefined behaviour.

Adding a note about this problem to fetch_update should hopefully prevent users from being misled, and also, a link to this common problem is, in my opinion, an improvement to our docs on atomics.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[m-ou-se]

Thanks!

@bors r+

[bors]

📌 Commit 3d28a1a has been approved by m-ou-se

It is now in the queue for this repository.