Atomicness of copy constructor of reference count object using InterlockedIncrement64

Question

I'm trying to wrap my head around how I can ensure that an objects reference count is thread safe.

class MyObject{
  //Other implementation details
private:
  mutable volatile LONGLONG * m_count;
  IData * m_data;
};

Assume the necessary class declaration are there, just keeping it simple. Here is the implementation of the copy constructor and destructor.

MyObject::MyObject(const MyObject& rhs) : m_count(rhs.m_count), m_data(rhs.m_data){
    InterlockedIncrement64(m_count);
}

MyObject::~MyObject(){
    if(InterlockedDecrement64(m_count) == 0)
    delete m_data;
}

Is this thread safe? How is the intilization list of the copy constructor seen, atomic or not? Does that even matter? Should I be setting the incremented value of the count in the initilization list (is this possible)?

As it stands is this good enough. I'm thinking it is, otherwise, how could I get into a scenario where thread1 is copying and thread2 is destroying concurrently when the count == 1. There has to be a handshake between the threads, meaning thread1 has to copy the object entirely before thread2's object goes out of scope correct?

After reading some of these responses I went back and did a little research. Boost implements their shared_ptr very similiarly. Here is the destructor call.

void release() // nothrow
{
    if( BOOST_INTERLOCKED_DECREMENT( &use_count_ ) == 0 )
    {
        dispose();
        weak_release();
    }
}

Some have suggested that in the boost documentation that it clearly states assignment is not thread safe. I agree and disagree. I think in my case I disagree. I only need the handshake between threadA and threadB. I don't think some of the problems described in some of the replies apply here (although they were eye opening replies that I did not completely think through).

Example ThreadA atach(SharedObject); //Shared object passed by value, count incremented etc etc.

ThreadB //Accepts the object, adds it to a list of shared objects. ThreadB is on a timer that notifies all SharedObjects of an event. Before Notifying, a copy of the list is made protected by a critical section. The CS is released, the copies are notified.

ThreadA detach(SharedObject); //Removes the shared object from the list of objects

Now, concurrently ThreadB is singaling the SharedOjbect and already made a copy of the list before ThreadA detached said shared object. Everything is ok no?

Answer 1

Technically, it should be safe.

Why? Because in order to copy an object, the source needs to have a "reference", so it's not going away during the copy. Also, nobody is accessing an object that's currently under construction.

Destructor is safe too, since there are no references left anyway.

You may want to reconsider copying the reference count, though. Those references don't actually exist; everyone with a reference to the original would somehow have to decrease the refcount for the copy too, provided it got the original reference before the copy was made. Copies should start like new objects, with a refcount of 1.

EDIT: likewise, if you're implementing an assignment operator (which is like a copy into an existing object), the refcount of the destination object should be left as it is.

Answer 2

Constructor is not safe, because initialization list is not atomic (I didn't find any references to this in Standard, but it will be hardly to implement anyway).

So if another thread will delete object that is currently copied by current thread - right between initialization list execution and InterlockedIncrement() execution - you will receive broken (already deleted m_data) m_data and m_count. This will lead at least to double deletion of m_data.

Placing InterlockedIncrement to initializer list would not help because thread switching can occur after ctor call but before m_count initialization.

I'm not sure it is possible to make it thread safe without external lock (mutex or critical section). You could at least check counter in ctor and throw exception/create "invalid" object if it is zero, but this is not good design, I wouldn't recommend it.

Answer 3

This code is safe so long as the calling code ensures that the object passed by reference is not destroyed during the execution of this function. This is the same for any function that takes a reference and you would have to work very hard to not do this.

The destructor is safe because the atomic decrement is guaranteed to be zero in one and only one thread. And if it is, it must be that every other thread has already finished using the object and already invoked its own decrement operation.

This assumes your interlocked operations all have full barriers.

how could I get into a scenario where thread1 is copying and thread2 is destroying concurrently when the count == 1. There has to be a handshake between the threads, meaning thread1 has to copy the object entirely before thread2's object goes out of scope correct?

You can't, so long as each thread has its own reference to the object. The object can't be destroyed while thread1 is copying so long as thread1 has its own reference to the object. Thread1 needn't copy before thread2's reference goes away because you would never, ever touch an object unless you had a reference to it.

Individual references have weak thread safety and shouldn't be accessed in different threads concurrently. If two threads want to access the same object, they should each have their own reference. When giving a reference to an object to some other code (potentially in another thread), follow this sequence of operations:

1. Have your own reference.

2. Create the reference for the other code from your own reference.

3. Now you may destroy your reference or give away the other reference.

Answer 4

Your copy constructor is not safe, and cannot be made safe.

But you can use your class safely if you never use new/delete, but only work with objects created and destroyed automatically (by scope).