Race condition with self

Question

I have a class with member functions that require a boost::unique_lock be acquired on the respective mutex before performing their operation. However, when the member function is called when there is an existing boost::unique_lock it deadlocks itself.

#include <iostream>
#include "boost/thread.hpp"
#include "boost/foreach.hpp"
#include <list>
class example {
    private:
    std::list< int > int_array;
    std::list< int >::iterator array_iterator;
    mutable boost::shared_mutex array_mutex;
    public:
    /*
     * Move the iterator forward.
     */
    inline example & next( ) {
        boost::unique_lock< boost::shared_mutex > lock( array_mutex );

        array_iterator++;
        if( array_iterator == int_array.end( ) ) {
            array_iterator = int_array.begin( );
        }

        return *this;
    }

    /*
     * Get int_array_mutex.
     */
    inline boost::shared_mutex & mutex( ) const {
        return array_mutex;
    }

    /*
     * Get int_array.
     */
    inline std::list< int > & array() {
        return int_array;
    }
};

int main() {
    example example_instance;

    boost::unique_lock< boost::shared_mutex> lock(example_instance.mutex());

    //Manipulate int_array...
    example_instance.array().push_back(1);
    example_instance.array().push_back(2);
    example_instance.array().push_back(3);
    example_instance.array().push_back(4);
    BOOST_FOREACH(int & x, example_instance.array()) {
        x++;
    }

    //Causes deadlock
    example_instance.next();

    std::cout << "This shall not be emitted." << std::endl;

    return 0;
}

How can I fix this?

Answer 1

I haven't used boost threading, but if I read your code correctly you lock the mutex in your main function then you call the examlpe_instance's next() method which also tries to obtain a lock on the array mutex... naturally that would cause a deadlock, since it doesn't seem like the lock is re-entrant.

Why are you acquiring the lock in the main function?

Update:

You can't re-acquire a lock twice within the same scope, unless you have a re-entrant lock (which you don't), so try changing the scope:

int main() {
    example example_instance;

    {// new scope
        boost::unique_lock< boost::shared_mutex> lock(example_instance.mutex());

        //Manipulate int_array...
        example_instance.array().push_back(1);
        example_instance.array().push_back(2);
        example_instance.array().push_back(3);
        example_instance.array().push_back(4);
        BOOST_FOREACH(int & x, example_instance.array()) {
            x++;
        }
    }// end scope

    // should not cause deadlock now
    example_instance.next();

    std::cout << "This shall not be emitted." << std::endl;

    return 0;
}

If boost works like I imagine it should, then boost should release the lock once it gets out of scope. Try the above modification and see if you still get a deadlock (if you still do, then you have to explicitly release the lock somehow, but I don't know how that's done in boost).

Answer 2

What you want is a recurisive_mutex which is more commonly known as a re-entrant mutex. This will allow you to lock the same mutex multiple times within the same thread without causing deadlocks. So to fix your example, just replace all instances of shared_mutex by recursive_mutex and it will work.

The problem is that you can now no longer use shared_lock and upgrade_to_unique_lock since they both require that you have a shared_mutex. As it turns out, according to these two links it looks like shared_mutex are a bad idea anyway. If you really need shared_mutex, you'll have to write your own shared_recursive_mutex because boost doesn't provide one.