Thought Clutter

Recently, I've been writing a fair amount of threaded C++ POSIX code, and (as I often do), I decided to wrap the POSIX mutexes in a convenient little wrapper class:

 
class Mutex
{
    pthread_mutex_t mMutex;
public:
    Mutex() { pthread_mutex_init(&mMutex, NULL); }
    ~Mutex() { pthread_mutex_destroy(&mMutex); }
 
    void Lock() { pthread_mutex_lock(&mMutex); }
    void Unlock() { pthread_mutex_unlock(&mMutex); }
    bool TryLock() { return (pthread_mutex_trylock(&mMutex) == 0)?(true):(false); }
};
 

I know others' opinions differ, but I personally like wrappers like this. They allow me a nice way to make sure that all my Mutexes are friendly C++ objects, play well as member variables, etc.

While writing some code, I caught myself writing a number of functions like the following:

 
void MyClass::MyFunc()
{
    mMutex.Lock();
    if (SomeCondition())
    {
        if(SomeOtherCondition())
        {
            // etc...
        }
    }
    mMutex.Unlock();
}
 

Now, whenever I write code like the above function, it makes my skin crawl. First of all, I hate writing nested if statements. Secondly, and more importantly, the above code is exception unsafe in that if any of the code before the mMutex.Unlock() throws an exception, the mutex won't be unlocked correctly.

First I thought that since I don't care about the exception in MyFunc(), I could get away with:

 
void MyClass::MyFunc()
{
    mMutex.Lock();
    try
    {
        // etc...
        mMutex.Unlock();
    }
    catch(...)
    {
        mMutex.Unlock();
        throw;
    }
}
 

But, I hate the multiple calls to unlock the mutex, the unneeded try/catch block, and just everything about this code.

After some thinking, I came to a conclusion, that in hindsight must be obvious to most C++ programmers out there:

 
class SafeLock
{
    Mutex &mMutex;
public:
    SafeLock(Mutex &inMutex) : mMutex(inMutex) { mMutex.Lock(); }
    ~SafeLock() { mMutex.Unlock(); }
};
 

Now all I need to do is the following:

 
void MyClass::MyFunc()
{
    SafeLock lock(mMutex);
    if(!SomeCondition()) return;
    if(!SomeOtherCondition()) return;
    // etc...
}
 

Additionally, I have found that SafeLock makes it easier on me to call functions that require a mutex to be held before calling it. I just pass references to the SafeLock to all functions that require the mutex held:

 
void MyFuncThatRequiresMutex(const SafeLock &lock)
{
    (void)lock;
    // etc...
}