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 --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h
index 44a68645e4771b7870240d86245b9c62e156f62a..2b19c9c6c6ac0a35bfe63a654e1239ff5ee32b9e 100644 (file)
--- 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 (file)
index 0000000..bf6cd14
--- /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 53fe34d07530f92ac5977bbe9a3cbb90941776b1..330f8887e583621fb23e2ab1580a364c00a61616 100644 (file)
--- 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();
 }