Another question on thread safe ref counting

Question

There are tons of questions on how to implement thread safe reference counters. And a common highly voted answer is: "use atomic increment/decrements". Ok, this is a good way to read and write refCounter whitout other thread changing it in between. But.

My code is:

void String::Release()
{
    if ( 0 == AtomicDecrement( &refCounter ) ) )
        delete buffer;
}

So. I decrement and read refCounter in safe. But what if other thread will INCREMENT my refCounter while I am comparing it to zero????

Am I wrong?

EDIT: (example)

String* globalString = new String(); // refCount == 1 after that.

// thread 0:
delete globalString; 
  // This invokes String::Release().
  // After AtomicDecrement() counter becomes zero. 
  // Exactly after atomic decrement current thread switches to thread 1.

// thread 1:
String myCopy = *globalString;
  // This invokes AddRef(); 
  // globalString is alive;
  // internal buffer is still not deleted but refCounter is zero;
  // We increment and switch back to thread 0 where buffer will be 
  // succefully deleted;

Am I wrong?

Answer 1

Your example sounds right to me.

However, the issue here is not about atomic operations, but manually deleting an object and then referencing a soon-to-be-deleted object. What if the reference count, instead of being 1, is 8?.

You need to avoid deleting and invalidating the object manually, and better use some smart pointers implementation aware of concurrency to handle the reference counting.

Whenever a pointer detects the refcount to be zero you need to lock the object to avoid being referenced by other thread, much like the double-checked locking for initializing the new reference.

Answer 2

Be careful !

It is not enough to protect a variable like a reference counter that manage the life cycle of something bigger.

I've seen code like the one in your question that ends up pretty bad ...

In your case is not only that someone could increment the counter after your comparison, but some thread can get the counter with value 1, then you decrement and DELETE the buffer and the other thread use deleted memory ... CRASH

my2c