C++: How To Simplified Thread Lock?

Question

I'm new to C++. While learning how does threading work, I found it is quite annoying to call WaitForSingleObject(x) at beginning and ReleaseMutex(x) at the end. So I wrote a class to do that for me, but I'm not sure the impact and am I doing it right. Wonder is there a more simplified way to achieve the same? Here's how I do it:

class MutexLock {
public:
    MutexLock(HANDLE hMutex) {
        m_hMutex = hMutex;
    }
    void    Lock() {
        WaitForSingleObject((m_hMutex), INFINITE);
    }

~MutexLock() {
    if (m_hMutex != NULL) {
        ReleaseMutex(m_hMutex);
        std::cout << "Mutex released." << std::endl;
    }
}

private:
    HANDLE  m_hMutex;
};

And how I use the class:

class TestMutex
{
public:
    TestMutex(void) {
        m_mutex = CreateMutex(NULL, FALSE, NULL);
        std::cout << "Mutex created." << std::endl;
    }
    ~TestMutex(void) {
        if (m_mutex != NULL)
            CloseHandle(m_mutex);
    }

    void    Func1(void) {
        MutexLock ml(m_mutex);
        ml.Lock();

        std::cout << "Func1: Owning mutex." << std::endl;
        std::cout << "Press enter key to end this." << std::endl;
        ReadKey(GetStdHandle(STD_INPUT_HANDLE));
    }
    void    Func2(void) {
        MutexLock ml(m_mutex);
        ml.Lock();
        //std::cout << "Press enter key to start this." << std::endl;
        //ReadKey(GetStdHandle(STD_INPUT_HANDLE));

        std::cout << "Func2: Owning mutex." << std::endl;
        std::cout << "Press enter key to end this." << std::endl;
        ReadKey(GetStdHandle(STD_INPUT_HANDLE));
    }

private:
    HANDLE  m_mutex;
};

In the main function:

int _tmain(int argc, _TCHAR* argv[])
{
    TestMutex * tm = new TestMutex();

    HANDLE aThread[2];
    for (int i = 0; i < 2; i++)
    {
        aThread[i] = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)ThreadProc, (LPVOID)tm, 0, 0);
    }

    WaitForMultipleObjects(2, aThread, TRUE, INFINITE);

    for (int i = 0; i < 2; i++)
    {
        CloseHandle(aThread[i]);
    }

    delete tm;
    ReadKey(GetStdHandle(STD_INPUT_HANDLE));

    return 0;
}

Is this the way normally how others do thread lock?

Answer 1

I do not believe it can get much "simpler" than what you have. You need to mutually exclude shared variables that are being modified by numerous processes. You could possibly have a "read only" type of situation where a mutex is not needed but that will not help your simplification.

Having the mutex release on its destruction(when it falls out of scope) is probably the best way and I have seen numerous threading libraries use that exact same method. Portable Tools Library, PTLib, is an abstraction library that includes threading abstraction and it releases mutexes when they fall out of scope but you still have to use them. However, you should also keep track of the number of calls and releases against the mutex so that you can signal the other thread when it is available.

Also, as Bgie pointed out in his answer, you do need to protect your code. Never trust other programmers, that includes your future self.

But your idea of releasing a lock when the scope is left is a good first general implementation, just needs some additional work :).

(Edit due to Bgie's comment)

Answer 2

Actually, you are allowing the consumer of your class API to misuse it, when you can easily prevent this. He has to call lock exactly once after construction. He might not know this, and forget to call it, or call it twice.

Simpler and less error-prone would be to make the lock method private and call it from the constructor.

But, as other commenters have written, the best is to use an existing library. In addition to what others mentioned, Qt also has a nice QMutexLocker class.