Opened 7 years ago
Closed 7 years ago
#11377 closed Bugs (fixed)
Boost condition variable always waits for system clock deadline
Reported by: | 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)
Change History (19)
by , 7 years ago
Attachment: | clock_changes.cpp added |
---|
by , 7 years ago
Attachment: | clock_changes_test.sh added |
---|
Test harness to run clock_changes and set the system clock
by , 7 years ago
Attachment: | condition_variable_steady.hpp added |
---|
condition_variable header rewritten to use steady clock instead of system clock
comment:1 by , 7 years ago
Component: | threads → thread |
---|
Please, don't use the threads component but the thread one.
comment:2 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 4 comment:3 by , 7 years ago
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 by , 7 years ago
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 by , 7 years ago
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 by , 7 years ago
Milestone: | To Be Determined → Boost 1.60.0 |
---|
Add to develop branch https://github.com/boostorg/thread/commit/9f883f6ad7377b88b79fa70cc5de68cbfb0213e4
comment:7 by , 7 years ago
Milestone: | Boost 1.60.0 → To Be Determined |
---|---|
Status: | assigned → new |
comment:8 by , 7 years ago
Status: | new → assigned |
---|
follow-up: 10 comment:9 by , 7 years ago
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 by , 7 years ago
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 by , 7 years ago
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:
- 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.
- 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 by , 7 years ago
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 by , 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
follow-up: 15 comment:14 by , 7 years ago
Milestone: | To Be Determined → Boost 1.60.0 |
---|
Please, could you try the develop branch
https://github.com/boostorg/thread/commit/8f5de1d7c34ff7248261874f273498a622bf76d4
comment:15 by , 7 years ago
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 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Test code to wait in various ways using boost threads/condition variables