How to specify what mutex locks are needed before entering a function

Question

Sometimes a function is written that requires one or more mutexes to be locked before entering that function. If this requirement is not specified, the function can be called without taking the relevant locks before entering, which could have catastrophic consequences.

Now, it is possible to specify something like this in the documentation of a function, but I really don't like that.

I was thinking to specify it in the preconditions for a function (assert's when entering the function), but what should the condition be?

Even if the std::mutex in C++11 did have a has_lock() function, there still would not be any guarantees that I'm the one who has the lock.

Is there a reason you can't use reentrant mutexes and then do the locking and unlocking in the function itself? — cmh, Jan 25 '13 at 14:36
I dislike reentrant mutexes, http://stackoverflow.com/a/293646/708004 explains it better than I can. — Ives, Jan 25 '13 at 14:44
"Non-reentrant mutexes lead to better code." Except for when they don't. — cmh, Jan 25 '13 at 14:48
Further, is there are reason why the mutexes have to be the same mutexes that live outside the scope of the function? — cmh, Jan 25 '13 at 14:49
Have your function require that you pass a lock object (or objects) in as an argument(s)? — Yakk - Adam Nevraumont, Jan 25 '13 at 14:51
Well, they lock the same resource. Imagine writing a function that removes some elements from a list. Before starting, a mutex needs to be taken to ensure the list is not modified by another thread halfway through. Now, the code to determine what elements need to be removed is quite long, and as it iterates over the list it obviously also requires the mutex to be locked. I'd very much like to refactor the code that determines what elements to remove to another function. Assume I also don't want the list to change between determining what elements to remove and removing them. — Ives, Jan 25 '13 at 14:53
What good does requiring the caller to send the lock objects by parameter do? The function is already able to access the locks if it would like to do so. — Ives, Jan 25 '13 at 14:54
let me understand: what you would like is to check if the *current thread* is holding some mutex(es), and return false (or assert) if that's not the case? — Andy Prowl, Jan 25 '13 at 14:57
I understand, I'm asking these questions so that your question is fully specified for anyone answering/ — cmh, Jan 25 '13 at 14:59
I guess one possible solution would be to wrap std::mutex and add an std::thread::id datamember that is set when the lock is taken to std::this_thread::id() and set to nothing before the lock is released. It would then be possible to check if this id is equal to the current thread id. — Ives, Jan 25 '13 at 15:05
@AndyProwl it looks like you had the same idea I had. Yes, that's an option. — Ives, Jan 25 '13 at 15:05
In your list example, why can't the helped function take its own mutex? Any situation this would lead to deadlock will lead to deadlock in the "mutex checking" solution and the intent is clearer. — cmh, Jan 25 '13 at 15:11

Andy Prowl · Accepted Answer · 2013-01-25T15:38:36.980

If you really don't want to use recursive mutexes and your goal is to figure out whether the current thread is holding a mutex without trying to acquire it, defining a mutex wrapper is probably the simples solution. Here is a wild shot:

#include <thread>
#include <mutex>
#include <iostream>

using namespace std;

template<typename M>
struct mutex_wrapper
{
    void lock() 
    { 
        m.lock(); 
        lock_guard<mutex> l(idGuardMutex); 
        threadId = this_thread::get_id();
    }

    void unlock() 
    { 
        lock_guard<mutex> l(idGuardMutex); 
        threadId = thread::id(); 
        m.unlock(); 
    }

    bool is_held_by_current_thread() const 
    { 
        lock_guard<mutex> l(idGuardMutex); 
        return (threadId == this_thread::get_id()); 
    }

private:

    mutable mutex idGuardMutex;
    thread::id threadId;
    M m;
};

And here is a simple example of how to use it:

int main()
{
    cout << boolalpha;
    mutex_wrapper<mutex> m;
    m.lock();
    cout << m.is_held_by_current_thread() << endl;
    m.unlock();
    cout << m.is_held_by_current_thread() << endl;
}

score 2 · Answer 2 · answered Jan 25 '13 at 15:44

I think the answer to your dilemma is simply don't use an external mutex. If a class manages a resource that needs to be synchronized, then it should use an internal mutex and handle all the synchronization itself. An external mutex is dangerous since it opens you up to the possibility of both deadlocks and unsynchronized access.

From the comments, it sounds like the problem you're struggling with is refactoring a synchronized collection. You want to move some code out of the class, but that code must be synchronized. Here's an example of how you can do it:

class MyCollection {
private:
    std::list<Foo> items;
    std::mutex lock;

public:
    template <class F> void ForEach( F function )
    {
        std::lock_guard<decltype(this->lock) guard( this->lock );

        for( auto item : items )
            function( *item );
    }
};

This technique still has the potential for deadlock. Since the function parameter is an arbitrary function, it might access the collection and thus acquire the mutex. On the other hand, this behaviour might be desired if "ForEach" should be read-only.

Hmm, no, the list thing was just an example. The real scenario is much more complicated, too complicated to explain in a reasonable length of text in fact. I don't have an external mutex. — Ives, Jan 25 '13 at 16:06
Well, if we're talking about an internal mutex, then just change it to a `std::recursive_mutex` and take the lock inside your function. This is exactly what a recursive mutex is for. — Peter Ruderman, Jan 25 '13 at 16:13

How to specify what mutex locks are needed before entering a function

2 Answers2