[Bug libstdc++/58038] New: std::this_thread::sleep_until can cause inifinite sleep

[Bug libstdc++/58038] New: std::this_thread::sleep_until can cause inifinite sleep

[mario.bielert@tu-dresden.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

            Bug ID: 58038
           Summary: std::this_thread::sleep_until can cause inifinite
                    sleep
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mario.bielert@tu-dresden.de

Created attachment 30576
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30576&action=edit
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.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think this should be fixed in <chrono> not <thread>

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-07-31
     Ever confirmed|0                           |1

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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);
    }

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Thanks!

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[mario.bielert@tu-dresden.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #5 from Mario Bielert <mario.bielert@tu-dresden.de> ---
Hello Jonathan,

I wonder whether this bug is solved or not, as you already posted in July a
posible solution?

Best Regards,

Mario

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's not fixed yet, sorry

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[m at matthewlai dot ca]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Matthew Lai <m at matthewlai dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |m at matthewlai dot ca

--- Comment #7 from Matthew Lai <m at matthewlai dot ca> ---
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.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[m at matthewlai dot ca]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #8 from Matthew Lai <m at matthewlai dot ca> ---
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!

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[dan at stahlke dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Dan Stahlke <dan at stahlke dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dan at stahlke dot org

--- Comment #9 from Dan Stahlke <dan at stahlke dot org> ---
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.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |5.0

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for gcc5.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(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.

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[m at matthewlai dot ca]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #13 from Matthew Lai <m at matthewlai dot ca> ---
(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!

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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

[Bug libstdc++/58038] std::this_thread::sleep_until can cause inifinite sleep

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58038

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |4.9.3

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Also fixed for 4.9.3