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?

Answer 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.

Answer 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();
}

Answer 3

The solution is to use std::recursive_mutex instead of std::mutex.

Other answers have already given the correct solution, but I would like to point out two other things:

1. Just because volatile was not designed for thread safety, it doesn't mean it has nothing to do with thread safety. In the original post, volatile bool alreadyLocked; is correct and enough.

2. This is by no means a "code smell"! When a system gets complex, recursive lock to a mutex is sometimes inevitable. If this were a "code smell" or could be resolved by a better design, the std::recursive_mutex would be a joke and should be dumped out of the C++ standard library. But it is still there, why?