Modify

Ticket #7665 (reopened Bugs)

Opened 18 months ago

Last modified 10 days ago

this_thread::sleep_for no longer uses steady_clock in thread

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

Description

In 1.51 sleep_for slept for set time no matter what [unless it received interrupt]. In 1.52 sleep_for behaves like normal sleep [system_clock, which introduces problems witch changing time while sleeping] if in new thread.

How to reproduce: Compile and run the test. Change time [date -s 10:00:00].

Main thread will exit sleep_for after 10 seconds. Boost::thread will end after some other time.

In correct situation [ex. boost 1.51] they should end simultaneously.

I think it's introduced in revision 80450.

Example program demonstrating this behavior is attached.

Attachments

sleep_for_test.cpp Download (485 bytes) - added by ewdevel@… 18 months ago.
Test for sleep_for that fails when time is changed.

Change History

Changed 18 months ago by ewdevel@…

Test for sleep_for that fails when time is changed.

comment:1 Changed 18 months ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from new to assigned

Which platform and compiler are you using?

comment:2 Changed 18 months ago by viboes

Hi, I have checked your program on MacOs? gcc-4.2.1, and I'm unable to reproduce the behavior you reported

gt-s8600:test viboes$ date
Jeu  8 nov 2012 22:00:03 CET
gt-s8600:test viboes$ ./a.out
Main thread START
Sleeping for 10 seconds - change time
Main thread END
Ended
gt-s8600:test viboes$ date
Jeu  8 nov 2012 10:10:06 CET

I have changed the date to 10:10

Last edited 18 months ago by viboes (previous) (diff)

comment:3 follow-up: ↓ 4 Changed 17 months ago by Eryk Wieliczko <ewdevel@…>

It seems that mac version [guessing from boost/chrono/detail/inlined] uses different sources for time keeping.

I've been able to reproduce this bug under: CentOS 6.2 x64 [gcc 4.4.6] VirtualBox?'ed Ubuntu 10.04 x64 [gcc 4.6.1]

Can someone check this under different linux distros?

comment:4 in reply to: ↑ 3 Changed 17 months ago by viboes

Replying to Eryk Wieliczko <ewdevel@…>:

It seems that mac version [guessing from boost/chrono/detail/inlined] uses different sources for time keeping.

I've been able to reproduce this bug under: CentOS 6.2 x64 [gcc 4.4.6] VirtualBox?'ed Ubuntu 10.04 x64 [gcc 4.6.1]

OK I will check.

comment:5 Changed 17 months ago by viboes

I don't know if there is a regression in trunk of Boost.Thread on Ubuntu as a lot of tests are failing independently of setting the date. It is surprising that the code is working on Ubuntu 12.04-64 testers. I'll continue the analysis.

Last edited 17 months ago by viboes (previous) (diff)

comment:6 Changed 17 months ago by viboes

  • Summary changed from this_thread::sleep_for no longer uses steady_clock in thread to l
  • Milestone changed from To Be Determined to Boost 1.53.0

I've made some changes on the sleep_for algo depending on BOOST_THREAD_SLEEP_FOR_IS_STEADY is defined or not. have you the possibility to check if this rework solves the issue?

Committed in trunk [81649]

comment:7 Changed 17 months ago by viboes

  • Summary changed from l to sleep_for doesn't use steady_clock

comment:8 Changed 17 months ago by viboes

  • Summary changed from sleep_for doesn't use steady_clock to this_thread::sleep_for no longer uses steady_clock in thread

comment:9 Changed 16 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

I hope the modification fixes the bug. Reopen it if you see the issue on 1.53 or on trunk.

comment:10 Changed 2 weeks ago by Volker_pruess@…

I think this needs to become reopened. We observe regression with boost 1.54.0, 1.55.0 and latest git version. Additionally the discussion #6787 also points in this direction.

We're using Ubuntu 12.04.4 LTS 64 bit for x86.

comment:11 Changed 2 weeks ago by viboes

And what do you think of the proposition in #6787?

comment:12 Changed 2 weeks ago by viboes

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:13 Changed 13 days ago by Volker Prüß <Volker_Pruess@…>

I'm not quite sure about the solution you proposed but I see the problem :

  • 'boost::this_thread::sleep_for' has to remain an interruption point
  • there are no standard library or pthread functions allowing interuptible operation based upon a stable clock, especially for a portable library.

I checked against the c++ library provided alongside with the gcc 4.6.3 and at least their implementation of sleep_for is not affected by changes in the system clock. It seems that the gcc implementation is based upon nanosleep ( http://stackoverflow.com/questions/12523122/what-is-glibcxx-use-nanosleep-all-about), IMHO therefore not interruptible.

I'm not quite sure whether an implementation of sleep_for (and sleep) that is affected by changes made to the system clock would be acceptable anyway. And moving sleep_for out of the namespace 'this_thread' would break existing code.

Seems to be a question for the standard library experts ...

comment:14 Changed 13 days ago by viboes

The standard doesn't includes interruptible threads. And Boost can do whatever we decide without the standard library experts advice ;-)

So it is up to you to decide if we want additional non-interruptible functions or not. By compatibility, we can not replace the current behavior.

comment:15 Changed 12 days ago by Volker Prüß <Volker_Pruess@…>

When you refer to the current behaviour you mean that boost::this_thread::sleep_for has been mentioned in the list of predefined interruption points ? The documentation about sleep_for also says

Suspends the current thread until the duration specified by by rel_time has elapsed.

Question is which clock is used to measure the duration. I would expect that most programmers would intuitively expect a stable clock here. But referring to the type of clock in the documentation would do it. So I would agree to your suggestion adding another set of methods.

comment:16 Changed 10 days ago by Volker Prüß <Volker_Pruess@…>

I saw that sleep_for is using pthread_cond_wait to achieve thread suspension and being interruptible. Is it possible to initialize the condition variable to use the monotonic clock as described  here ?

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as reopened
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.