25

I am using a conditional variable to stop a thread until another thread has completed processing it's task queue (long story). So, on one thread I lock and wait:

boost::mutex::scoped_lock lock(m_mutex);
m_condition.wait(lock);

Once the other thread has completed it's tasks, it signals the waiting thread as follows:

boost::mutex::scoped_lock lock(m_parent.m_mutex);
m_parent.m_condition.notify_one();

The problem I am seeing is that the waiting thread does not stop waiting unless I set a breakpoint on the instructions following it (I am using xcode, fyi). Yes, this seems strange. Does anyone know why this might be happening? Am I mis-using the condition variable?

1
  • The code is a bit confusing ... is the first m_mutex the same instance as the second m_parent.m_mutex?
    – SkyWalker
    Nov 30, 2016 at 9:24

2 Answers 2

44

Yes, you are misusing the condition variable. "Condition variables" are really just the signaling mechanism. You also need to be testing a condition. In your case what might be happening is that the thread that is calling notify_one() actually completes before the thread that calls wait() even starts. (Or at least, the notify_one() call is happening before the wait() call.) This is called a "missed wakeup."

The solution is to actually have a variable which contains the condition you care about:

bool worker_is_done=false;

boost::mutex::scoped_lock lock(m_mutex);
while (!worker_is_done) m_condition.wait(lock);

and

boost::mutex::scoped_lock lock(m_mutex);
worker_is_done = true;
m_condition.notify_one();

If worker_is_done==true before the other thread even starts waiting then you'll just fall right through the while loop without ever calling wait().

This pattern is so common that I'd almost go so far as to say that if you don't have a while loop wrapping your condition_variable.wait() then you always have a bug. In fact, when C++11 adopted something similar to the boost::condtion_variable they added a new kind of wait() that takes a predicate lambda expression (essentially it does the while loop for you):

std::condition_variable cv;
std::mutex m;
bool worker_is_done=false;


std::unique_lock<std::mutex> lk(m);
cv.wait(lk, []{return worker_is_done;});
5
  • What is boost::mutex::scoped_lock lock(m_mutex); in the waiting thread needed for? notify_one() does not take args.
    – droid192
    Jan 14, 2016 at 16:38
  • It is there to provide an atomic write to worker_is_done. The alternative is to declare the type as atomic<bool> instead of just bool. Feb 6, 2016 at 18:40
  • @DavidStone What if we are running the code on a processor in which 1 byte assignment is atomic eg. x86? We won't need the lock in that case, is it? Sep 22, 2016 at 6:50
  • 1
    @user3286661: No, your processor is irrelevant. If you have two possibly overlapping accesses to a variable, at least one of which is a write, then your program's behavior is undefined unless the variable is a mutex or an atomic, basically. Your hardware may be 'safe', but your compiler can do whatever optimizations it wants under the assumption that you are not doing anything undefined. For instance, maybe it will combine multiple variables into one because you cannot tell the difference (unless you have undefined behavior). Sep 23, 2016 at 1:00
  • @DavidStone Could you please write an answer to this? I'd be very thankful. Sep 23, 2016 at 9:57
2

I had implemented an example that illustrates how to use boost condition, based in the discussion.

#include <iostream>

#include <boost/asio.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/thread.hpp>

boost::mutex io_mutex;
bool worker_is_done = false;
boost::condition_variable condition;

void workFunction()
{
    std::cout << "Waiting a little..." << std::endl;
    boost::this_thread::sleep(boost::posix_time::seconds(1));
    worker_is_done = true;
    std::cout << "Notifying condition..." << std::endl;
    condition.notify_one();
    std::cout << "Waiting a little more..." << std::endl;
    boost::this_thread::sleep(boost::posix_time::seconds(1));
}

int main()
{
    boost::mutex::scoped_lock lock(io_mutex);
    boost::thread workThread(&workFunction);

    while (!worker_is_done) condition.wait(lock);
    std::cout << "Condition notified." << std::endl;
    workThread.join();
    std::cout << "Thread finished." << std::endl;

    return 0;
}

Boost condition variable example

1
  • 5
    Shouldn't you take the lock in workFunction before setting worker_is_done to true?
    – Hugh White
    Jun 4, 2015 at 13:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.