Exception thrown when locking boost::unique_lock which is already locked in other thread

Question

There is global object boost::unique_lock. One thread locks it. When other thread tries to lock the exception "already owns the mutex" thrown. Why this is happening? I expect the thread to be blocked until other thread will call unlock.

boost::mutex mutex;
boost::unique_lock<boost::mutex> lock(mutex);

static void* scheduller(void* arg)
{
    boost::this_thread::sleep(boost::posix_time::seconds(5));
    lock.lock();    
    return 0;
}

static void* usb(void* arg)
{
    lock.lock();
    boost::this_thread::sleep(boost::posix_time::seconds(20));
    return 0;
}

int BEvent_test()
{
    lock.unlock();
    int a = 0;
    boost::thread thread1(usb,&a);
    boost::thread thread2(scheduller,&a);
    boost::this_thread::sleep(boost::posix_time::seconds(100));
    return 0;

}

In BEvent_test function I'm doing unlock because unique lock is locked in constructor. Delays make sure that scheduller thread starts after usb thread. When it tries to do lock() exception is thrown. Why?

Answer 1

Your use of unique_lock is wrong, in particular those objects are not supposed to be shared between different threads, because they are not thread-safe. Yes, this sounds weird at first, but it does make sense, because being thread-safe and providing thread-safety are two distinct things.

The idea of unique_lock is to help you manage locks on mutexes. For that, you create those objects locally only. That's also the reason that it was initially locked, which you seem to "work around" by unlocking it. However, you should rather try to use their scope/lifetime to manage the lock.

Now, you say that you need a unique_lock in order to use an event. Yes and no. You need one, but not exactly the same one for every thread using the event. Instead, you can use different, local lock instances from different threads. BTW: The reason that the event needs the lock is that while waiting for the event, the lock must be released. After receiving the event (or timeout), it also has to reacquire the lock. In other words, the lock serves as a proxy to the mutex, offering indirection and some added guarantees.

The reason that you receive an exception is that you try to lock the unique_lock twice. I don't think that it matters that this is from the same thread, I wouldn't even say that it really is from the same thread. However, when you lock a mutex (via the unique_lock), you say that you are now entering a critical section. This implies that before, you weren't in that critical section. If the unique_lock now finds the lock already held, it means that something in your code went wrong, because the code seems to be confused whether it is inside a critical section or not. For that reason, you get an exception as motivation to fix your code.

Answer 2

I am not familiar with the boost synchronization. From the documentation here it seems to me that the idea is the code where the unique_lock object is created to have ownership for a mutex for a given duration (time, the lifetime of the unique_lock, etc). I think you should then try to lock on the mutex, not the unique_lock in the other threads.

The way I see it, unique_lock is useful to lock a mutex in a method and know that whatever happens, on the exit of the method (unique_lock destruction), the mutex will be freed. You don't have to make sure that you catch exceptions, etc in order the mutex to be freed.

Answer 3

The message does not mean you have the mutex locked on the other thread. It means you already have the mutex locked on the same thread. And since your mutex is not a recursive lockable, this is an error.

---

You're doing manual locking/unlocking of the mutex.

It's hard to see from the code fragment shown, but it's easy to

• do it wrong (forget to unlock in time)
• it's near impossible to have it exception safe (the code shown certainly isn't).

Instead you should use scoped_lock (or lock_guard/unique_lock) to balance the lock/unlock automatically and in an exception safe manner.

---

UPDATE

On re-reading your code, you were just using unique_lock entirely wrong:

Live On Coliru

#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/thread.hpp>
#include <iostream>

#define TRACE(msg) do { std::cout << msg << " " << __FUNCTION__ << " at " << relative_ms() << "ms\n"; } while(false)

int relative_ms() {
    using boost::posix_time::second_clock;
    static auto t0 = second_clock::local_time();

    return (second_clock::local_time() - t0).total_milliseconds();
}

boost::mutex mutex;

static void scheduler()
{
    boost::unique_lock<boost::mutex> lock(mutex);
    TRACE("Enter");

    boost::this_thread::sleep_for(boost::chrono::seconds(2));

    TRACE("Leave");
}

void usb()
{
    boost::unique_lock<boost::mutex> lock(mutex);
    TRACE("Enter");

    boost::this_thread::sleep_for(boost::chrono::seconds(5));

    TRACE("Leave");
}

int main()
{
    TRACE("Enter");
    boost::thread thread1(usb);
    boost::thread thread2(scheduler);
    boost::this_thread::sleep_for(boost::chrono::seconds(10));
    TRACE("Leave");
}

Prints e.g.

Enter main at 0ms
Enter usb at 0ms
Leave usb at 5000ms
Enter scheduler at 5000ms
Leave scheduler at 7000ms
Leave main at 10000ms

or

Enter main at 0ms
Enter scheduler at 0ms
Leave scheduler at 2000ms
Enter usb at 2000ms
Leave usb at 7000ms
Leave main at 10000ms

depending only on which thread grabs the mutex first.