2 of 3
added 322 characters in body
Water
  • 3.3k
  • 3
  • 28
  • 60

C++ member update visbility inside a critical section when not atomic

I stumbled across the following Code Review StackExchange and decided to read it for practice. In the code, there is the following:

Note: I am not looking for a code review and this is just a copy paste of the code from the link so you can focus in on the issue at hand without the other code interfering. I am not interested in implementing a 'smart pointer', just understanding the memory model:

// Copied from the link provided (all inside a class)

unsigned int count;
mutex m_Mutx;

void deref()
{
    m_Mutx.lock();
    count--;
    m_Mutx.unlock();
    if (count == 0)
    {
        delete rawObj;
        count = 0;
    }
}

Seeing this makes me immediately think "what if two threads enter when count == 1 and neither see the updates of each other? Can both end up seeing count as zero and double delete? And is it possible for two threads to cause count to become -1 and then deletion never happens?

The mutex will make sure one thread enters the critical section, however does this guarantee that all threads will be properly updated? What does the C++ memory model tell me so I can say this is a race condition or not?

I looked at the Memory model cppreference page and std::memory_order cppreference, however the latter page seems to deal with a parameter for atomic. I didn't find the answer I was looking for or maybe I misread it. Can anyone tell me if what I said is wrong or right, and whether or not this code is safe or not?

For correcting the code if it is broken:

Is the correct answer for this to turn count into an atomic member? Or does this work and after releasing the lock on the mutex, all the threads see the value?

I'm also curious if this would be considered the correct answer:

Note: I am not looking for a code review and trying to see if this kind of solution would solve the issue with respect to the C++ memory model.

#include <atomic>
#include <mutex>

struct ClassNameHere {
    int* rawObj;
    std::atomic<unsigned int> count;
    std::mutex mutex;

    // ...

    void deref()
    {
        std::scoped_lock lock{mutex};
        count--;
        if (count == 0)
            delete rawObj;
    }
};
Water
  • 3.3k
  • 3
  • 28
  • 60