Opened 9 years ago

Closed 9 years ago

#9708 closed Bugs (fixed)

boost::condition_variable::timed_wait unexpectedly wakes up while should wait infinite

Reported by: nikolay@… Owned by: viboes
Milestone: Boost 1.56.0 Component: thread
Version: Boost 1.52.0 Severity: Regression
Keywords: Cc:

Description

After upgdate from boost 1.44 to boost 1.52 following issue appears: boost::condition_variable::timed_wait(..., boost::posix_time::time_duration(boost::posix_time::pos_infin)) always immediately return false. In boost 1.44 it waits infinite for a condition notified.

Simple test:

int _tmain(int argc, _TCHAR* argv[])
{
        // test std
        std::condition_variable scv;
        std::mutex sm;

        bool flag = false;
        std::thread t([&]()
        {
                std::unique_lock<std::mutex> l(sm);
                if (std::cv_status::timeout == scv.wait_for(l, std::chrono::duration<int, std::ratio<1,1>>::max()))
                {
                        // wait_for return timeout. it means that time period has elapsed.
                        std::terminate();
                }
                if (!flag)
                        std::terminate(); // we should sleep yet
        });
        
        std::this_thread::sleep_for(std::chrono::seconds(2));
        {
                std::unique_lock<std::mutex> l(sm);
                flag = true;
        }
        scv.notify_one();
        t.join();
        

        // test boost
        boost::condition_variable bcv;
        boost::mutex bm;

        flag = false;
        std::thread bt([&]()
        {
                boost::unique_lock<boost::mutex> l(bm);
                if (!bcv.timed_wait(l, boost::posix_time::time_duration(boost::posix_time::pos_infin)))
                {
                        // timed_wait return false. it means that time period has elapsed.
                        std::terminate(); // positive infinite should never be elapsed
                }
                if (!flag)
                        std::terminate(); // we should sleep yet
        });

        std::this_thread::sleep_for(std::chrono::seconds(2));
        {
                boost::unique_lock<boost::mutex> l(bm);
                flag = true;
        }
        bcv.notify_one();
        bt.join();

        return 0;
}

compiled: MSVS2012 UP4, Win7 Pro.

Change History (12)

comment:1 by viboes, 9 years ago

Owner: changed from Anthony Williams to viboes
Status: newassigned

have you tried with

                if (boost::cv_status::timeout == scv.wait_for(l, boost::chrono::duration<int, std::ratio<1,1>>::max()))
                {
                        // wait_for return timeout. it means that time period has elapsed.
                        std::terminate();
                }

?

comment:2 by nikolay@…, 9 years ago

bcv.wait_for(l, boost::chrono::duration<int, boost::ratio<1,1>>::max()) works good.

But the issue with timed_wait.

I have some code like:

void foo(boost::posix_time::time_duration& td)
{
...
cv.timed_wait(..., td);
...
}

void f1()
{
...
foo(boost::posix_time::time_duration(boost::posix_time::pos_infin));
...
}

void f2()
{
...
foo(boost::posix_time::minutes(2));
...
}

void f3()
{
...
foo(boost::posix_time::milliseconds(500));
...
}

comment:3 by viboes, 9 years ago

I suspect that boost::posix_time::pos_infin can be seen as a negative duration. Could you confirm?

comment:4 by anonymous, 9 years ago

boost::posix_time::pos_infin - positive infinity
boost::posix_time::neg_infin - negative infinity

time_duration td(pos_infin);
td.is_pos_infinity(); --> true
td.is_negative();
--> false

As I see possible this issue may be caused by some changes inside Boost.DateTime library. I found new note in the Boost.DateTime documentation:

When a time_duration is a special value, either by construction or other means, the following accessor functions will give unpredictable results: hours(), minutes(), seconds(), ticks(), fractional_seconds(), total_nanoseconds(), total_microseconds(), total_milliseconds(), total_seconds()

So it seems that Boost.Thread incorrectry call total_milliseconds() inside timed_wait on duration which is special value. Possible code in timed_wait should be like:

        template<typename duration_type>
        bool timed_wait(unique_lock<mutex>& m,duration_type const& wait_duration)
        {
            if (wait_duration.is_pos_infinity())
            {
                wait(m); // or do_wait(m,detail::timeout::sentinel());
                return true;
            }
            if (wait_duration.is_special_value())
            {
                        //throw some error
            }
            return do_wait(m,wait_duration.total_milliseconds());
        }

comment:5 by viboes, 9 years ago

timed_value is deprecated. Even if the patch is simple, it is not complete. If you provide a complete patch with tests and the needed documentation I would apply it.

Last edited 9 years ago by viboes (previous) (diff)

comment:6 by viboes, 9 years ago

Type: BugsFeature Requests

comment:7 by viboes, 9 years ago

Summary: regression: boost::condition_variable::timed_wait unexpectedly wakes up while should wait infiniteAlign to new Boost.DtaeTime behavior (was regression: boost::condition_variable::timed_wait unexpectedly wakes up while should wait infinite)

comment:8 by viboes, 9 years ago

Summary: Align to new Boost.DtaeTime behavior (was regression: boost::condition_variable::timed_wait unexpectedly wakes up while should wait infinite)was regression: boost::condition_variable::timed_wait unexpectedly wakes up while should wait infinite
Type: Feature RequestsBugs

comment:9 by viboes, 9 years ago

Summary: was regression: boost::condition_variable::timed_wait unexpectedly wakes up while should wait infiniteboost::condition_variable::timed_wait unexpectedly wakes up while should wait infinite

comment:10 by nikolay@…, 9 years ago

What is "timed_value"? If you mean "timed_wait" then I have failed to find any notes in the Boost.Thread documentation that this function is deprecated.

Anyway currently situation looks like following:

  • there was bug in Boost.Thread in incorrect usage of Boost.DateTime
  • in previous releases it was lucky that code in Boost.Thread works
  • in latest releases this issue (incorrect usage of Boost.DateTime) is not hidden and Boost.Thread code does not work as expected
  • Boost.Thread users have an regression after update to the latest boost releases
  • I have suggested some changes to fix the problem in the Boost.Thread. I know that it is not final code, but I hope it is a good start point to Boost.Thread authors/sustainers to make an correct and complete changes
  • you can fix this issue by any other changes (even with breaking changes), for example, you can always throw something like std::invalid_arg in case if wait_duration.is_special_value(). It is your responsibility to chose correct fix from the view of the Boost.Thread library

P.S. I am not familiar with git so now I have lose an ability to generate any formal patches to Boost...

comment:12 by viboes, 9 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.