Re: Proposed patch to skip the last atomic decrements in _Sp_counted_base::_M_release

[Jonathan Wakely <jwakely@redhat.com>]

On 07/12/20 07:30 -0500, Maged Michael via Libstdc++ wrote:
>       void
>       _M_release() noexcept
>+      {
>+        if (!__LP64__ || sizeof(_Atomic_word) != 4 || sizeof(long long) !=
>8)
>+          {
>+            _M_release_orig();
>+            return;
>+          }
>+        _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&_M_use_count);
>+        _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&_M_weak_count);
>+        if ((__atomic_load_n((long long*)(&_M_use_count), __ATOMIC_ACQUIRE)
>+            == 0x100000001))
>+          {

I think we want a comment here explaining what it's doing:

             // Both counts are 1, so there are no weak references and
             // we are releasing the last strong reference. No other
             // threads can observe the effects of this _M_release()
             // call (e.g. calling use_count()) without a data race.


>+            _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_use_count);
>+            _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_weak_count);
>+            _M_dispose();

_M_dispose() will run the destructor for the owned object. If that
object has a reference to the shared_ptr then it can call use_count()
on it. That isn't a data race, because it's within the same thread.
Previously the object's destructor would have observed that the use
count had dropped to zero. In this new implementation, the use count
is still 1 when the object is destroyed.

Similarly, the object's destructor could create a new weak_ptr from
*this. Previously that would see that the use count is zero, and so
fail. With the new code it would succeed, and increase the weak count,
and be left with a dangling pointer after the destroy here:

>+            _M_destroy();

I think to avoid these problems you need to set both counts to 0
before calling _M_dispose(). Those can be simple non-atomic writes
though, since we know that only the current thread can observe those
values without data races.