Home > Mobile >  Does boost atomic reference counting example contain a bug?
Does boost atomic reference counting example contain a bug?

Time:05-06

I'm referring to this example. The authors use memory_order_release to decrement the counter. And they even state in the discussion section that using memory_order_acq_rel instead would be excessive. But wouldn't the following scenario in theory lead to that x is never deleted?

  • we have two threads on different CPUs
  • each of them owns an instance of a shared pointer, both pointers share ownership over the same control block, no other pointers referring that block exist
  • each thread has the counter in its cache and the counter is 2 for both of them
  • the first thread destroys its pointer, the counter in this thread now is 1
  • the second thread destroys its pointer, however cache invalidation signal from the first thread may still be queued, so it decrements the value from its own cache and gets 1
  • both threads didn't delete x, but there are no shared pointers sharing our control block => memory leak

The code sample from the link:

#include <boost/intrusive_ptr.hpp>
#include <boost/atomic.hpp>

class X {
public:
  typedef boost::intrusive_ptr<X> pointer;
  X() : refcount_(0) {}

private:
  mutable boost::atomic<int> refcount_;
  friend void intrusive_ptr_add_ref(const X * x)
  {
    x->refcount_.fetch_add(1, boost::memory_order_relaxed);
  }
  friend void intrusive_ptr_release(const X * x)
  {
    if (x->refcount_.fetch_sub(1, boost::memory_order_release) == 1) {
      boost::atomic_thread_fence(boost::memory_order_acquire);
      delete x;
    }
  }
};

The quote from discussion section:

It would be possible to use memory_order_acq_rel for the fetch_sub operation, but this results in unneeded "acquire" operations when the reference counter does not yet reach zero and may impose a performance penalty.

CodePudding user response:

All modifications of a single atomic variable happen in a global modification order. It is not possible for two threads to disagree about this order.

The fetch_sub operation is an atomic read-modify-write operation and is required to always read the value of the atomic variable immediately before the modification from the same operation in the modification order.

So it is not possible for the second thread to read 2 when the first thread's fetch_sub was first in the modification order. The implementation must assure that such a cache incoherence cannot happen, if necessary with the help of locks if the hardware doesn't support this atomic access natively. (That is what the is_lock_free and is_always_lock_free members of the atomic are there to check for.)

This is all independent of the memory orders of the operations. These matter only for access to other memory locations than the atomic variable itself.

  • Related