7
\$\begingroup\$

I would like to create a wrapper for a Boost conditional variable:

class Cond_wrap
{
private:
    boost::condition_variable cond;
    boost::mutex              mutex;
    bool work_to_do;
public:
    Cond_wrap()
    {
        work_to_do = false; 
    }
    void notify_all()
    {
        boost::mutex::scoped_lock lock(mutex);
        work_to_do = true;
        cond.notify_all();
    }
    void wait()
    {
        boost::mutex::scoped_lock lock(mutex);
        while(!work_to_do)
        {
            cond.wait(lock);
            work_to_do = false;
        }
    }
    bool timed_wait(unsigned int timeout)
    {
        boost::mutex::scoped_lock lock(mutex);
        if(!work_to_do)
        {
            if(cond.timed_wait(lock, boost::chrono::milliseonds(timeout)))
            {
                work_to_do = false;
                return true;
            }
            else
            {
                return false;
            }
        }
        else
        {
            return false;
        }
};

Cond_wrap condition_wrap;

void worker_func()
{
    {
        condition_wrap.notify_all();
    }
    std::cout << "After notify" << std::endl;
}

int main()
{
    boost::thread work(worker_func);
    work.detach();

    {
        boost::this_thread::sleep_for(boost::chrono::milliseonds(500));
        condition_wrap.wait();
        //there is work to do
    }
    return 0;
}

What is the main goal of that wrapper? I would like to avoid a situation in which a condition will be notified before I call waiter. Regarding this, I want to provide a help variable bool which should remember if a condition was previously set.

Small example of what I mean:

boost::condition_variable cond;
boost::mutex              mutex;

void worker_func()
{
    cond.notify_all();
    std::cout << "After notify" << std::endl;
}

void main()
{
    boost::mutex::soped_lock lock(mutex);
    boost::thread work(worker_func);
    boost::this_thread::sleep_for(boost::chrono::milliseonds(500));
    cond.wait(lock); // here is deadlock
}

The wrapper is also provides an easier way of using a Boost conditional variable.

\$\endgroup\$
3
\$\begingroup\$

The short answer here is: don't do this. It does not make boost::condition_variable easier to use. You're actually just limiting what you can do with it. You're not even exposing the entire interface...

But if you insist on doing such a thing, your wrapper is close. You're not handling spurious wakeup in timed_wait() - that can return true even without being signaled. You should just take advantage of the fact that the various wait() overloads also take a predicate:

void wait()
{
    boost::mutex::unique_lock lock(mutex);
    cond.wait(lock, [this]{ return work_to_do; });
    work_to_do = false;
}

bool timed_wait(unsigned int timeout)
{
    boost::mutex::unique_lock lock(mutex);
    if (cond.timed_wait(lock, boost::chrono::milliseconds(timeout),
                        [this]{ return work_to_do; })
    {
        work_to_do = false;
        return true;
    }
    else
    {
        return false;
    }
}

Really adding predicates is the way to handle calling wait() after the notify was called. The wrapper is not a good solution to this in my opinion.

\$\endgroup\$
  • \$\begingroup\$ Thank you for help, rearding to your post in your opinion better way is to use condition variable in pair with boolean instead of create wrapper ? Whole api an be overed this is only 'main' part. \$\endgroup\$ – user5093757 Jul 16 '15 at 10:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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