https://gcc.gnu.org/g:01670a4095791733e0389acead832e3da757c9d7
commit r15-3639-g01670a4095791733e0389acead832e3da757c9d7 Author: Jonathan Wakely <jwak...@redhat.com> Date: Mon Sep 2 12:29:04 2024 +0100 libstdc++: Refactor loops in std::__platform_semaphore Refactor the loops to all use the same form, and to not need explicit 'break' or 'continue' jumps. This also avoids a -Wunused-variable warning with -Wsystem-headers. Also fix a bug for absolute timeouts specified with a time that isn't implicitly convertible to __clock_t::time_point, e.g. one with a higher resolution such as picoseconds. Use chrono::ceil to round up to the next time point representable by the clock. libstdc++-v3/ChangeLog: * include/bits/semaphore_base.h (__platform_semaphore): Refactor loops to all use similar forms. (__platform_semaphore::_M_try_acquire_until): Use chrono::ceil to explicitly convert to __clock_t::time_point. * testsuite/30_threads/semaphore/try_acquire_for.cc: Check that using a very high resolution timeout compiles. * testsuite/30_threads/semaphore/platform_try_acquire_for.cc: New test. Diff: --- libstdc++-v3/include/bits/semaphore_base.h | 58 ++++++++-------------- .../semaphore/platform_try_acquire_for.cc | 7 +++ .../30_threads/semaphore/try_acquire_for.cc | 13 +++++ 3 files changed, 40 insertions(+), 38 deletions(-) diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h index 44a68645e477..2b19c9c6c6ac 100644 --- a/libstdc++-v3/include/bits/semaphore_base.h +++ b/libstdc++-v3/include/bits/semaphore_base.h @@ -73,52 +73,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_ALWAYS_INLINE void _M_acquire() noexcept { - for (;;) - { - auto __err = sem_wait(&_M_semaphore); - if (__err && (errno == EINTR)) - continue; - else if (__err) - std::__terminate(); - else - break; - } + while (sem_wait(&_M_semaphore)) + if (errno != EINTR) + std::__terminate(); } _GLIBCXX_ALWAYS_INLINE bool _M_try_acquire() noexcept { - for (;;) + while (sem_trywait(&_M_semaphore)) { - auto __err = sem_trywait(&_M_semaphore); - if (__err && (errno == EINTR)) - continue; - else if (__err && (errno == EAGAIN)) + if (errno == EAGAIN) // already locked return false; - else if (__err) + else if (errno != EINTR) std::__terminate(); - else - break; + // else got EINTR so retry } return true; } _GLIBCXX_ALWAYS_INLINE void - _M_release(std::ptrdiff_t __update) noexcept + _M_release(ptrdiff_t __update) noexcept { for(; __update != 0; --__update) - { - auto __err = sem_post(&_M_semaphore); - if (__err) - std::__terminate(); - } + if (sem_post(&_M_semaphore)) + std::__terminate(); } bool _M_try_acquire_until_impl(const chrono::time_point<__clock_t>& __atime) noexcept { - auto __s = chrono::time_point_cast<chrono::seconds>(__atime); auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); @@ -128,19 +113,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static_cast<long>(__ns.count()) }; - for (;;) + while (sem_timedwait(&_M_semaphore, &__ts)) { - if (auto __err = sem_timedwait(&_M_semaphore, &__ts)) - { - if (errno == EINTR) - continue; - else if (errno == ETIMEDOUT || errno == EINVAL) - return false; - else - std::__terminate(); - } - else - break; + if (errno == ETIMEDOUT) + return false; + else if (errno != EINTR) + std::__terminate(); } return true; } @@ -152,10 +130,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if constexpr (std::is_same_v<__clock_t, _Clock>) { - return _M_try_acquire_until_impl(__atime); + using _Dur = __clock_t::duration; + return _M_try_acquire_until_impl(chrono::ceil<_Dur>(__atime)); } else { + // TODO: if _Clock is monotonic_clock we could use + // sem_clockwait with CLOCK_MONOTONIC. + const typename _Clock::time_point __c_entry = _Clock::now(); const auto __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc b/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc new file mode 100644 index 000000000000..bf6cd142bf01 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc @@ -0,0 +1,7 @@ +// { dg-options "-D_GLIBCXX_USE_POSIX_SEMAPHORE" } +// { dg-do run { target c++20 } } +// { dg-additional-options "-pthread" { target pthread } } +// { dg-require-gthreads "" } +// { dg-add-options libatomic } + +#include "try_acquire_for.cc" 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 53fe34d07530..330f8887e583 100644 --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc @@ -78,8 +78,21 @@ void test02() b.wait(1); } +void +test03() +{ + using tick = std::chrono::system_clock::duration::period; + using halftick = std::ratio<tick::num, 2 * tick::den>; + std::chrono::duration<long long, halftick> timeout(1); + std::binary_semaphore s(1); + + // Using higher resolution than chrono::system_clock should compile: + s.try_acquire_for(timeout); +} + int main() { test01(); test02(); + test03(); }