Modify

Opened 5 years ago

Closed 21 months ago

#7665 closed Bugs (fixed)

this_thread::sleep_for no longer uses steady_clock in thread

Reported by: ewdevel@… Owned by: viboes
Milestone: Boost 1.60.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 (1)

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

Download all attachments as: .zip

Change History (33)

Changed 5 years ago by ewdevel@…

Test for sleep_for that fails when time is changed.

comment:1 Changed 5 years 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 5 years 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 5 years ago by viboes (previous) (diff)

comment:3 follow-up: Changed 5 years 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 5 years 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 5 years 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 5 years ago by viboes (previous) (diff)

comment:6 Changed 5 years ago by viboes

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

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 5 years ago by viboes

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

comment:8 Changed 5 years 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 5 years ago by viboes

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

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

comment:10 Changed 3 years 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 3 years ago by viboes

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

comment:12 Changed 3 years ago by viboes

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:13 Changed 3 years 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 3 years 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 3 years 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 3 years 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 ?

comment:17 Changed 3 years ago by viboes

I was not aware of pthread_condattr_setclock. I will adapt condition_variable to use it on the platforms providing it.

Thanks for the pointer.

comment:18 Changed 3 years ago by viboes

Unfortunately pthread_condattr_setclock is not available on MacOs??.

comment:19 Changed 3 years ago by viboes

  • Milestone changed from Boost 1.53.0 to To Be Determined

comment:20 Changed 3 years ago by viboes

  • Milestone changed from To Be Determined to Boost 1.57.0

comment:21 Changed 3 years ago by viboes

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

comment:22 Changed 22 months ago by viboes

  • Milestone changed from Boost 1.57.0 to To Be Determined
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm writing a patch that uses pthread_condattr_setclock. As I said 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.

Is there any one that can test it?

comment:23 Changed 22 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.60.0

comment:24 Changed 22 months ago by viboes

  • Status changed from reopened to new

comment:25 Changed 22 months ago by viboes

  • Milestone changed from Boost 1.60.0 to To Be Determined

comment:26 Changed 22 months ago by viboes

  • Status changed from new to assigned

comment:27 Changed 21 months ago by 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

comment:29 Changed 21 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.60.0

comment:30 Changed 21 months ago by mail@…

I'm experiencing the same problem (Linux Mint, 3.13.0-37-generic # 64-Ubuntu SMP, boost 1.54).

Can confirm that the threads are behaving differently with the test code.

Checked out modular-boost, switched to develop for boost-thread, compiled everything and tried the test code above. Adding

#define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC 1

fixes the bug for me.

comment:31 Changed 21 months ago by viboes

Thanks for the report.

comment:32 Changed 21 months ago by viboes

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

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain viboes.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.