Correct way to implement simple thread safe generic mutex

Question

I'm wondering if my way of implementing a generic mutex is a good software design pattern, and is it thread safe?

Here is my mutex class:

#ifdef HAVE_WINDOWS_H
void *CSimpleMutex::object;
#endif
#ifdef _PTHREADS
pthread_mutex_t CSimpleMutex::object;
#endif

CSimpleMutex::CSimpleMutex(bool lockable)
{
    isLockableMutex = lockable;
    if (!lockable)
    {
#ifdef _WINDOWS
        object = CreateMutex(NULL, false, NULL);
#endif
#ifdef _PTHREADS
        pthread_mutex_init(&object, NULL);
#endif
    }
    else
    {
#ifdef _WINDOWS
        InitializeCriticalSection(&mutex);
#endif
#ifdef _PTHREADS
        pthread_mutex_init(&mutex, NULL);
#endif
    }
}

CSimpleMutex::~CSimpleMutex()
{
    if (!isLockableMutex)
    {
#ifdef _WINDOWS
        if(object!=NULL)
        {
            CloseHandle(object);
        }
#endif
#ifdef _PTHREADS
        pthread_mutex_destroy(&object);
#endif
    }
    else
    {
#ifdef _WINDOWS
        DeleteCriticalSection(&mutex);
#endif
#ifdef _PTHREADS
        pthread_mutex_destroy(&mutex);
#endif  
    }
}

// Aquires a lock
void CSimpleMutex::Lock()
{
    if (!isLockableMutex)
        return;
#ifdef _WINDOWS
    EnterCriticalSection(&mutex);
#endif
#ifdef _PTHREADS
    pthread_mutex_lock(&mutex);
#endif
}

// Releases a lock
void CSimpleMutex::Unlock()
{
    if (!isLockableMutex)
        return;
#ifdef _WINDOWS
    LeaveCriticalSection(&mutex);
#endif
#ifdef _PTHREADS
    pthread_mutex_unlock(&mutex);
#endif
}

This is how it is used:

class CEnvironment : public CHandleBase
{
    private:
        CSimpleMutex *mutex;
    public:
        CEnvironment(){mutex = new CSimpleMutex(true);};
        ~CEnvironment(){delete mutex;};
        void Lock() { mutex->Lock(); };
        void Unlock() { mutex->Unlock(); };
        void DoStuff(void *data);
};

When I want to use CEnvironment I do something like this:

env->Lock();
env->DoStuff(inData);
env->Unlock();

Note that Windows mutexes (those created by `CreateMutex`) are cross-process mutexes, so each locking/unlocking operation necessitates a system call to the kernel, even when uncontested, which can be expensive. If you only need mutexes for within your process, use [`CRITICAL_SECTION` objects](http://msdn.microsoft.com/en-us/library/windows/desktop/ms682530%28v=vs.85%29.aspx) instead, which have very fast uncontested locks which don't require system calls. — Adam Rosenfield, Jul 05 '12 at 19:57
@ybungalobill See http://stackoverflow.com/questions/800383/what-is-the-difference-between-mutex-and-critical-section — , Jul 05 '12 at 20:21
@IngeHenriksen: Irrelevant question. The term is "inter-process", not "lockable". — Yakov Galka, Jul 06 '12 at 07:51

score 1 · Accepted Answer · answered Jul 05 '12 at 20:24

You'd typically use RAII to acquire and release a lock on entering and leaving a given scope, respectively, so that the lock is automatically released if an exception is thrown within that scope. In your case, you could define an addition lock "guard" class that acquires the lock in its constructor and releases in its destructor. The C++11 standard, for example, defines several mutex classes and a std::lock_guard class that provide the behavior I just described.

The paper "Strategized Locking, Thread-safe Interface, and Scoped Locking" by Doug Schmidt provides additional design details and ideas that may be of use.

Correct way to implement simple thread safe generic mutex

1 Answers1