Synchronization: Consolidate the logic for whether steady clocks are supported
for relative timeouts
PiperOrigin-RevId: 523789416
Change-Id: Ide4cfdcae9ea7bffca3355c80ea9c8833a9536e6
diff --git a/absl/synchronization/BUILD.bazel b/absl/synchronization/BUILD.bazel
index b1c4e9a..b6721e1 100644
--- a/absl/synchronization/BUILD.bazel
+++ b/absl/synchronization/BUILD.bazel
@@ -61,7 +61,9 @@
"//absl/synchronization:__pkg__",
],
deps = [
+ "//absl/base",
"//absl/base:config",
+ "//absl/base:core_headers",
"//absl/base:raw_logging_internal",
"//absl/time",
],
@@ -75,6 +77,7 @@
deps = [
":kernel_timeout_internal",
"//absl/base:config",
+ "//absl/random",
"//absl/time",
"@com_google_googletest//:gtest_main",
],
@@ -348,6 +351,7 @@
":kernel_timeout_internal",
":synchronization",
":thread_pool",
+ "//absl/random",
"//absl/time",
"@com_google_googletest//:gtest_main",
],
diff --git a/absl/synchronization/CMakeLists.txt b/absl/synchronization/CMakeLists.txt
index 9926fb7..b6b3b27 100644
--- a/absl/synchronization/CMakeLists.txt
+++ b/absl/synchronization/CMakeLists.txt
@@ -44,7 +44,9 @@
COPTS
${ABSL_DEFAULT_COPTS}
DEPS
+ absl::base
absl::config
+ absl::core_headers
absl::raw_logging_internal
absl::time
)
@@ -59,6 +61,7 @@
DEPS
absl::kernel_timeout_internal
absl::config
+ absl::random_random
absl::time
GTest::gmock_main
)
@@ -257,6 +260,7 @@
${ABSL_TEST_COPTS}
DEPS
absl::kernel_timeout_internal
+ absl::random_random
absl::synchronization
absl::thread_pool
absl::time
diff --git a/absl/synchronization/internal/futex.h b/absl/synchronization/internal/futex.h
index 8d97326..55078f1 100644
--- a/absl/synchronization/internal/futex.h
+++ b/absl/synchronization/internal/futex.h
@@ -83,31 +83,6 @@
class FutexImpl {
public:
- // Atomically check that `*v == val`, and if it is, then sleep until the
- // timeout `t` has been reached, or until woken by `Wake()`.
- static int WaitUntil(std::atomic<int32_t>* v, int32_t val,
- KernelTimeout t) {
- // Monotonic waits are disabled for production builds because go/btm
- // requires synchronized clocks.
- // TODO(b/160682823): Find a way to enable this when BTM is not enabled
- // since production builds don't always run on Borg.
-#if defined(CLOCK_MONOTONIC) && !defined(__GOOGLE_GRTE_VERSION__)
- constexpr bool kRelativeTimeoutSupported = true;
-#else
- constexpr bool kRelativeTimeoutSupported = false;
-#endif
-
- if (!t.has_timeout()) {
- return Wait(v, val);
- } else if (kRelativeTimeoutSupported && t.is_relative_timeout()) {
- auto rel_timespec = t.MakeRelativeTimespec();
- return WaitRelativeTimeout(v, val, &rel_timespec);
- } else {
- auto abs_timespec = t.MakeAbsTimespec();
- return WaitAbsoluteTimeout(v, val, &abs_timespec);
- }
- }
-
// Atomically check that `*v == val`, and if it is, then sleep until the until
// woken by `Wake()`.
static int Wait(std::atomic<int32_t>* v, int32_t val) {
diff --git a/absl/synchronization/internal/futex_waiter.cc b/absl/synchronization/internal/futex_waiter.cc
index 0a76bc9..7c07fbe 100644
--- a/absl/synchronization/internal/futex_waiter.cc
+++ b/absl/synchronization/internal/futex_waiter.cc
@@ -35,6 +35,28 @@
constexpr char FutexWaiter::kName[];
#endif
+int FutexWaiter::WaitUntil(std::atomic<int32_t>* v, int32_t val,
+ KernelTimeout t) {
+#ifdef CLOCK_MONOTONIC
+ constexpr bool kHasClockMonotonic = true;
+#else
+ constexpr bool kHasClockMonotonic = false;
+#endif
+
+ // We can't call Futex::WaitUntil() here because the prodkernel implementation
+ // does not know about KernelTimeout::SupportsSteadyClock().
+ if (!t.has_timeout()) {
+ return Futex::Wait(v, val);
+ } else if (kHasClockMonotonic && KernelTimeout::SupportsSteadyClock() &&
+ t.is_relative_timeout()) {
+ auto rel_timespec = t.MakeRelativeTimespec();
+ return Futex::WaitRelativeTimeout(v, val, &rel_timespec);
+ } else {
+ auto abs_timespec = t.MakeAbsTimespec();
+ return Futex::WaitAbsoluteTimeout(v, val, &abs_timespec);
+ }
+}
+
bool FutexWaiter::Wait(KernelTimeout t) {
// Loop until we can atomically decrement futex from a positive
// value, waiting on a futex while we believe it is zero.
@@ -54,7 +76,7 @@
}
if (!first_pass) MaybeBecomeIdle();
- const int err = Futex::WaitUntil(&futex_, 0, t);
+ const int err = WaitUntil(&futex_, 0, t);
if (err != 0) {
if (err == -EINTR || err == -EWOULDBLOCK) {
// Do nothing, the loop will retry.
diff --git a/absl/synchronization/internal/futex_waiter.h b/absl/synchronization/internal/futex_waiter.h
index a6a7209..11dfa93 100644
--- a/absl/synchronization/internal/futex_waiter.h
+++ b/absl/synchronization/internal/futex_waiter.h
@@ -43,6 +43,11 @@
static constexpr char kName[] = "FutexWaiter";
private:
+ // Atomically check that `*v == val`, and if it is, then sleep until the
+ // timeout `t` has been reached, or until woken by `Wake()`.
+ static int WaitUntil(std::atomic<int32_t>* v, int32_t val,
+ KernelTimeout t);
+
// Futexes are defined by specification to be 32-bits.
// Thus std::atomic<int32_t> must be just an int32_t with lockfree methods.
std::atomic<int32_t> futex_;
diff --git a/absl/synchronization/internal/kernel_timeout.cc b/absl/synchronization/internal/kernel_timeout.cc
index bfbf282..48ea628 100644
--- a/absl/synchronization/internal/kernel_timeout.cc
+++ b/absl/synchronization/internal/kernel_timeout.cc
@@ -21,9 +21,13 @@
#include <algorithm>
#include <chrono> // NOLINT(build/c++11)
#include <cstdint>
+#include <cstdlib>
+#include <cstring>
#include <ctime>
#include <limits>
+#include "absl/base/attributes.h"
+#include "absl/base/call_once.h"
#include "absl/base/config.h"
#include "absl/time/time.h"
@@ -37,15 +41,12 @@
#endif
int64_t KernelTimeout::SteadyClockNow() {
-#ifdef __GOOGLE_GRTE_VERSION__
- // go/btm requires synchronized clocks, so we have to use the system
- // clock.
- return absl::GetCurrentTimeNanos();
-#else
+ if (!SupportsSteadyClock()) {
+ return absl::GetCurrentTimeNanos();
+ }
return std::chrono::duration_cast<std::chrono::nanoseconds>(
std::chrono::steady_clock::now().time_since_epoch())
.count();
-#endif
}
KernelTimeout::KernelTimeout(absl::Time t) {
diff --git a/absl/synchronization/internal/kernel_timeout.h b/absl/synchronization/internal/kernel_timeout.h
index 4e361a6..06404a7 100644
--- a/absl/synchronization/internal/kernel_timeout.h
+++ b/absl/synchronization/internal/kernel_timeout.h
@@ -128,6 +128,11 @@
// case of a spurious wakeup.
std::chrono::nanoseconds ToChronoDuration() const;
+ // Returns true if steady (aka monotonic) clocks are supported by the system.
+ // This method exists because go/btm requires synchronized clocks, and
+ // thus requires we use the system (aka walltime) clock.
+ static constexpr bool SupportsSteadyClock() { return true; }
+
private:
// Returns the current time, expressed as a count of nanoseconds since the
// epoch used by an arbitrary clock. The implementation tries to use a steady
diff --git a/absl/synchronization/internal/kernel_timeout_test.cc b/absl/synchronization/internal/kernel_timeout_test.cc
index c853c4b..e3286f7 100644
--- a/absl/synchronization/internal/kernel_timeout_test.cc
+++ b/absl/synchronization/internal/kernel_timeout_test.cc
@@ -14,14 +14,34 @@
#include "absl/synchronization/internal/kernel_timeout.h"
+#include <ctime>
#include <chrono> // NOLINT(build/c++11)
#include <limits>
+#include "absl/random/random.h"
#include "gtest/gtest.h"
#include "absl/base/config.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
+// Test go/btm support by randomizing the value clock_gettime() for
+// CLOCK_MONOTONIC. This works by overriding a weak symbol in glibc.
+// We should be resistant to this randomization when !SupportsSteadyClock().
+#ifdef __GOOGLE_GRTE_VERSION__
+extern "C" int __clock_gettime(clockid_t c, struct timespec* ts);
+
+extern "C" int clock_gettime(clockid_t c, struct timespec* ts) {
+ if (c == CLOCK_MONOTONIC &&
+ !absl::synchronization_internal::KernelTimeout::SupportsSteadyClock()) {
+ absl::SharedBitGen gen;
+ ts->tv_sec = absl::Uniform(gen, 0, 1'000'000'000);
+ ts->tv_nsec = absl::Uniform(gen, 0, 1'000'000'000);
+ return 0;
+ }
+ return __clock_gettime(c, ts);
+}
+#endif // __GOOGLE_GRTE_VERSION__
+
namespace {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \
diff --git a/absl/synchronization/internal/pthread_waiter.cc b/absl/synchronization/internal/pthread_waiter.cc
index 8d90cc4..366aaea 100644
--- a/absl/synchronization/internal/pthread_waiter.cc
+++ b/absl/synchronization/internal/pthread_waiter.cc
@@ -88,14 +88,8 @@
// KernelTimeout requested. The return value is the same as the return
// value of pthread_cond_timedwait().
int PthreadWaiter::TimedWait(KernelTimeout t) {
-#ifndef __GOOGLE_GRTE_VERSION__
- constexpr bool kRelativeTimeoutSupported = true;
-#else
- constexpr bool kRelativeTimeoutSupported = false;
-#endif
-
assert(t.has_timeout());
- if (kRelativeTimeoutSupported && t.is_relative_timeout()) {
+ if (KernelTimeout::SupportsSteadyClock() && t.is_relative_timeout()) {
#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);
diff --git a/absl/synchronization/internal/sem_waiter.cc b/absl/synchronization/internal/sem_waiter.cc
index 7dd27fb..3b3849b 100644
--- a/absl/synchronization/internal/sem_waiter.cc
+++ b/absl/synchronization/internal/sem_waiter.cc
@@ -53,13 +53,7 @@
// 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 (KernelTimeout::SupportsSteadyClock() && 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);
diff --git a/absl/synchronization/internal/stdcpp_waiter.cc b/absl/synchronization/internal/stdcpp_waiter.cc
index 8b5d1df..355718a 100644
--- a/absl/synchronization/internal/stdcpp_waiter.cc
+++ b/absl/synchronization/internal/stdcpp_waiter.cc
@@ -50,7 +50,7 @@
if (!t.has_timeout()) {
cv_.wait(lock);
} else {
- auto wait_result = t.is_relative_timeout()
+ auto wait_result = t.SupportsSteadyClock() && t.is_relative_timeout()
? cv_.wait_for(lock, t.ToChronoDuration())
: cv_.wait_until(lock, t.ToChronoTimePoint());
if (wait_result == std::cv_status::timeout) {
diff --git a/absl/synchronization/internal/waiter_test.cc b/absl/synchronization/internal/waiter_test.cc
index 66b255d..57a1a3d 100644
--- a/absl/synchronization/internal/waiter_test.cc
+++ b/absl/synchronization/internal/waiter_test.cc
@@ -14,9 +14,11 @@
#include "absl/synchronization/internal/waiter.h"
+#include <ctime>
#include <iostream>
#include <ostream>
+#include "absl/random/random.h"
#include "absl/synchronization/internal/create_thread_identity.h"
#include "absl/synchronization/internal/futex_waiter.h"
#include "absl/synchronization/internal/kernel_timeout.h"
@@ -29,6 +31,24 @@
#include "absl/time/time.h"
#include "gtest/gtest.h"
+// Test go/btm support by randomizing the value clock_gettime() for
+// CLOCK_MONOTONIC. This works by overriding a weak symbol in glibc.
+// We should be resistant to this randomization when !SupportsSteadyClock().
+#ifdef __GOOGLE_GRTE_VERSION__
+extern "C" int __clock_gettime(clockid_t c, struct timespec* ts);
+
+extern "C" int clock_gettime(clockid_t c, struct timespec* ts) {
+ if (c == CLOCK_MONOTONIC &&
+ !absl::synchronization_internal::KernelTimeout::SupportsSteadyClock()) {
+ absl::SharedBitGen gen;
+ ts->tv_sec = absl::Uniform(gen, 0, 1'000'000'000);
+ ts->tv_nsec = absl::Uniform(gen, 0, 1'000'000'000);
+ return 0;
+ }
+ return __clock_gettime(c, ts);
+}
+#endif // __GOOGLE_GRTE_VERSION__
+
namespace {
TEST(Waiter, PrintPlatformImplementation) {
