Avoid deadlock in a single thread

Question

I have the following problem: I have a class that needs to be protected from simultaneous access from different threads. The class has two methods: lock() and unlock() using (g_mutex_lock / g_mutex_unlock with a per-object GMutex). Now a locking method looks like this:

void Object::method()
{
    lock();
    // do stuff modifying the object
    unlock();
}

Now lets assume that I have two mwthods of this type, method1() and method2() which I call one after another:

object.method1();
// but what if some other thread modifies object in between
object.method2();

I tried locking the object before this block und unlocking it again, but in this case there is a deadlock even with a single thread because the GMutex doesn't know that it has already been locked by the same thread. A solution would be to modify the method to accept an additional bool to determine whether the object is already locked. But is there a more elegant concept? Or is this a shortcoming regarding the design concept in total?

Answer 1

The recursive mutex solution mentioned in other responses and comments will work just fine, but in my experience it leads to code that is harder to maintain, because once you switch to a recursive mutex it is all too easy to abuse it and lock all over the place.

Instead, I prefer to reorganize the code so that locking once is sufficient. In your example I would define the class as follows:

class Object {
public:
    void method1() {
        GMutexLock scopeLock(&lock);
        method1_locked();
    }
    void method2() {
        GMutexLock scopeLock(&lock);
        method2_locked();
    }
    void method1_and_2() {
        GMutexLock scopeLock(&lock);
        method1_locked();
        method2_locked();
    }

private:
    void method1_locked();
    void method2_locked();

    GMutex lock;
};

The "locked" versions of your methods are private, so they are only accessible from inside the class. The class takes responsibility in never calling these without the lock taken.

From the outside you have three choices of methods to call, depending on which of the methods you want to run.

Note that another improvement I've made is to not use explicit locking primitives but instead use the scope indirectly to lock and unlock. This is what GMutexLock does. An example implementation for this class is below:

class GMutexLock {
private:
    GMutex* m;

    GMutexLock(const GMutexLock &mlock); // not allowed
    GMutexLock &operator=(const GMutexLock &); // not allowed

public:
    GMutexLock(GMutex* mutex) {
        m = mutex;
        g_mutex_lock(m);
    }

    ~GMutexLock() {
        g_mutex_unlock(m);
    }
};

Answer 2

Look up "recursive mutex" or "reentrant mutex" (versus the non-recursive mutex you're using now). These enable what you want. Some folks are not fans of recursive mutexes and feel they enable messy design.

Note that a recursive mutex cannot be locked on one thread and unlocked on another.

Answer 3

I personally would never use recursive mutexes (especially as such).

I would do some private functions that doesn't lock mutexes and lock around them in public functions' implementations.