Bug 68519 - condition_variable::wait_for does not work properly with float duration
Summary: condition_variable::wait_for does not work properly with float duration
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 5.1.1
: P3 normal
Target Milestone: 6.5
Assignee: Jonathan Wakely
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-24 13:49 UTC by Jorge Bellon
Modified: 2018-08-08 15:53 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-11-24 00:00:00


Attachments
preprocessed source file that reproduces the bug (125.30 KB, text/plain)
2015-11-24 13:49 UTC, Jorge Bellon
Details
source file that reproduces the bug (385 bytes, text/plain)
2015-11-24 13:50 UTC, Jorge Bellon
Details
source file that reproduces the bug (data race fixed) (384 bytes, text/plain)
2015-11-24 15:13 UTC, Jorge Bellon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jorge Bellon 2015-11-24 13:49:06 UTC
Created attachment 36823 [details]
preprocessed source file that reproduces the bug

If std::chrono::duration<float> is used as timeout datatype for std::condition_variable::wait_for, then the thread, apparently, does not become blocked.

Correct result is got by changing timeout type to std::chrono::duration<double>.
Expected output:
{{{
Do something...
Do something...
Do something...
Do something...
Exit...
}}}

G++ version and configuration:
Usando especificaciones internas.
COLLECT_GCC=/bin/g++
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/5.1.1/lto-wrapper
Objetivo: x86_64-redhat-linux
Configurado con: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --disable-libgcj --with-default-libstdcxx-abi=c++98 --with-isl --enable-libmpx --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Modelo de hilos: posix
gcc versión 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC)

Note: installed in Fedora 22 using official repositories.
Comment 1 Jorge Bellon 2015-11-24 13:50:08 UTC
Created attachment 36824 [details]
source file that reproduces the bug
Comment 2 Markus Trippelsdorf 2015-11-24 14:45:36 UTC
Build with -fsanitize=thread:

==================
WARNING: ThreadSanitizer: data race (pid=15270)
  Write of size 1 at 0x000000403540 by main thread:
    #0 main /var/tmp/t.cc:32 (a.out+0x0000004016d4)

  Previous read of size 1 at 0x000000403540 by thread T1 (mutexes: write M10):
Do something...
    #0 void thread_loop<std::chrono::duration<float, std::ratio<1l, 1l> > >(std::chrono::duration<float, std::ratio<1l, 1l> >) /var/tmp/t.cc:17 (a.out+0x000000401ad3)
Do something...
    #1 void std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/functional:1430 (a.out+0x00000040193e)
    #2 std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)>::operator()() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/functional:1419 (a.out+0x00000040193e)
Do something...
    #3 std::thread::_State_impl<std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)> >::_M_run() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/thread:186 (a.out+0x00000040193e)
    #4 execute_native_thread_routine <null> (libstdc++.so.6+0x0000000d082e)

  Location is global 'finish' of size 1 at 0x000000403540 (a.out+0x000000403540)

  Mutex M10 (0x0000004035a0) created at:
    #0 pthread_mutex_lock <null> (libtsan.so.0+0x000000042be0)
Do something...
    #1 __gthread_mutex_lock /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/x86_64-pc-linux-gnu/bits/gthr-default.h:748 (a.out+0x0000004019ee)
    #2 std::mutex::lock() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/bits/mutex.h:103 (a.out+0x0000004019ee)
    #3 std::unique_lock<std::mutex>::lock() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/bits/mutex.h:254 (a.out+0x0000004019ee)
    #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/bits/mutex.h:184 (a.out+0x0000004019ee)
Do something...
    #5 void thread_loop<std::chrono::duration<float, std::ratio<1l, 1l> > >(std::chrono::duration<float, std::ratio<1l, 1l> >) /var/tmp/t.cc:16 (a.out+0x0000004019ee)
    #6 void std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/functional:1430 (a.out+0x00000040193e)
    #7 std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)>::operator()() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/functional:1419 (a.out+0x00000040193e)
    #8 std::thread::_State_impl<std::_Bind_simple<void (*(std::chrono::duration<float, std::ratio<1l, 1l> >))(std::chrono::duration<float, std::ratio<1l, 1l> >)> >::_M_run() /usr/lib/gcc/x86_64-pc-linux-gnu/6.0.0/include/g++-v6/thread:186 (a.out+0x00000040193e)
Do something...
    #9 execute_native_thread_routine <null> (libstdc++.so.6+0x0000000d082e)

  Thread T1 (tid=15272, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x00000002bdd8)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0x0000000d0b34)
Do something...
    #2 __libc_start_main ../csu/libc-start.c:289 (libc.so.6+0x00000002062f)

SUMMARY: ThreadSanitizer: data race /var/tmp/t.cc:32 in main
==================
Comment 3 Jorge Bellon 2015-11-24 15:13:31 UTC
Created attachment 36827 [details]
source file that reproduces the bug (data race fixed)

I've uploaded again the source file with the fixed data race (typo when writting the example, sorry).

Now it passes both -fsanitize=thread and -fsanitize=address (but still does not work as expected).
Comment 4 Markus Trippelsdorf 2015-11-24 15:23:36 UTC
(In reply to Jorge Bellon from comment #3)
> Created attachment 36827 [details]
> source file that reproduces the bug (data race fixed)
> 
> I've uploaded again the source file with the fixed data race (typo when
> writting the example, sorry).
> 
> Now it passes both -fsanitize=thread and -fsanitize=address (but still does
> not work as expected).

OK. Thanks.
Comment 6 Jonathan Wakely 2017-12-11 10:52:08 UTC
That's why the bug is still open.
Comment 7 Jonathan Wakely 2017-12-14 15:45:15 UTC
The problem is that duration<float> doesn't have sufficient precision to represent now+1s as a float (the value is the same as now)

#include <chrono>
constexpr std::chrono::seconds now(1513266095);
constexpr auto then = now + std::chrono::duration<float>(1);
static_assert( now < then, "" );

The timed waiting functions would have to check for such rounding errors.
Comment 8 Jonathan Wakely 2017-12-14 20:42:24 UTC
Author: redi
Date: Thu Dec 14 20:41:52 2017
New Revision: 255665

URL: https://gcc.gnu.org/viewcvs?rev=255665&root=gcc&view=rev
Log:
PR libstdc++/68519 use native duration to avoid rounding errors

	PR libstdc++/68519
	* include/std/condition_variable (condition_variable::wait_for):
	Convert duration to native clock's duration before addition.
	* testsuite/30_threads/condition_variable/members/68519.cc: New test.

Added:
    trunk/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/condition_variable
Comment 9 Jonathan Wakely 2017-12-14 20:45:14 UTC
Fixed for GCC 8.
Comment 10 Jonathan Wakely 2018-08-07 22:51:12 UTC
Author: redi
Date: Tue Aug  7 22:50:40 2018
New Revision: 263380

URL: https://gcc.gnu.org/viewcvs?rev=263380&root=gcc&view=rev
Log:
PR libstdc++/68519 use native duration to avoid rounding errors

Backport from mainline
2017-12-14  Jonathan Wakely  <jwakely@redhat.com>

	PR libstdc++/68519
	* include/std/condition_variable (condition_variable::wait_for):
	Convert duration to native clock's duration before addition.
	* testsuite/30_threads/condition_variable/members/68519.cc: New test.

Added:
    branches/gcc-7-branch/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc
Modified:
    branches/gcc-7-branch/libstdc++-v3/ChangeLog
    branches/gcc-7-branch/libstdc++-v3/include/std/condition_variable
Comment 11 Jonathan Wakely 2018-08-07 22:55:16 UTC
Fixed for 7.4 too.
Comment 12 Jonathan Wakely 2018-08-08 15:41:17 UTC
Author: redi
Date: Wed Aug  8 15:40:41 2018
New Revision: 263420

URL: https://gcc.gnu.org/viewcvs?rev=263420&root=gcc&view=rev
Log:
PR libstdc++/68519 use native duration to avoid rounding errors

Backport from mainline
2017-12-14  Jonathan Wakely  <jwakely@redhat.com>

	PR libstdc++/68519
	* include/std/condition_variable (condition_variable::wait_for):
	Convert duration to native clock's duration before addition.
	* testsuite/30_threads/condition_variable/members/68519.cc: New test.

Added:
    branches/gcc-6-branch/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc
Modified:
    branches/gcc-6-branch/libstdc++-v3/ChangeLog
    branches/gcc-6-branch/libstdc++-v3/include/std/condition_variable
Comment 13 Jonathan Wakely 2018-08-08 15:53:11 UTC
Fixed for 6.5 too.