Deadlock with boost::condition_variable

Question

I am a bit stuck with the problem, so it is my cry for help.

I have a manager that pushes some events to a queue, which is proceeded in another thread. I don't want this thread to be 'busy waiting' for events in the queue, because it may be empty all the time (as well as it may always be full). Also I need m_bShutdownFlag to stop the thread when needed. So I wanted to try a condition_variable for this case: if something was pushed to a queue, then the thread starts its work.

Simplified code:

class SomeManager {
public:
    SomeManager::SomeManager()
        : m_bShutdownFlag(false) {}

    void SomeManager::Initialize() {
        boost::recursive_mutex::scoped_lock lock(m_mtxThread);
        boost::thread thread(&SomeManager::ThreadProc, this);
        m_thread.swap(thread);
    }

    void SomeManager::Shutdown() {
        boost::recursive_mutex::scoped_lock lock(m_mtxThread);
        if (m_thread.get_id() != boost::thread::id()) {
            boost::lock_guard<boost::mutex> lockEvents(m_mtxEvents);
            m_bShutdownFlag = true;
            m_condEvents.notify_one();
            m_queue.clear();
        }
    }

    void SomeManager::QueueEvent(const SomeEvent& event) {
        boost::lock_guard<boost::mutex> lockEvents(m_mtxEvents);
        m_queue.push_back(event);
        m_condEvents.notify_one();
    }

private:
    void SomeManager::ThreadProc(SomeManager* pMgr) {
        while (true) {
            boost::unique_lock<boost::mutex> lockEvents(pMgr->m_mtxEvents);
            while (!(pMgr->m_bShutdownFlag || pMgr->m_queue.empty()))
                pMgr->m_condEvents.wait(lockEvents);

            if (pMgr->m_bShutdownFlag)
                break;
            else
                /* Thread-safe processing of all the events in m_queue */
        }
    }

    boost::thread m_thread;
    boost::recursive_mutex m_mtxThread;
    bool m_bShutdownFlag;

    boost::mutex m_mtxEvents;
    boost::condition_variable m_condEvents;
    SomeThreadSafeQueue m_queue;
}

But when I test it with two (or more) almost simultaneous calls to QueueEvent, it gets locked at the line boost::lock_guard<boost::mutex> lockEvents(m_mtxEvents); forever.

Seems like the first call doesn't ever release lockEvents, so all the rest just keep waiting for its freeing.

Please, help me to find out what am I doing wrong and how to fix this.

Answer 1

There's a few things to point out on your code:

1. You may wish to join your thread after calling shutdown, to ensure that your main thread doesn't finish before your other thread.
2. m_queue.clear(); on shutdown is done outside of your m_mtxEvents mutex lock, meaning it's not as thread safe as you think it is.
3. your 'thread safe processing' of the queue should be just taking an item off and then releasing the lock while you go off to process the event. You've not shown that explicitly, but failure to do so will result in the lock preventing items from being added.

The good news about a thread blocking like this, is that you can trivially break and inspect what the other threads are doing, and locate the one that is holding the lock. It might be that as per my comment #3 you're just taking a long time to process an event. On the other hand it may be that you've got a dead lock. In any case, what you need is to use your debugger to establish exactly what you've done wrong, since your sample doesn't have enough in it to demonstrate your problem.