You might notice that this removes handling of EINVAL from the call to
sem_timedwait. That error can only happen with a negative ts_nsec value,
which can only happen for a timestamp before the epoch. We should handle
that properly, not just for the case where ts_nsec happens to be
negative. I opened PR 116586 for that, as it's bigger than just
<semaphore>.
Tested x86_64-linux.
-- >8 --
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.
---
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(-)
create mode 100644
libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc
diff --git a/libstdc++-v3/include/bits/semaphore_base.h
b/libstdc++-v3/include/bits/semaphore_base.h
index 44a68645e47..2b19c9c6c6a 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 00000000000..bf6cd142bf0
--- /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 53fe34d0753..330f8887e58 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();
}
--
2.46.0
