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

Why is std::memory_order_relaxed enough for atomic_conditional_increment? #107

Open
kakashidinho opened this issue Mar 11, 2023 · 3 comments

Comments

@kakashidinho
Copy link

kakashidinho commented Mar 11, 2023

Hi, I'm just trying to understand the rationale behind the using of std::memory_order_relaxed in sp_counted_base_std_atomic.hpp's atomic_conditional_increment's implementation.

atomic_conditional_increment's implementation (click to expand)
inline std::int_least32_t atomic_conditional_increment( std::atomic_int_least32_t * pw ) BOOST_SP_NOEXCEPT
{
    // long r = *pw;
    // if( r != 0 ) ++*pw;
    // return r;

    std::int_least32_t r = pw->load( std::memory_order_relaxed );

    for( ;; )
    {
        if( r == 0 )
        {
            return r;
        }

        if( pw->compare_exchange_weak( r, r + 1, std::memory_order_relaxed, std::memory_order_relaxed ) )
        {
            return r;
        }
    }    
}

AFAIK this function is used by weak_ptr to obtain the shared_ptr if the obj is still alive (ref count is still > 0).
Consider the following case:

shared_ptr A;
if ((A = weak_ptr.lock()) != nullptr) // internally does `if (atomic_conditional_increment() != 0)`
{
   // modify object pointed by A
}

Shouldn't we use std::memory_order_acquire to ensure that the modification of object pointed by A won't be reordered before the nullptr check?

@pdimov
Copy link
Member

pdimov commented Mar 11, 2023

The nullptr check and the modification would be on the same side of the acquire anyway, so they can still be "reordered".

"Reordered" is not the right way to think about this anyway, because operations are never observably reordered within the same thread. In order for "reordering" to be observed, there needs to be another thread that can perceive events in a different order.

Try adding another thread to the example and see if you can demonstrate a problem.

@kakashidinho
Copy link
Author

kakashidinho commented Mar 11, 2023

Honestly I'm not an expert in C++ memory order. So I'm using this question in hope it could help me understand a bit better.

I meant the modification could be reordered to before the ref count is increased successfully/unsuccessfully by compare_exchange_weak.
What if at the same time other thread does the decrement of ref count to zero, and deletes the object? Could it be possible to happen?
Though this most likely leads to a wrong prediction, and the processor could throw away the modification's results on the 1st thread anw. If I understand correctly how processor's branch prediction works.

@pdimov
Copy link
Member

pdimov commented Mar 12, 2023

What if at the same time other thread does the decrement of ref count to zero, and deletes the object? Could it be possible to happen?

No, because if this happens first, the atomic_conditional_increment will fail, and if it happens second, the reference count will not be zero so the object won't be destroyed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants