Message ID | YIGm7Va6B5Is6iwA@redhat.com |
---|---|
State | New |
Headers | show |
Series | libstdc++: Fix semaphore to work with system_clock timeouts | expand |
On Thu, Apr 22, 2021 at 8:48 PM Jonathan Wakely via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The __cond_wait_until_impl function takes a steady_clock timeout, but > then sometimes tries to compare it to a time from the system_clock, > which won't compile. Additionally, that function gets called with > system_clock timeouts, which also won't compile. This makes the function > accept timeouts for either clock, and compare to the time from the right > clock. > > This fixes the compilation error that was causing two tests to fail on > non-futex targets, so we can revert the r12-11 change to disable them. > > libstdc++-v3/ChangeLog: > > * include/bits/atomic_timed_wait.h (__cond_wait_until_impl): > Handle system_clock as well as steady_clock. > * testsuite/30_threads/semaphore/try_acquire_for.cc: Re-enable. > * testsuite/30_threads/semaphore/try_acquire_until.cc: > Re-enable. > > I'm testing this now on x86_64-linux, powerpc64le-linux, sparc-linux, > power-aix and sparc-solaris. It looks good so far, so I'll push to > trunk when the tests finish. > > This should also go to the gcc-11 branch, or the timed waits for > semaphores can't be used with system_clock times on non-futed targets. Fine with me. > >
diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h index 70e5335cfd7..ec7ff51cdbc 100644 --- a/libstdc++-v3/include/bits/atomic_timed_wait.h +++ b/libstdc++-v3/include/bits/atomic_timed_wait.h @@ -139,12 +139,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // (e.g. __ulock_wait())which is better than pthread_cond_clockwait #endif // ! PLATFORM_TIMED_WAIT - // returns true if wait ended before timeout - template<typename _Dur> + // Returns true if wait ended before timeout. + // _Clock must be either steady_clock or system_clock. + template<typename _Clock, typename _Dur> bool __cond_wait_until_impl(__condvar& __cv, mutex& __mx, - const chrono::time_point<chrono::steady_clock, _Dur>& __atime) + const chrono::time_point<_Clock, _Dur>& __atime) { + static_assert(std::__is_one_of<_Clock, chrono::steady_clock, + chrono::system_clock>::value); + auto __s = chrono::time_point_cast<chrono::seconds>(__atime); auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); @@ -155,12 +159,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT - __cv.wait_until(__mx, CLOCK_MONOTONIC, __ts); - return chrono::steady_clock::now() < __atime; -#else - __cv.wait_until(__mx, __ts); - return chrono::system_clock::now() < __atime; -#endif // ! _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT + if constexpr (is_same_v<chrono::steady_clock, _Clock>) + __cv.wait_until(__mx, CLOCK_MONOTONIC, __ts); + else +#endif + __cv.wait_until(__mx, __ts); + return _Clock::now() < __atime; } // returns true if wait ended before timeout diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc index 248ecb07e56..e7edc9eeef1 100644 --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc @@ -21,8 +21,6 @@ // { dg-require-gthreads "" } // { dg-add-options libatomic } -// { dg-skip-if "FIXME: fails" { ! futex } } - #include <semaphore> #include <chrono> #include <thread> diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc index eb1351cd2bf..49ba33b4999 100644 --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc @@ -21,8 +21,6 @@ // { dg-additional-options "-pthread" { target pthread } } // { dg-add-options libatomic } -// { dg-skip-if "FIXME: fails" { ! futex } } - #include <semaphore> #include <chrono> #include <thread>