From 1a72ea7bb8ba27679dd8b179a4671d7d9b099d4b Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Sat, 8 Apr 2023 09:52:09 -0700 Subject: Synchronization: Support true relative timeouts using the POSIX proposed standard pthread_cond_clockwait() and sem_clockwait(). These are currently implemented in glibc >= 2.30. These methods take a clock and use an absolute time with reference to that clock, so KernelTimeout now can produce these values. PiperOrigin-RevId: 522824226 Change-Id: Ife98713f6f95d800b1f8e52d5364a3dbebc4f8a6 --- absl/synchronization/internal/kernel_timeout.cc | 35 ++++++++++++++- absl/synchronization/internal/kernel_timeout.h | 28 ++++++++++-- .../internal/kernel_timeout_test.cc | 52 ++++++++++++++++++++++ absl/synchronization/internal/pthread_waiter.cc | 10 +++++ absl/synchronization/internal/sem_waiter.cc | 34 +++++++++++--- absl/synchronization/internal/sem_waiter.h | 2 + 6 files changed, 150 insertions(+), 11 deletions(-) diff --git a/absl/synchronization/internal/kernel_timeout.cc b/absl/synchronization/internal/kernel_timeout.cc index 2e485097..bfbf282f 100644 --- a/absl/synchronization/internal/kernel_timeout.cc +++ b/absl/synchronization/internal/kernel_timeout.cc @@ -14,6 +14,10 @@ #include "absl/synchronization/internal/kernel_timeout.h" +#ifndef _WIN32 +#include +#endif + #include #include // NOLINT(build/c++11) #include @@ -101,7 +105,7 @@ int64_t KernelTimeout::MakeAbsNanos() const { return kMaxNanos; } - int64_t nanos = RawNanos(); + int64_t nanos = RawAbsNanos(); if (is_relative_timeout()) { // We need to change epochs, because the relative timeout might be @@ -128,7 +132,7 @@ int64_t KernelTimeout::InNanosecondsFromNow() const { return kMaxNanos; } - int64_t nanos = RawNanos(); + int64_t nanos = RawAbsNanos(); if (is_absolute_timeout()) { return std::max(nanos - absl::GetCurrentTimeNanos(), 0); } @@ -143,6 +147,33 @@ struct timespec KernelTimeout::MakeRelativeTimespec() const { return absl::ToTimespec(absl::Nanoseconds(InNanosecondsFromNow())); } +#ifndef _WIN32 +struct timespec KernelTimeout::MakeClockAbsoluteTimespec(clockid_t c) const { + if (!has_timeout()) { + return absl::ToTimespec(absl::Nanoseconds(kMaxNanos)); + } + + int64_t nanos = RawAbsNanos(); + if (is_absolute_timeout()) { + nanos -= absl::GetCurrentTimeNanos(); + } else { + nanos -= SteadyClockNow(); + } + + struct timespec now; + ABSL_RAW_CHECK(clock_gettime(c, &now) == 0, "clock_gettime() failed"); + absl::Duration from_clock_epoch = + absl::DurationFromTimespec(now) + absl::Nanoseconds(nanos); + if (from_clock_epoch <= absl::ZeroDuration()) { + // Some callers have assumed that 0 means no timeout, so instead we return a + // time of 1 nanosecond after the epoch. For safety we also do not return + // negative values. + return absl::ToTimespec(absl::Nanoseconds(1)); + } + return absl::ToTimespec(from_clock_epoch); +} +#endif + KernelTimeout::DWord KernelTimeout::InMillisecondsFromNow() const { constexpr DWord kInfinite = std::numeric_limits::max(); diff --git a/absl/synchronization/internal/kernel_timeout.h b/absl/synchronization/internal/kernel_timeout.h index 952bd2c6..4e361a6a 100644 --- a/absl/synchronization/internal/kernel_timeout.h +++ b/absl/synchronization/internal/kernel_timeout.h @@ -15,6 +15,10 @@ #ifndef ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ #define ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ +#ifndef _WIN32 +#include +#endif + #include #include // NOLINT(build/c++11) #include @@ -78,6 +82,18 @@ class KernelTimeout { // this method in the case of a spurious wakeup. struct timespec MakeRelativeTimespec() const; +#ifndef _WIN32 + // Convert to `struct timespec` for interfaces that expect an absolute timeout + // on a specific clock `c`. This is similar to `MakeAbsTimespec()`, but + // callers usually want to use this method with `CLOCK_MONOTONIC` when + // relative timeouts are requested, and when the appropriate interface expects + // an absolute timeout relative to a specific clock (for example, + // pthread_cond_clockwait() or sem_clockwait()). If !has_timeout(), attempts + // to convert to a reasonable absolute timeout, but callers should to test + // has_timeout() prefer to use a more appropriate interface. + struct timespec MakeClockAbsoluteTimespec(clockid_t c) const; +#endif + // Convert to unix epoch nanos for interfaces that expect an absolute timeout // in nanoseconds. If !has_timeout() or is_relative_timeout(), attempts to // convert to a reasonable absolute timeout, but callers should to test @@ -125,12 +141,18 @@ class KernelTimeout { // after the unix epoch. // - If the low bit is 1, then the high 63 bits is the number of nanoseconds // after the epoch used by SteadyClockNow(). + // + // In all cases the time is stored as an absolute time, the only difference is + // the clock epoch. The use of absolute times is important since in the case + // of a relative timeout with a spurious wakeup, the program would have to + // restart the wait, and thus needs a way of recomputing the remaining time. uint64_t rep_; // Returns the number of nanoseconds stored in the internal representation. - // Together with is_absolute_timeout() and is_relative_timeout(), the return - // value is used to compute when the timeout should occur. - int64_t RawNanos() const { return static_cast(rep_ >> 1); } + // When combined with the clock epoch indicated by the low bit (which is + // accessed through is_absolute_timeout() and is_relative_timeout()), the + // return value is used to compute when the timeout should occur. + int64_t RawAbsNanos() const { return static_cast(rep_ >> 1); } // Converts to nanoseconds from now. Since the return value is a relative // duration, it should be recomputed by calling this method in the case of a diff --git a/absl/synchronization/internal/kernel_timeout_test.cc b/absl/synchronization/internal/kernel_timeout_test.cc index 26ee34a2..c853c4bc 100644 --- a/absl/synchronization/internal/kernel_timeout_test.cc +++ b/absl/synchronization/internal/kernel_timeout_test.cc @@ -64,6 +64,13 @@ TEST(KernelTimeout, FiniteTimes) { EXPECT_TRUE(t.is_absolute_timeout()); EXPECT_FALSE(t.is_relative_timeout()); EXPECT_EQ(absl::TimeFromTimespec(t.MakeAbsTimespec()), when); +#ifndef _WIN32 + EXPECT_LE( + absl::AbsDuration(absl::Now() + duration - + absl::TimeFromTimespec( + t.MakeClockAbsoluteTimespec(CLOCK_REALTIME))), + absl::Milliseconds(10)); +#endif EXPECT_LE( absl::AbsDuration(absl::DurationFromTimespec(t.MakeRelativeTimespec()) - std::max(duration, absl::ZeroDuration())), @@ -89,6 +96,10 @@ TEST(KernelTimeout, InfiniteFuture) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -110,6 +121,10 @@ TEST(KernelTimeout, DefaultConstructor) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -131,6 +146,10 @@ TEST(KernelTimeout, TimeMaxNanos) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -152,6 +171,10 @@ TEST(KernelTimeout, Never) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -170,6 +193,10 @@ TEST(KernelTimeout, InfinitePast) { EXPECT_FALSE(t.is_relative_timeout()); EXPECT_LE(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::FromUnixNanos(1)); +#ifndef _WIN32 + EXPECT_LE(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::FromUnixSeconds(1)); +#endif EXPECT_EQ(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::ZeroDuration()); EXPECT_LE(absl::FromUnixNanos(t.MakeAbsNanos()), absl::FromUnixNanos(1)); @@ -200,6 +227,13 @@ TEST(KernelTimeout, FiniteDurations) { EXPECT_LE(absl::AbsDuration(absl::Now() + duration - absl::TimeFromTimespec(t.MakeAbsTimespec())), absl::Milliseconds(5)); +#ifndef _WIN32 + EXPECT_LE( + absl::AbsDuration(absl::Now() + duration - + absl::TimeFromTimespec( + t.MakeClockAbsoluteTimespec(CLOCK_REALTIME))), + absl::Milliseconds(5)); +#endif EXPECT_LE( absl::AbsDuration(absl::DurationFromTimespec(t.MakeRelativeTimespec()) - duration), @@ -241,6 +275,12 @@ TEST(KernelTimeout, NegativeDurations) { EXPECT_LE(absl::AbsDuration(absl::Now() - absl::TimeFromTimespec(t.MakeAbsTimespec())), absl::Milliseconds(5)); +#ifndef _WIN32 + EXPECT_LE(absl::AbsDuration(absl::Now() - absl::TimeFromTimespec( + t.MakeClockAbsoluteTimespec( + CLOCK_REALTIME))), + absl::Milliseconds(5)); +#endif EXPECT_EQ(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::ZeroDuration()); EXPECT_LE( @@ -263,6 +303,10 @@ TEST(KernelTimeout, InfiniteDuration) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -284,6 +328,10 @@ TEST(KernelTimeout, DurationMaxNanos) { // absl::InfiniteFuture(), but we should return a very large value. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), @@ -305,6 +353,10 @@ TEST(KernelTimeout, OverflowNanos) { // Timeouts should still be far in the future. EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), absl::Now() + absl::Hours(100000)); +#ifndef _WIN32 + EXPECT_GT(absl::TimeFromTimespec(t.MakeClockAbsoluteTimespec(CLOCK_REALTIME)), + absl::Now() + absl::Hours(100000)); +#endif EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), absl::Hours(100000)); EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), diff --git a/absl/synchronization/internal/pthread_waiter.cc b/absl/synchronization/internal/pthread_waiter.cc index a8dafd94..8d90cc45 100644 --- a/absl/synchronization/internal/pthread_waiter.cc +++ b/absl/synchronization/internal/pthread_waiter.cc @@ -78,6 +78,11 @@ PthreadWaiter::PthreadWaiter() : waiter_count_(0), wakeup_count_(0) { #define ABSL_INTERNAL_HAS_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP 1 #endif +#if defined(__GLIBC__) && \ + (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30)) +#define ABSL_INTERNAL_HAVE_PTHREAD_COND_CLOCKWAIT 1 +#endif + // Calls pthread_cond_timedwait() or possibly something else like // pthread_cond_timedwait_relative_np() depending on the platform and // KernelTimeout requested. The return value is the same as the return @@ -94,6 +99,11 @@ int PthreadWaiter::TimedWait(KernelTimeout t) { #ifdef ABSL_INTERNAL_HAS_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP const auto rel_timeout = t.MakeRelativeTimespec(); return pthread_cond_timedwait_relative_np(&cv_, &mu_, &rel_timeout); +#elif defined(ABSL_INTERNAL_HAVE_PTHREAD_COND_CLOCKWAIT) && \ + defined(CLOCK_MONOTONIC) + const auto abs_clock_timeout = t.MakeClockAbsoluteTimespec(CLOCK_MONOTONIC); + return pthread_cond_clockwait(&cv_, &mu_, CLOCK_MONOTONIC, + &abs_clock_timeout); #endif } diff --git a/absl/synchronization/internal/sem_waiter.cc b/absl/synchronization/internal/sem_waiter.cc index 89af5de2..7dd27fb7 100644 --- a/absl/synchronization/internal/sem_waiter.cc +++ b/absl/synchronization/internal/sem_waiter.cc @@ -43,12 +43,34 @@ SemWaiter::SemWaiter() : wakeups_(0) { } } -bool SemWaiter::Wait(KernelTimeout t) { - struct timespec abs_timeout; - if (t.has_timeout()) { - abs_timeout = t.MakeAbsTimespec(); +#if defined(__GLIBC__) && \ + (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30)) +#define ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT 1 +#endif + +// Calls sem_timedwait() or possibly something else like +// sem_clockwait() depending on the platform and +// KernelTimeout requested. The return value is the same as a call to the return +// value to a call to sem_timedwait(). +int SemWaiter::TimedWait(KernelTimeout t) { +#ifndef __GOOGLE_GRTE_VERSION__ + constexpr bool kRelativeTimeoutSupported = true; +#else + constexpr bool kRelativeTimeoutSupported = false; +#endif + + if (kRelativeTimeoutSupported && t.is_relative_timeout()) { +#if defined(ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT) && defined(CLOCK_MONOTONIC) + const auto abs_clock_timeout = t.MakeClockAbsoluteTimespec(CLOCK_MONOTONIC); + return sem_clockwait(&sem_, CLOCK_MONOTONIC, &abs_clock_timeout); +#endif } + const auto abs_timeout = t.MakeAbsTimespec(); + return sem_timedwait(&sem_, &abs_timeout); +} + +bool SemWaiter::Wait(KernelTimeout t) { // Loop until we timeout or consume a wakeup. // Note that, since the thread ticker is just reset, we don't need to check // whether the thread is idle on the very first pass of the loop. @@ -73,10 +95,10 @@ bool SemWaiter::Wait(KernelTimeout t) { if (errno == EINTR) continue; ABSL_RAW_LOG(FATAL, "sem_wait failed: %d", errno); } else { - if (sem_timedwait(&sem_, &abs_timeout) == 0) break; + if (TimedWait(t) == 0) break; if (errno == EINTR) continue; if (errno == ETIMEDOUT) return false; - ABSL_RAW_LOG(FATAL, "sem_timedwait failed: %d", errno); + ABSL_RAW_LOG(FATAL, "SemWaiter::TimedWait() failed: %d", errno); } } first_pass = false; diff --git a/absl/synchronization/internal/sem_waiter.h b/absl/synchronization/internal/sem_waiter.h index 47d5bf3d..c22746f9 100644 --- a/absl/synchronization/internal/sem_waiter.h +++ b/absl/synchronization/internal/sem_waiter.h @@ -46,6 +46,8 @@ class SemWaiter : public WaiterCrtp { static constexpr char kName[] = "SemWaiter"; private: + int TimedWait(KernelTimeout t); + sem_t sem_; // This seems superfluous, but for Poke() we need to cause spurious -- cgit v1.2.3