Opened 7 years ago

Closed 7 years ago

#11562 closed Bugs (fixed)

(condition_variable_any::wait_until + recursive_mutex + steady_clock) timer expires after computer time is set forward on Ubuntu 64-bit

Reported by: boriss@… Owned by: viboes
Milestone: Boost 1.61.0 Component: thread
Version: Boost 1.59.0 Severity: Showstopper
Keywords: steady_clock, Linux 64-bit Cc:

Description

We are using boost:chrono for the Timer implementation. Below is our implementation:

boost::chrono::time_point<boost::chrono::steady_clock> untilTime(boost::chrono::steady_clock::now() + boost::chrono::milliseconds(dueTime));

boost::condition_variable  m_condition;
(void)m_condition.wait_until(guard, untilTime);

After computer time on Ubuntu Linux 64-bit (tested with 12.04 and 15.04) is set forward for the time bigger then timer interval, timer expires immediately.

We have reproduced the problem with different boost versions 1.51, 1.54,1.55,1.57,1.59

On Ubuntu Linux 32-bit (versions 12.04, 13.04) and Windows 7 (32-bit and 64-bit) problem cannot be reproduced.

Attachments (2)

ThreadsTest.cpp (1.1 KB ) - added by boriss@… 7 years ago.
gdb.bt (11.6 KB ) - added by boriss@… 7 years ago.

Download all attachments as: .zip

Change History (64)

comment:1 by anonymous, 7 years ago

Severity: ProblemShowstopper

comment:2 by viboes, 7 years ago

Component: chronothread
Status: newassigned

I've a patch that uses pthread_condattr_setclock. I have no platform providing it (I'm using MacOS). The user will need to define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC until I find a way to detect it.

Added to develop branch https://github.com/boostorg/thread/commit/9f883f6ad7377b88b79fa70cc5de68cbfb0213e4

Can you test it?

comment:3 by viboes, 7 years ago

Status: assignednew

comment:4 by boriss@…, 7 years ago

We have tested and wait_until is now working OK. We have found out that sleep function is NOT working anymore after this fix:

boost::this_thread::sleep(boost::posix_time::milliseconds(timeout));

It seems that it stays in the endless loop.

Please add this changes also to the boost::condition not only to the boost::condition_variable

comment:5 by boriss@…, 7 years ago

Function sleep is deprecated from 1.56 version. But sleep_for also hangs after your fix:

boost::this_thread::sleep_for(boost::chrono::milliseconds(timeout));

comment:6 by boriss@…, 7 years ago

It hangs also if system time is not changed

comment:7 by viboes, 7 years ago

I've added a patch that uses pthread_condattr_setclock. I have no platform providing it (I'm using MacOS). The user will need to define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC until I find a way to detect it.

Add to develop branch https://github.com/boostorg/thread/commit/9f883f6ad7377b88b79fa70cc5de68cbfb0213e4

Can you test it?

comment:8 by viboes, 7 years ago

Status: newassigned

comment:9 by boriss@…, 7 years ago

Like i described above, we have compiled boost 1.59.0. We did the testing and found out that wait_until is NOT working but sleep/sleep_for is working OK. Then we have add your fix (we have replaced condition_variable_fwd.hpp) and compile boost again. After that wait_until is working OK , but sleep/sleep_for is NOT working anymore. I seems that it stays in endless loop. We have tested also with boost version 1.55.0 and it is the same behavior.

comment:10 by viboes, 7 years ago

Have you added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC?

in reply to:  10 comment:11 by viboes, 7 years ago

Replying to viboes:

Have you added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC?

Curiously the patch works for another tester #11377.

comment:12 by boriss@…, 7 years ago

We have added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. But after inserting the patch sleep/sleep_for is NOT working anymore. I seems that it stays in endless loop.

in reply to:  12 ; comment:13 by anonymous, 7 years ago

Replying to boriss@…:

We have added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. But after inserting the patch sleep/sleep_for is NOT working anymore. I seems that it stays in endless loop.

Will this changes also be added to the boost::condition?

in reply to:  13 ; comment:14 by viboes, 7 years ago

Replying to anonymous:

Replying to boriss@…:

We have added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. But after inserting the patch sleep/sleep_for is NOT working anymore. I seems that it stays in endless loop.

Will this changes also be added to the boost::condition?

Do you mean boost::condition_variable_any? if yes, the answer is yes if it works.

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

in reply to:  14 comment:15 by anonymous, 7 years ago

Replying to viboes:

Replying to anonymous:

Replying to boriss@…:

We have added the define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. But after inserting the patch sleep/sleep_for is NOT working anymore. I seems that it stays in endless loop.

Will this changes also be added to the boost::condition?

Do you mean boost::condition_variable_any? if yes, the answer is yes if it works.

Yes i mean boost::condition_variable_any. But please check sleep and sleep_for functions which are not working after your patch. Tester #11377 was testing only sleep_until function.

comment:16 by viboes, 7 years ago

There are a lot of issues with this time issue. If

boost::chrono::time_point<boost::chrono::steady_clock> untilTime(boost::chrono::steady_clock::now() + boost::chrono::milliseconds(dueTime));

boost::condition_variable  m_condition;
(void)m_condition.wait_until(guard, untilTime);

works now I will propose to close this issue. I will not fix the sleep issue as deprecated. Please create an issue for sleep_for if it doesn't exist yet.

I believe that there is already one #7665.

comment:17 by viboes, 7 years ago

Please could you try to replace in thread/v2/thread.hpp line #94

#ifdef BOOST_THREAD_SLEEP_FOR_IS_STEADY

by

#if defined BOOST_THREAD_SLEEP_FOR_IS_STEADY && ! defined BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC

in reply to:  17 ; comment:18 by boriss@…, 7 years ago

Replying to viboes:

Please could you try to replace in thread/v2/thread.hpp line #94

#ifdef BOOST_THREAD_SLEEP_FOR_IS_STEADY

by

#if defined BOOST_THREAD_SLEEP_FOR_IS_STEADY && ! defined BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC

After this change also sleep_for is working OK. Please add changes from boost::condition_variable also to boost::condition_variable_any

Can we expect the patch for this or will be in the next boost release? It is already known when 1.60 version will be released?

in reply to:  18 comment:19 by viboes, 7 years ago

Replying to boriss@…:

Replying to viboes:

Please could you try to replace in thread/v2/thread.hpp line #94

#ifdef BOOST_THREAD_SLEEP_FOR_IS_STEADY

by

#if defined BOOST_THREAD_SLEEP_FOR_IS_STEADY && ! defined BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC

After this change also sleep_for is working OK. Please add changes from boost::condition_variable also to boost::condition_variable_any

Can we expect the patch for this or will be in the next boost release? It is already known when 1.60 version will be released?

Now that we know that it works for sleep_for also, I will start the adaptation of boost::condition_variable_any.

Thanks for your test and patience.

comment:20 by viboes, 7 years ago

Milestone: To Be DeterminedBoost 1.60.0

comment:22 by boriss@…, 7 years ago

We are using 1.59 release sources. After adding changes from boost::condition_variable_any (file condition_variable.hpp) we get an Segmentation fault error:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe5bad700 (LWP 3978)]
0x00007ffff75e859a in boost::chrono::detail::duration_cast<boost::chrono::duration<long, boost::ratio<1l, 1000000000l> >, boost::chrono::duration<long, boost::ratio<1l, 1000000000l> > >::operator() (this=0x0, fd=...)
    at /usr/local/include/boost/chrono/duration.hpp:313
313	        BOOST_CONSTEXPR ToDuration operator()(const FromDuration& fd) const

Using gdb debugger we got in backtrace all the time the following print out:

at /usr/local/include/boost/thread/pthread/condition_variable.hpp:311
#1460 0x00007ffff75e0731 in boost::condition_variable_any::wait_until<boost::unique_lock<boost::recursive_mutex>, boost::chrono::duration<long, boost::ratio<1l, 1000000000l> > > (this=0x91b6d0, lock=..., t=...)

After updating also latest boost::chrono from development branch we got the same error.

comment:23 by viboes, 7 years ago

Please, is it possible for you to try with the develop branch?

comment:24 by viboes, 7 years ago

Resolution: fixed
Status: closedreopened

comment:25 by boriss@…, 7 years ago

We have cloned develop branch with:

git clone --recursive https://github.com/boostorg/boost.git

and after that we got some errors during compiling of boost.

Can you give me instructions how to checkout/clone on Ubuntu Linux develop branch and how to compile?

comment:26 by viboes, 7 years ago

Maybe this can helps https://svn.boost.org/trac/boost/wiki/TryModBoost

Please, can you send me what error are you getting?

comment:27 by boriss@…, 7 years ago

We have managed to compile develop branch and we get the same segmentation error like described above. It seems that the cause is in condition_variable_any (file condition_variable.hpp).

comment:28 by viboes, 7 years ago

Please, could you tell me with which program are you having this crash? is a Boost.Thread test?

Could you show the part related to

boost::condition_variable_any::wait_until<boost::unique_lock<boost::recursive_mutex>, boost::chrono::duration<long, boost::ratio<1l, 1000000000l> > >

I see that there is a boost::recursive_mutex. Could you tell me if you can change it to a mutex?

comment:29 by boriss@…, 7 years ago

We are testing with our application that we are developing. We cannot change to a mutex since we need recursive_mutex.

comment:30 by boriss@…, 7 years ago

Our code is following:

boost::recursive_mutex m_CsQueuedItems;
boost::recursive_mutex::scoped_lock _lock1(m_CsQueuedItems);
boost::condition_variable_any m_EvQueuedItems;

boost::chrono::time_point<boost::chrono::steady_clock> untilTime(boost::chrono::steady_clock::now() + boost::chrono::milliseconds(500));

m_EvQueuedItems.wait_until(_lock1, untilTime);

We are calling wait_until from the new thread.

If we use mutex instead of recursive_mutex then we don't get segmentation fault. But like explained above we need recursive mutex. Hope this helps.

comment:31 by viboes, 7 years ago

I will do my best, trying to reproduce the issue on my PC. Have you reached to isolate the bug in a simple example?

comment:32 by viboes, 7 years ago

Milestone: Boost 1.60.0To Be Determined

comment:33 by viboes, 7 years ago

I don't reach to reproduce myself the issue. I need to know if the issue is with condition_variable_any or with recursive_mutex.

Please could you try to isolate the bug in a simple example?

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

comment:34 by viboes, 7 years ago

Summary: Timer (using steady_clock) expires after computer time is set forward on Ubuntu 64-bit(condition_variable_any::wait_until + recursive_mutex + steady_clock) timer expires after computer time is set forward on Ubuntu 64-bit

comment:35 by viboes, 7 years ago

Oh I missed that you have already tested with mutex and it works.

comment:36 by viboes, 7 years ago

Could you check if BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC is defined in your platform?

comment:38 by boriss@…, 7 years ago

We didn't changed our code. And before your latest changes in file condition_variable.hpp we didn't have this problem.

in reply to:  38 ; comment:39 by boriss@…, 7 years ago

Replying to boriss@…:

We didn't changed our code. And before your latest changes in file condition_variable.hpp we didn't have this problem.

We have defined BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. And it works if we use changes only from condition_variable

in reply to:  38 comment:40 by viboes, 7 years ago

Replying to boriss@…:

We didn't changed our code. And before your latest changes in file condition_variable.hpp we didn't have this problem.

This is not enough, If you code was not well formed. Can you check if you are in this case a linked?

in reply to:  39 comment:41 by viboes, 7 years ago

Replying to boriss@…:

Replying to boriss@…:

We didn't changed our code. And before your latest changes in file condition_variable.hpp we didn't have this problem.

We have defined BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC. And it works if we use changes only from condition_variable

Does it means that I can close the ticket?

comment:42 by viboes, 7 years ago

Resolution: fixed
Status: reopenedclosed

comment:43 by viboes, 7 years ago

Milestone: To Be DeterminedBoost 1.60.0

comment:44 by boriss@…, 7 years ago

Resolution: fixed
Status: closedreopened

Sorry, i missed your latest post. I believe that our code is well formed. We used/tested our code with different boost versions and we didn't have this problems. We get segmentation fault only after including your changes from file condition_variable.hpp

comment:45 by viboes, 7 years ago

Resolution: fixed
Status: reopenedclosed

comment:46 by boriss@…, 7 years ago

Resolution: fixed
Status: closedreopened

We have prepared small example where we have problem with condition_variable_any: .h file:

class ThreadsTest:public CppUnit::TestFixture
{
  CPPUNIT_TEST_SUITE(ThreadsTest);
  CPPUNIT_TEST(testThreads);
  CPPUNIT_TEST_SUITE_END();

public:
  void setUp();
  void tearDown();
  void testThreads();
  void runThread();
  boost::recursive_mutex m_CsQueuedItems;
  boost::condition_variable_any m_EvQueuedItems;
  bool mIsInterrupted;
};

.cpp file:

void ThreadsTest::runThread()
{
  while (!mIsInterrupted)
  {
    {
      boost::recursive_mutex::scoped_lock _lock1(m_CsQueuedItems);
      boost::chrono::time_point<boost::chrono::steady_clock> untilTime(boost::chrono::steady_clock::now() + boost::chrono::milliseconds(500));
      m_EvQueuedItems.wait_until(_lock1, untilTime);
    }
  }
}

void ThreadsTest::testThreads()
{
  mIsInterrupted=false;
  {
    boost::recursive_mutex::scoped_lock _lock1(m_CsQueuedItems);
    boost::thread newThread(boost::bind(&ThreadsTest::runThread, this));
  }
  ThreadBase::Sleep(1000);
  mIsInterrupted=true;
  ThreadBase::Sleep(1000);
}

We used boost1.60.0 beta1, rc4 with added definition BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC.

We got segmentation fault error. testThreads() function is part of unit test and is called first.

Can you please check the example and let us know if you see any problem with this code. If there is no problem with our example, please try to fix the problem in boost library.

comment:47 by viboes, 7 years ago

Do you use version 2, 3, or 4 (BOOST_THREAD_VERSION)?

comment:48 by viboes, 7 years ago

Could you post a complete example with the back trace of the segmentation fault?

in reply to:  47 ; comment:49 by boriss@…, 7 years ago

Replying to viboes:

Do you use version 2, 3, or 4 (BOOST_THREAD_VERSION)?

We use version 2

by boriss@…, 7 years ago

Attachment: ThreadsTest.cpp added

by boriss@…, 7 years ago

Attachment: gdb.bt added

in reply to:  48 ; comment:50 by boriss@…, 7 years ago

Replying to viboes:

Could you post a complete example with the back trace of the segmentation fault?

Please find attached example (ThreadsTest.cpp) and back trace (gdb.bt)

in reply to:  49 comment:51 by viboes, 7 years ago

Replying to boriss@…:

Replying to viboes:

Do you use version 2, 3, or 4 (BOOST_THREAD_VERSION)?

We use version 2

Sorry, I missed your reply.

Could you use version 4?

in reply to:  50 comment:52 by viboes, 7 years ago

Replying to boriss@…:

Replying to viboes:

Could you post a complete example with the back trace of the segmentation fault?

Please find attached example (ThreadsTest.cpp) and back trace (gdb.bt)

The backtrace is pointing to lines that let me think that BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC is not defined.

#4  0x00000000004339de in boost::condition_variable_any::wait_until<boost::unique_lock<boost::recursive_mutex>, boost::chrono::duration<long, boost::ratio<1l, 1000000000l> > > (this=0x7fffffffe108, lock=..., t=...) at /usr/local/include/boost/thread/pthread/condition_variable.hpp:311

Am I missing something?

Anyway, the bt indicates a loop on line 311. The code in line #311 is

        template <class lock_type, class Duration>
        cv_status
        wait_until(
            lock_type& lock,
            const chrono::time_point<chrono::steady_clock, Duration>& t)
        {
            using namespace chrono;
            typedef time_point<steady_clock, nanoseconds> nano_sys_tmpt;
            wait_until(lock,
                        nano_sys_tmpt(ceil<nanoseconds>(t.time_since_epoch())));
            return steady_clock::now() < t ? cv_status::no_timeout :
                                             cv_status::timeout;
        }

The problem is in line #339

        inline cv_status wait_until(
            unique_lock<mutex>& lk,
            chrono::time_point<chrono::steady_clock, chrono::nanoseconds> tp)
        {
            using namespace chrono;
            nanoseconds d = tp.time_since_epoch();
            timespec ts = boost::detail::to_timespec(d);
            if (do_wait_until(lk, ts)) return cv_status::no_timeout;
            else return cv_status::timeout;
        }

Instead of using

            lock_type& lock,

uses

            unique_lock<mutex>& lk,

Hrr. I will fix this very quickly.

comment:54 by viboes, 7 years ago

Resolution: fixed
Status: reopenedclosed

comment:55 by boriss@…, 7 years ago

Resolution: fixed
Status: closedreopened

With Boost 1.60 + above mentioned patch we get compile error durring compilaton of our application:

/usr/local/include/boost/thread/pthread/condition_variable.hpp: In destructor ‘boost::thread_cv_detail::lock_on_exit<MutexType>::~lock_on_exit() [with MutexType = int]’:
/usr/local/include/boost/thread/pthread/condition_variable.hpp:400:57:   instantiated from ‘bool boost::condition_variable_any::do_wait_until(lock_type&, const timespec&) [with lock_type = int]’
/usr/local/include/boost/thread/pthread/condition_variable.hpp:346:37:   instantiated from here
/usr/local/include/boost/thread/pthread/condition_variable.hpp:52:21: error: request for member ‘lock’ in ‘*((boost::thread_cv_detail::lock_on_exit<int>*)this)->boost::thread_cv_detail::lock_on_exit<int>::m’, which is of non-class type ‘int’
/usr/local/include/boost/thread/pthread/condition_variable.hpp: In member function ‘void boost::thread_cv_detail::lock_on_exit<MutexType>::activate(MutexType&) [with MutexType = int]’:
/usr/local/include/boost/thread/pthread/condition_variable.hpp:406:15:   instantiated from ‘bool boost::condition_variable_any::do_wait_until(lock_type&, const timespec&) [with lock_type = int]’
/usr/local/include/boost/thread/pthread/condition_variable.hpp:346:37:   instantiated from here
/usr/local/include/boost/thread/pthread/condition_variable.hpp:45:17: error: request for member ‘unlock’ in ‘m_’, which is of non-class type ‘int’

We see also that you changed only condition_variable.hpp

comment:56 by viboes, 7 years ago

Please, could you give me an example?

comment:57 by viboes, 7 years ago

Please could you add

        template <class lock_type>

just before

        inline bool do_wait_until(
          lock_type& m,
          struct timespec const &timeout)
        {

and tell me if this fixes the compilation error?

comment:58 by anonymous, 7 years ago

This line "template <class lock_type>" is already there, even in the version at: https://github.com/boostorg/thread/blob/develop/include/boost/thread/pthread/condition_variable.hpp

  • see line 393

comment:59 by viboes, 7 years ago

The issue is in line 339

https://github.com/boostorg/thread/blob/develop/include/boost/thread/pthread/condition_variable.hpp#L339

Sorry it was before this line

inline cv_status wait_until
Last edited 7 years ago by viboes (previous) (diff)

comment:61 by boriss@…, 7 years ago

After your latest fix our application compile successfully. Also changing computer time doesn't cause problems anymore. Thanks for your support.

comment:62 by viboes, 7 years ago

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