Modify

Opened 2 years ago

Closed 2 years ago

#11377 closed Bugs (fixed)

Boost condition variable always waits for system clock deadline

Reported by: Dave Bacher <dbacher@…> Owned by: viboes
Milestone: Boost 1.60.0 Component: thread
Version: Boost 1.56.0 Severity: Problem
Keywords: condition variable, clock Cc:

Description

On Linux systems using pthreads, the Boost condition variable wait operations are always converted to use the system clock as a time base. This causes problems when the system clock is changed. When the system clock is set back during the wait, the wait operation does not return until the system clock has returned to the absolute time calculated when the wait operation was scheduled.

The attached test code and test harness demonstrates this behavior. On my Ubuntu Linux 14.04 system, the test code (when run with the 'until' or 'for' wait operations) hangs after the harness sets the system clock.

(see attached test case: build clock_changes and run clock_changes_test.sh)

g++ -g -Wall -o clock_changes clock_changes.cpp -I/tools/boost_1_56_0 -L/tools/boost_1_56_0/stage/lib -lboost_thread -lboost_chrono -lboost_program_options -lboost_system -lpthread

In our application (and I suppose in a variety of use cases), the application wants to wait for a time relative to a monotonic clock regardless of the behavior of the system clock.

The boost sleep operations demonstrate the correct behavior in the face of system clock changes.

The Pthread API supports using a monotonic clock for timed waits by setting the clock when the condition variable is initialized. And Issue 7665 seems to imply that condition_variable will be changed to use a monotonic clock: https://svn.boost.org/trac/boost/ticket/7665#comment:16

In case it's useful, I have attached a modification to boost::condition_variable to create a new class boost::condition_variable_steady that behaves consistently in the face of system clock changes.

(see condition_variable_steady.hpp)

I'm not sure what the right behavior should be in boost, but I would suggest at least providing a class that operates independently of the system clock.

Note: I'm currently using boost 1.56.0, but the condition_variable.hpp header is effectively the same in 1.57.0 and 1.58.0, so I'm almost certain the same issue exists in the most recent release (1.58.0).

Attachments (3)

clock_changes.cpp (2.9 KB) - added by Dave Bacher <dbacher@…> 2 years ago.
Test code to wait in various ways using boost threads/condition variables
clock_changes_test.sh (345 bytes) - added by Dave Bacher <dbacher@…> 2 years ago.
Test harness to run clock_changes and set the system clock
condition_variable_steady.hpp (9.5 KB) - added by Dave Bacher <dbacher@…> 2 years ago.
condition_variable header rewritten to use steady clock instead of system clock

Download all attachments as: .zip

Change History (19)

Changed 2 years ago by Dave Bacher <dbacher@…>

Attachment: clock_changes.cpp added

Test code to wait in various ways using boost threads/condition variables

Changed 2 years ago by Dave Bacher <dbacher@…>

Attachment: clock_changes_test.sh added

Test harness to run clock_changes and set the system clock

Changed 2 years ago by Dave Bacher <dbacher@…>

condition_variable header rewritten to use steady clock instead of system clock

comment:1 Changed 2 years ago by viboes

Component: threadsthread

Please, don't use the threads component but the thread one.

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

comment:2 Changed 2 years ago by viboes

Owner: changed from Anthony Williams to viboes
Status: newassigned

comment:3 Changed 2 years ago by viboes

Hi,

there is already a family of no_interruption_point sleeps that don't use condition variables

boost::this_thread::no_interruption_point::sleep_for
boost::this_thread::no_interruption_point::sleep_until

Does this fix you issue?

Could you resume what is the difference between your condition_variable_steady and the condition_variable?

comment:4 in reply to:  3 Changed 2 years ago by David Bacher <dbacher@…>

Thanks for your attention.

Does this fix you issue?

Unfortunately, no. The issue is not with sleep operations, but with semaphore wait operations. When we wait on a semaphore and the system time changes, the wait does not return until the absolute time of the system clock reaches the deadline. Thus, if the system clock is set backwards while the process waits on a semaphore, the process may end up waiting longer than it should -- a 5 second wait may turn into a 1 minute wait if the system clock is set backwards by one minute. In our use case, we don't care about the absolute system time, we just want to time out after a specific duration.

Could you resume what is the difference between your condition_variable_steady and the condition_variable?

There is a minor change in condition_variable_steady to use the monotonic clock when initializing the semaphore and performing wait operations. This fixes the problem described above because the monotonic clock is not affected by changes in the system time.

comment:5 Changed 2 years ago by viboes

Ok, I have adapted a patch from your condition_variable_steady 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.

I will commit it soon on develop branch. Could you test it?

comment:6 Changed 2 years ago by viboes

Milestone: To Be DeterminedBoost 1.60.0

comment:7 Changed 2 years ago by viboes

Milestone: Boost 1.60.0To Be Determined
Status: assignednew

comment:8 Changed 2 years ago by viboes

Status: newassigned

comment:9 Changed 2 years ago by Dave Bacher <dbacher@…>

Thanks for addressing this. I will try to help test.

As of 9/8, I'm unable to build the thread library from the master or develop branches:

$ ./b2 --with-thread
...
gcc.compile.c++ bin.v2/libs/thread/build/gcc-4.8/release/threading-multi/pthread/thread.o
In file included from ./boost/utility/result_of.hpp:202:0,
                 from ./boost/thread/detail/invoker.hpp:28,
                 from ./boost/thread/future.hpp:20,
                 from libs/thread/src/pthread/thread.cpp:19:
./boost/preprocessor/iteration/detail/iter/forward1.hpp:47:37: fatal error: boost/utility/detail/result_of_iterate.hpp: No such file or directory
 #        include BOOST_PP_FILENAME_1
                                     ^
compilation terminated.

    "g++"  -ftemplate-depth-128 -O3 -finline-functions -Wno-inline -Wall -pedantic -pthread -fPIC -m64 -Wextra -Wno-long-long -Wno-unused-parameter -Wno-variadic-macros -Wunused-function -pedantic -DBOOST_ALL_NO_LIB=1 -DBOOST_ATOMIC_DYN_LINK=1 -DBOOST_SYSTEM_DYN_LINK=1 -DBOOST_THREAD_BUILD_DLL=1 -DBOOST_THREAD_DONT_USE_CHRONO -DBOOST_THREAD_POSIX -DNDEBUG  -I"." -c -o "bin.v2/libs/thread/build/gcc-4.8/release/threading-multi/pthread/thread.o" "libs/thread/src/pthread/thread.cpp"

I'm new to building boost from the git sources, so it's possible I'm doing something wrong. The chrono and system libraries build fine in my clone.

comment:10 in reply to:  9 Changed 2 years ago by Dave Bacher <dbacher@…>

Replying to Dave Bacher <dbacher@…>:

As of 9/8, I'm unable to build the thread library from the master or develop branches:

Sorry, I checked the Boost.Thread Jenkins build and found I was missing the "headers" build step.

comment:11 Changed 2 years ago by Dave Bacher <dbacher@…>

The patch is working properly for me.

With #define BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC, the test code waits for the specified duration even with the adjustment of the system clock. In the output below, the test completes after 5 seconds even though the system clock was adjusted backwards. Previously, the test would hang until the system clock reached the time the test was started plus 5 seconds.

$ sh clock_changes_test.sh 
[sudo] password: 
Setup sudo authentication
Starting test to wait for 5 seconds
Adjusting time to 01:00:00
Waiting for 5 sec
WAIT_UNTIL
Tue Sep  8 01:00:00 PDT 2015
Waiting...
Steady duration/start/stop: 5
   3104032404935 nanoseconds since boot / 3109032558479 nanoseconds since boot
System duration/start/stop: -52747 seconds
   1441751952904865440 nanoseconds since Jan 1, 1970 / 1441699204998073008 nanoseconds since Jan 1, 1970
Test complete, please check your system clock
Tue Sep  8 01:00:05 PDT 2015

Regarding when to use the monotonic clock, I think there are separate two issues:

  1. Policy -- when should the monotonic clock be used? I would prefer that using the monotonic clock is the default. But perhaps other applications have other expectations.
  1. Availability -- how to determine if the monotonic clock is available? On my Linux system, linux/time.h #defines CLOCK_MONOTONIC. At runtime, clock_gettime returns an error if the specified clock id is not available.

comment:12 Changed 2 years ago by viboes

Thanks for checking the patch, and glad to see it works for you.

As soon as I'm able to detect if the following works as expected I will swithc the implementation on platforms supporting it.

  pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);

comment:13 Changed 2 years 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:14 Changed 2 years ago by viboes

Milestone: To Be DeterminedBoost 1.60.0

comment:15 in reply to:  14 Changed 2 years ago by Dave Bacher <dbacher@…>

Replying to viboes:

Please, could you try the develop branch

https://github.com/boostorg/thread/commit/8f5de1d7c34ff7248261874f273498a622bf76d4

The latest develop branch works for my test case. My test code explicitly #defines BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC.

comment:16 Changed 2 years ago by viboes

Resolution: fixed
Status: assignedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain viboes.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.