[Bug libstdc++/60421] New: std::this_thread::sleep_for doesn't sleep for all arguments

[Bug libstdc++/60421] New: std::this_thread::sleep_for doesn't sleep for all arguments

[jaak at ristioja dot ee]

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

            Bug ID: 60421
           Summary: std::this_thread::sleep_for doesn't sleep for all
                    arguments
           Product: gcc
           Version: 4.8.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jaak at ristioja dot ee

#include <chrono>
  #include <limits>
  #include <thread>
  int main() {
   
std::this_thread::sleep_for(std::chrono::duration<uint64_t>(std::numeric_limits<uint64_t>::max()));
    return 0;
  }

This doesn't even sleep a millisecond. Relevant strace output:

  nanosleep({18446744073709551615, 0}, NULL) = -1 EINVAL (Invalid argument)

Hence the §30.3.2.7 of the C++11 standard is violated as sleep_for does not
sleep by the relative amount of time specified by its argument. Bug may be run
into during testing.

Also happens with 4.7.3.
>From gcc-bugs-return-445449-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Mar 05 09:24:08 2014
Return-Path: <gcc-bugs-return-445449-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 27677 invoked by alias); 5 Mar 2014 09:24:08 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 27633 invoked by uid 48); 5 Mar 2014 09:24:05 -0000
From: "redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libstdc++/60422] New: <bits/xxx.h> headers should give a diagnostic when included directly
Date: Wed, 05 Mar 2014 09:24:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libstdc++
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: redi at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter
Message-ID: <bug-60422-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-03/txt/msg00318.txt.bz2
Content-length: 953

http://gcc.gnu.org/bugzilla/show_bug.cgi?id`422

            Bug ID: 60422
           Summary: <bits/xxx.h> headers should give a diagnostic when
                    included directly
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org

Some users do dumb things like:

#include <algorithm>
#include <ext/concurrence.h>
#include <bits/unique_ptr.h>
#include <bits/shared_ptr.h>

and expect to be able to use std::shared_ptr.  This happens to work with GCC
4.8, but not with 4.9 because we re-arranged some headers to allow the library
to use shared_ptr internally.

In order to stop this nonsense we should add preprocessor checks to the
headers:

#ifndef _MEMORY
#error Do not include this header directly, use <memory> instead
#else
...
#endif

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[plasmahh at gmx dot net]

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

Dennis Lubert <plasmahh at gmx dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |plasmahh at gmx dot net

--- Comment #1 from Dennis Lubert <plasmahh at gmx dot net> ---
Note that for me to reproduce this, _GLIBCXX_USE_NANOSLEEP must be defined. In
that case it uses directly the nanosleep call, casting the uint64max value to
time_t which is signed (which is unfortunately not visible in strace), gets
negative and this is what the nonsleep manpage says for glibc:

       EINVAL The value in the tv_nsec field was not in the range 0 to
999999999 or tv_sec was negative.

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
should probably be fixed along with PR 58038

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-09-19
   Target Milestone|---                         |5.0
     Ever confirmed|0                           |1

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[redi at gcc dot gnu.org]

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

--- Comment #3 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++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[redi at gcc dot gnu.org]

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

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

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

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

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[jaak at ristioja dot ee]

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

--- Comment #5 from Jaak Ristioja <jaak at ristioja dot ee> ---
(In reply to Jonathan Wakely from comment #4)
> Fixed for gcc5.

Looking at the diff <thread> of revision 221708, I fail to see how the

  if (__rtime <= __rtime.zero())
    return;

check in sleep_for() prevents the EINVAL in nanosleep(3p), as is the original
issue.

I mean aren't we dealing here with unsigned (decltype(_rtime)) to signed
(std::time_t) conversion?

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[jaak at ristioja dot ee]

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

Jaak Ristioja <jaak at ristioja dot ee> changed:

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

--- Comment #6 from Jaak Ristioja <jaak at ristioja dot ee> ---
I mean when I sleep for UINT64_MAX hours/years/millenia, you can't possibly
wrap that into a single nanosleep call due to the limitations of the type
time_t of the tv_sec parameter of the first argument to nanosleep. One
obviously can not get around using loop.

Additionally, the nanosleep code is also missing proper EINTR handling, which
again could break the sleep.

Reopening.

[Bug libstdc++/60421] std::this_thread::sleep_for doesn't sleep for all arguments

[redi at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
You're right, sorry, I ended up reducing the scope of the fix and it only deals
with PR58038, see https://gcc.gnu.org/ml/libstdc++/2015-03/msg00078.html for
some commentary.

At one point I added:

  if (__s < chrono::seconds::zero() || __s < __rtime)
    __s = chrono::seconds::max();

which would detect some cases where the duration_cast to chrono::seconds
overflows, but that overflow is still undefined behaviour so I didn't include
that in the commit. I want to fix it properly to avoid any undefined behaviour
(probably converting to duration<long double> and comparing to
duration<time_t>::max()).

> I mean when I sleep for UINT64_MAX hours/years/millenia, you can't possibly
> wrap that into a single nanosleep call due to the limitations of the type
> time_t of the tv_sec parameter of the first argument to nanosleep. One
> obviously can not get around using loop.

To be honest, I'm not very concerned about the failure to sleep for 290 billion
years, if we sleep for duration<time_t>::max() instead of
duration<uint64_t>::max() and don't loop you'll never know the difference.

> Additionally, the nanosleep code is also missing proper EINTR handling,
> which again could break the sleep.

Yes, see the mailing list post above.

I'll deal with this for gcc 6.