Locking/unlocking mutex inside private functions

Question

Imagine you have a big function that locks/unlocks a mutex inside and you want to break the function into smaller functions:

#include <pthread.h>

class MyClass : public Uncopyable
{
public:
    MyClass() : m_mutexBuffer(PTHREAD_MUTEX_INITIALIZER), m_vecBuffer() {}
    ~MyClass() {}

    void MyBigFunction()
    {
        pthread_mutex_lock(&m_mutexBuffer);

        if (m_vecBuffer.empty())
        {
            pthread_mutex_unlock(&m_mutexBuffer);
            return;
        }

        // DoSomethingWithBuffer1();

        unsigned char ucBcc = CalculateBcc(&m_vecBuffer[0], m_vecBuffer.size());

        // DoSomethingWithBuffer2();

        pthread_mutex_unlock(&m_mutexBuffer);
    }

private:
    void DoSomethingWithBuffer1()
    {
        // Use m_vecBuffer
    }

    void DoSomethingWithBuffer2()
    {
        // Use m_vecBuffer
    }

private:
    pthread_mutex_t m_mutexBuffer;
    std::vector<unsigned char> m_vecBuffer;
};

How should I go about locking/unlocking the mutex inside the smaller functions?

Should I unlock the mutex first, then lock it straightaway and finally unlock it before returning?

void DoSomethingWithBuffer1()
{
    pthread_mutex_unlock(&m_mutexBuffer);
    pthread_mutex_lock(&m_mutexBuffer);
    // Use m_vecBuffer
    pthread_mutex_unlock(&m_mutexBuffer);
}

If m_vecBuffer is empty you return and never unlock the mutex? — gx_
After your edit: your "fix" is not the right solution. Use an RAII guard object. — gx_
I already use ScopedLock RAII class in my real code but thanks gx_ for pointing that out anyway. — jpen

syam syam · Accepted Answer · 2013-09-16T13:24:51

How should I go about locking/unlocking the mutex inside the smaller functions?

If your semantics require your mutex to be locked during the whole MyBigFunction() operation then you can't simply unlock it and relock it in the middle of the function.

My best bet would be to ignore the mutex in the smaller DoSomethingWithBuffer...() functions, and simply require that these functions are called with the mutex being already locked. This shouldn't be a problem since those functions are private.

On a side note, your mutex usage is incorrect: it is not exception safe, and you have code paths where you don't release the mutex. You should either use C++11's mutex and lock classes or boost's equivalents if you are using C++03. At worst if you can't use boost, write a small RAII wrapper to hold the lock.

Locking/unlocking mutex inside private functions

2 Answers