Created attachment 30576 [details] Small test case, build with g++ -std=c++0x When using sleep_until() I get an bug with unsigned long scalar representations of a duration. If this duratoiin is in past, then you get an overflow in the length of the argument for sleep_for(). This causes an almost infinte sleep, instead of a fast return. I solved this with defining two seperate implementions for sleep_until using enable_if. ################################################# /// sleep_until for signed representations template<typename _Clock, typename _Duration> inline typename std::enable_if<std::is_signed<typename _Duration::rep>::value, void>::type sleep_until(const chrono::time_point<_Clock, _Duration>& __atime) { sleep_for(__atime - _Clock::now()); } /// sleep_until for unsigned representations template<typename _Clock, typename _Duration> inline typename std::enable_if<std::is_unsigned<typename _Duration::rep>::value, void>::type sleep_until(const chrono::time_point<_Clock, _Duration>& __atime) { typename _Clock::time_point _now = _Clock::now(); // check if we should sleep till a time point in past if(__atime > _now) // if not, procede as usual sleep_for(__atime - _now); } ################################################# Sorry for not providing a .patch file, as I'm hacked my local installed headers.
I think this should be fixed in <chrono> not <thread>
On second thoughts, <chrono> is doing everything as specified by the standard. It seems unfortunate that we need this, but I don't immediately see any way around it. I think I'd prefer to do this than use SFINAE: template<typename _Clock, typename _Duration> inline void sleep_until(const chrono::time_point<_Clock, _Duration>& __atime) { auto __now = _Clock::now(); // check if we should sleep till a time point in past if (std::is_unsigned<typename _Duration::rep>::value && __atime <= __now) return; sleep_for(__atime - __now); }
Or we could just check unconditionally, which handles situations where the clock is adjusted while sleeping: /// sleep_until template<typename _Clock, typename _Duration> inline void sleep_until(const chrono::time_point<_Clock, _Duration>& __atime) { auto __now = _Clock::now(); while (__now < __atime) { sleep_for(__atime - __now); __now = _Clock::now(); } } I have another timing related bug to fix so will deal with this too.
Thanks!
Hello Jonathan, I wonder whether this bug is solved or not, as you already posted in July a posible solution? Best Regards, Mario
It's not fixed yet, sorry
I also encountered this bug trying to use std::this_thread::sleep_until() for video frame spacing (so the sleeps are very short, and sometimes become negative). Without this fix there is no way to safely use std::this_thread::sleep_until(), because even if the caller checks for negative sleep durations, there is always the chance that the thread gets preempted after entering the function, and before it checks the current time. Theoretically speaking any amount of time can elapse between those 2 points.
Here is another case where this bug caused a hard-to-find problem - http://stackoverflow.com/questions/17574287/boostthread-attributes-setting-call-stack-size (last comment) Since a patch is already available, is there an ETA for this bug to be fixed? I'd love to be able to take out my workaround in my program. Thanks!
It seems the same problem affects sleep_for, with negative values of magnitude at least one second: std::this_thread::sleep_for(std::chrono::seconds(-1)); Tested with Fedora's libstdc++ 4.8.3.
Author: redi Date: Thu Mar 26 19:59:08 2015 New Revision: 221708 URL: https://gcc.gnu.org/viewcvs?rev=221708&root=gcc&view=rev Log: PR libstdc++/58038 PR libstdc++/60421 * include/std/thread (this_thread::sleep_for): Check for negative durations. (this_thread::sleep_until): Check for times in the past. * testsuite/30_threads/this_thread/58038.cc: New. * testsuite/30_threads/this_thread/60421.cc: New. Added: trunk/libstdc++-v3/testsuite/30_threads/this_thread/58038.cc trunk/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/std/thread
(In reply to Matthew Lai from comment #8) > Here is another case where this bug caused a hard-to-find problem - > http://stackoverflow.com/questions/17574287/boostthread-attributes-setting- > call-stack-size > > (last comment) That seems to be about Boost.Thread so nothing to do with us.
Fixed for gcc5.
(In reply to Jonathan Wakely from comment #11) > (In reply to Matthew Lai from comment #8) > > Here is another case where this bug caused a hard-to-find problem - > > http://stackoverflow.com/questions/17574287/boostthread-attributes-setting- > > call-stack-size > > > > (last comment) > > That seems to be about Boost.Thread so nothing to do with us. Yeah that's in Boost, but it's the exact same bug in Boost. I linked it just to show the kind of problem this can cause. And thanks for the fix!
Author: redi Date: Sat Apr 11 11:47:09 2015 New Revision: 222003 URL: https://gcc.gnu.org/viewcvs?rev=222003&root=gcc&view=rev Log: PR libstdc++/58038 * include/std/thread (this_thread::sleep_for): Check for negative durations. (this_thread::sleep_until): Check for times in the past. * testsuite/30_threads/this_thread/58038.cc: New. * testsuite/30_threads/this_thread/60421.cc: New. Added: branches/gcc-4_9-branch/libstdc++-v3/testsuite/30_threads/this_thread/58038.cc branches/gcc-4_9-branch/libstdc++-v3/testsuite/30_threads/this_thread/60421.cc Modified: branches/gcc-4_9-branch/libstdc++-v3/ChangeLog branches/gcc-4_9-branch/libstdc++-v3/include/std/thread
Also fixed for 4.9.3