tjwrona
tjwrona

Reputation: 9035

Locking a mutex in a larger scope without causing a deadlock

I have class where one member function calls another member function twice. The function it calls locks a mutex in order to be thread safe. The way the calls work, in order for the calling function to be thread safe I need to make sure both function calls are done without another thread doing InnerFunction in between.

Basically I need the outer function to lock the same mutex that the inner function uses to ensure that no other calls to that inner function are executed before the outer function is complete.

Example:

int InnerFunction()
{
    const std::lock_guard<std::mutex> lock(m_mutex);

    // Do stuff
}

int OuterFunction()
{
    // Lock the mutex to ensure thread safety
    const std::lock_guard<std::mutex> lock(m_mutex);

    // Call the inner function twice while the mutex is locked
    // both calls need to complete before we can free the mutex or we can end up with a race condition
    InnerFunction();
    InnerFunction();
} // mutex unlocked here

The issue is, if they are locking the same mutex, when the inner function goes to obtain the lock it won't be able to since it will already be locked by the outer function. What is the best way to handle a situation like this?

Simply removing the lock from InnerFunction does not work because InnerFunction can be called by itself and it needs to lock the mutex to ensure it does not have a race condition with another call to the same function from a different thread.

Upvotes: 0

Views: 930

Answers (1)

rustyx
rustyx

Reputation: 85286

This is simply a question of design.

  1. Public functions should lock the mutex.
  2. Private functions should require a locked mutex as a precondition.
  3. Do not call public functions internally.

If InnerFunction can be invoked externally as well as internally, then it should be split into a public one that locks the mutex and a private (unlocked) one that actually does the work.

class A {
public:
    int InnerFunction() {
        std::lock_guard<std::mutex> lock(m_mutex);
        unlockedInnerFunction();
    }

    int OuterFunction() {
        std::lock_guard<std::mutex> lock(m_mutex);
        unlockedInnerFunction();
        unlockedInnerFunction();
    }

private:
    // The functions below require a locked mutex

    int unlockedInnerFunction() {
        // Do stuff
    }

    std::mutex m_mutex;
};

Note that this is not what std::recursive_mutex is for.

std::recursive_mutex is a hack for situations when you don't control the entire call chain, like invoking user-provided callbacks which might in turn call back into your code, preemptible ISRs, etc.

Applying std::recursive_mutex here would be inefficient and a code smell.

Upvotes: 3

Related Questions