How do I make sure there is only 1 mutex?

Question

Hi guys. I am running some thread safe code here. I am using a mutex to protect the section of code that needs to be run only by only 1 thread at a time. The problem I have is using this code sometimes I end up with 2 Mutex objects. This is a static function by the way. How do I make sure only 1 mutex object gets created??

/*static*/ MyClass::GetResource()
{

if (m_mutex == 0)
{
// make a new mutex object
m_mutex = new MyMutex();
}

m_mutex->Lock();

Answer 1

The issue is the thread could be interrupted after checking if m_mutex is 0, but not before it creates the mutex, allowing another thread to run through the same code.

Don't assign to m_mutex right away. Create a new mutex, and then do an atomic compare exchange.

You don't mention your target platform, but on Windows:

MyClass::GetResource()
{
    if (m_mutex == 0)
    {
        // make a new mutex object
        MyMutex* mutex = new MyMutex();

        // Only set if mutex is still NULL.
        if (InterlockedCompareExchangePointer(&m_mutex, mutex, 0) != 0)
        {
           // someone else beat us to it.
           delete mutex;
        }
    }
    m_mutex->Lock();

Otherwise, replace with whatever compare/swap function your platform provides.

Another option is to use one-time initialization support, which is available on Windows Vista and up, or just pre-create the mutex if you can.

Answer 2

Simply create m_mutex outside of GetResource(), before it can ever be called - this removes the critical section around the actual creation of the mutex.

MyClass::Init()
{
  m_mutex = new Mutex;
}    

MyClass::GetResource()
{
  m_mutex->Lock();
  ...
  m_mutex->Unlock();
}

Answer 3

Why use a pointer anyway? Why not replace the pointer with an actual instance that does not require dynamic memory management? This avoids the race condition, and does not impose a performance hit on every call into the function.

Answer 4

Lazy mutex initialization isn't really appropriate for static methods; you need some guarantee that nobody races to the initialization. The following uses the compiler to generate a single static mutex for the class.

/* Header (.hxx) */
class MyClass
{
    ...

  private:
    static mutable MyMutex m_mutex;  // Declares, "this mutex exists, somewhere."
};


/* Compilation Unit (.cxx) */
MyMutex MyClass::m_mutex;            // The aforementioned, "somewhere."

MyClass::GetResource()
{
    m_mutex.Lock();
    ...
    m_mutex.Unlock();
}

Some other solutions will require extra assumptions of your fellow programmers. With the "call init()" method, for instance, you have to be sure that the initialization method were called, and everybody would have to know this rule.