How can I lock twice with the same mutex on the same thread?

Question

I have this class (simplified):

// thing.h

#include <mutex>

class Thing
{
public:
    void process();
    void inner();

private:
    std::mutex lock;
};

// thing.cpp

#include "Thing.h"

using namespace std;

void Thing::process()
{
    lock_guard<mutex> locking(lock);

    inner();
}

void Thing::inner()
{
    lock_guard<mutex> locking(lock);
}

If I make a call to process, I get an exception:

Microsoft C++ exception: std::system_error at memory location 0x006FF16C.

Locking on the same lock in the same thread causes this exception. How can I do this without the exception? I thought about adding a flag:

volatile bool alreadyLocked;

Changing inner to:

void Thing::inner()
{
     if (!alreadyLocked)
     {
         lock_guard<mutex> locking(lock);
         alreadyLocked = true;
         ...something magic happens here...
         alreadyLocked = false;
     }
}

However this feels brittle... is there a right way to do this?

Solution 1

Firstly, volatile variables are NOT thread-safe. You must use std::atomic<T> to have thread-safe variables. volatile has nothing to do with thread safety.

To solve your issue, you can use std::recursive_mutex, which can be locked/unlocked multiple times from the same thread.

From cppreference:

A calling thread owns a recursive_mutex for a period of time that starts when it successfully calls either lock or try_lock. During this period, the thread may make additional calls to lock or try_lock. The period of ownership ends when the thread makes a matching number of calls to unlock.

When a thread owns a recursive_mutex, all other threads will block (for calls to lock) or receive a false return value (for try_lock) if they attempt to claim ownership of the recursive_mutex.

---

Additionally, please consider refactoring your code so that locking a mutex twice is not required. Improving your design could probably avoid this issue.

Solution 2

There is a coding hack to get around this design problem; it’s called a recursive mutex. But you really should fix the design problem, not try to work around it. Separate your code into two layers: all the work inside your class should be done by private member functions that don’t lock anything; the external interface should be implemented through public member functions, and they lock the mutex.

So:

class Thing {
public:
    void process();
    void inner();
private:
    void do_process();
    void do_inner();
    std::mutex mtx;
};

void Thing::process() {
    std::lock_guard<std::mutex> lock(mtx);
    do_process();
}

void Thing::inner() {
    std::lock_guard<std::mutex> lock(mtx);
    do_inner();
}

void Thing::do_process() {
    do_inner();
}

Comments

[SubMachine]

+1 for the last note. In most cases locking a mutex multiple times is a code smell