Path: ...!news.mixmin.net!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: Paavo Helde <eesnimi@osa.pri.ee>
Newsgroups: comp.lang.c++
Subject: Re: Atomic caching of smart pointers
Date: Thu, 26 Sep 2024 13:14:02 +0300
Organization: A noiseless patient Spider
Lines: 198
Message-ID: <vd3c5a$4sp5$1@dont-email.me>
References: <vc7ahq$2akr4$1@dont-email.me> <vc7f5o$2atht$5@dont-email.me>
 <vca82m$32puo$1@dont-email.me> <vca8v5$327mo$1@dont-email.me>
 <vcb5ij$3c3g5$1@dont-email.me> <vcb5r9$3c740$1@dont-email.me>
 <vcb65v$3c73m$1@dont-email.me> <vcbhoq$3epub$1@dont-email.me>
 <vd2sln$2lkc$1@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Injection-Date: Thu, 26 Sep 2024 12:14:03 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="25af6294e0cdc6ebb8363fbc0b8f79fa";
	logging-data="160549"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18U61Ra1JgKQQkZC5XIAJcudkYHsYdaxTk="
User-Agent: Mozilla Thunderbird
Cancel-Lock: sha1:ZFtMIWw2zYkfrpH9CMqPamixgts=
In-Reply-To: <vd2sln$2lkc$1@dont-email.me>
Content-Language: en-US
Bytes: 8246

On 26.09.2024 08:49, Chris M. Thomasson wrote:
> On 9/17/2024 2:22 AM, Paavo Helde wrote:
>> On 17.09.2024 09:04, Chris M. Thomasson wrote:
>>> On 9/16/2024 10:59 PM, Chris M. Thomasson wrote:
>>>> On 9/16/2024 10:54 PM, Paavo Helde wrote:
>>> [...]
>>>>>>>>> template<typename T>
>>>>>>>>> class CachedAtomicPtr {
>>>>>>>>> public:
>>>>>>>>>      CachedAtomicPtr(): ptr_(nullptr) {}
>>>>>>>>>
>>>>>>>>>      /// Store p in *this if *this is not yet assigned.
>>>>>>>>>      /// Return pointer stored in *this, which can be \a p or not.
>>>>>>>>>      Ptr<T> AssignIfNull(Ptr<T> p) {
>>>>>>>>>          const T* other = nullptr;
>>>>>>>>>          if (ptr_.compare_exchange_weak(other, p.get(), 
>>>>>>>>> std::memory_order_release, std::memory_order_acquire)) {
>>>>>>>>>              p->IncrementRefcount();
>>>>>>>>>              return p;
>>>>>>>>>          } else {
>>>>>>>>>              // wrap in an extra smartptr (increments refcount)
>>>>>>>>>              return Ptr<T>(other);
>>>>>>>>>          }
>>>
>>> ^^^^^^^^^^^^^^^^^^
>>>
>>> Is Ptr<T> an intrusive reference count? I assume it is.
>>
>> Yes. Otherwise I could not generate new smartpointers from bare T*.
>>
>>
>> FYI, here is my current full compilable code together with a test 
>> harness (no relacy, could not get it working, so this just creates a 
>> number of threads which make use of the CachedAtomicPtr objects in 
>> parallel.
>>
>> #include <cstddef>
>> #include <atomic>
>> #include <iostream>
>> #include <stdexcept>
>> #include <deque>
>> #include <mutex>
>> #include <thread>
>> #include <vector>
>>
>> /// debug instrumentation
>> std::atomic<int> gAcount = 0, gBcount = 0, gCASFailureCount = 0;
>> /// program exit code
>> std::atomic<int> exitCode = EXIT_SUCCESS;
>>
>> void Assert(bool x) {
>>      if (!x) {
>>          throw std::logic_error("Assert failed");
>>      }
>> }
>>
>> class RefCountedBase {
>> public:
>>      RefCountedBase(): refcount_(0) {}
>>      RefCountedBase(const RefCountedBase&): refcount_(0) {}
>>      RefCountedBase(RefCountedBase&&) = delete;
>>      RefCountedBase& operator=(const RefCountedBase&) = delete;
>>      RefCountedBase& operator=(RefCountedBase&&) = delete;
>>
>>      void Capture() const noexcept {
>>          ++refcount_;
>>      }
>>      void Release() const noexcept {
>>          if (--refcount_ == 0) {
>>              delete const_cast<RefCountedBase*>(this);
>>          }
>>      }
>>      virtual ~RefCountedBase() {}
>>
>>
>> private:
>>      mutable std::atomic<std::size_t> refcount_;
>> };
>>
>> template<class T>
>> class Ptr {
>> public:
>>      Ptr(): ptr_(nullptr) {}
>>      explicit Ptr(const T* ptr): ptr_(ptr) { if (ptr_) { ptr_- 
>>  >Capture(); } }
>>      Ptr(const Ptr& b): ptr_(b.ptr_) { if (ptr_) { ptr_->Capture(); } }
>>      Ptr(Ptr&& b) noexcept: ptr_(b.ptr_) { b.ptr_ = nullptr; }
>>      ~Ptr() { if (ptr_) { ptr_->Release(); } }
>>      Ptr& operator=(const Ptr& b) {
>>          if (b.ptr_) { b.ptr_->Capture(); }
>>          if (ptr_) { ptr_->Release(); }
>>          ptr_ = b.ptr_;
>>          return *this;
>>      }
>>      Ptr& operator=(Ptr&& b) noexcept {
>>          if (ptr_) { ptr_->Release(); }
>>          ptr_ = b.ptr_;
>>          b.ptr_ = nullptr;
>>          return *this;
>>      }
>>      const T* operator->() const { return ptr_; }
>>      const T& operator*() const { return *ptr_; }
>>      explicit operator bool() const { return ptr_!=nullptr; }
>>      const T* get() const { return ptr_; }
>> private:
>>      mutable const T* ptr_;
>> };
>>
>> template<typename T>
>> class CachedAtomicPtr {
>> public:
>>      CachedAtomicPtr(): ptr_(nullptr) {}
>>      /// Store p in *this if *this is not yet assigned.
>>      /// Return pointer stored in *this, which can be \a p or not.
>>      Ptr<T> AssignIfNull(Ptr<T> p) {
>>          const T* other = nullptr;
>>          if (ptr_.compare_exchange_strong(other, p.get(), 
>> std::memory_order_release, std::memory_order_acquire)) {
>>              p->Capture();
> 
> Only one thread should ever get here, right? It just installed the 
> pointer p.get() into ptr_, right?

Yes, that's the idea. The first thread which manages to install non-null 
pointer will increase the refcount, others will fail and their objects 
will be released when refcounts drop to zero.

> 
> 
>>              return p;
>>          } else {
>>              ++gCASFailureCount;
>>              return Ptr<T>(other);
> 
>>          }
>>      }
>>      Ptr<T> Load() const {
>>          return Ptr<T>(ptr_);
>>      }
> 
> Now this is the crux of an potential issue. Strong thread safety allows 
> for a thread to take a reference even if it does not already own one. 
> This is not allowed in basic thread safety.
> 
> So, for example this scenario needs strong thread safety:
> 
> static atomic_ptr<foo> g_foo(nullptr);
> 
> thread_a()
> {
>      g_foo = new foo();
> }
> 
> thread_b()
> {
>      local_ptr<foo> l_foo = g_foo;
>      if (l_foo) l_foo->bar();
> }
> 
> 
> thread_c()
> {
>      g_foo = nullptr;
> }
> 

In my usage case I do not have thread_c() because nobody is changing the 
pointer any more after it is set. It is kept alive while the 
CachedAtomicPtr object is alive. Load() will increment the refcounter, 
so the pointed B object will stay alive even if CachedAtomicPtr is 
destroyed. My test harness is checking this scenario as well.

> 
> This example does not work with shared_ptr, but should work with 
> atomic<shared_ptr>, it should even be lock-free on archs that support 
========== REMAINDER OF ARTICLE TRUNCATED ==========