diff --git a/libc/bionic/bionic_futex.cpp b/libc/bionic/bionic_futex.cpp index 0ac1f6ea6..aa8234fb0 100644 --- a/libc/bionic/bionic_futex.cpp +++ b/libc/bionic/bionic_futex.cpp @@ -28,6 +28,7 @@ #include "private/bionic_futex.h" +#include #include #include "private/bionic_time_conversions.h" @@ -35,7 +36,6 @@ static inline __always_inline int FutexWithTimeout(volatile void* ftx, int op, int value, bool use_realtime_clock, const timespec* abs_timeout, int bitset) { - const timespec* futex_abs_timeout = abs_timeout; // pthread's and semaphore's default behavior is to use CLOCK_REALTIME, however this behavior is // essentially never intended, as that clock is prone to change discontinuously. // @@ -46,16 +46,26 @@ static inline __always_inline int FutexWithTimeout(volatile void* ftx, int op, i // We have seen numerous bugs directly attributable to this difference. Therefore, we provide // this general workaround to always use CLOCK_MONOTONIC for waiting, regardless of what the input // timespec is. - timespec converted_monotonic_abs_timeout; - if (abs_timeout && use_realtime_clock) { - monotonic_time_from_realtime_time(converted_monotonic_abs_timeout, *abs_timeout); - if (converted_monotonic_abs_timeout.tv_sec < 0) { + timespec converted_timeout; + if (abs_timeout) { + if ((op & FUTEX_CMD_MASK) == FUTEX_LOCK_PI) { + if (!use_realtime_clock) { + realtime_time_from_monotonic_time(converted_timeout, *abs_timeout); + abs_timeout = &converted_timeout; + } + } else { + op &= ~FUTEX_CLOCK_REALTIME; + if (use_realtime_clock) { + monotonic_time_from_realtime_time(converted_timeout, *abs_timeout); + abs_timeout = &converted_timeout; + } + } + if (abs_timeout->tv_sec < 0) { return -ETIMEDOUT; } - futex_abs_timeout = &converted_monotonic_abs_timeout; } - return __futex(ftx, op, value, futex_abs_timeout, bitset); + return __futex(ftx, op, value, abs_timeout, bitset); } int __futex_wait_ex(volatile void* ftx, bool shared, int value, bool use_realtime_clock, @@ -66,6 +76,22 @@ int __futex_wait_ex(volatile void* ftx, bool shared, int value, bool use_realtim int __futex_pi_lock_ex(volatile void* ftx, bool shared, bool use_realtime_clock, const timespec* abs_timeout) { - return FutexWithTimeout(ftx, (shared ? FUTEX_LOCK_PI : FUTEX_LOCK_PI_PRIVATE), 0, - use_realtime_clock, abs_timeout, 0); + // We really want FUTEX_LOCK_PI2 which is default CLOCK_MONOTONIC, but that isn't supported + // on linux before 5.14. FUTEX_LOCK_PI uses CLOCK_REALTIME. Here we verify support. + + static atomic_int lock_op = 0; + int op = atomic_load_explicit(&lock_op, memory_order_relaxed); + if (op == 0) { + uint32_t tmp = 0; + if (__futex(&tmp, FUTEX_LOCK_PI2, 0, nullptr, 0) == 0) { + __futex(&tmp, FUTEX_UNLOCK_PI, 0, nullptr, 0); + op = FUTEX_LOCK_PI2; + } else { + op = FUTEX_LOCK_PI; + } + atomic_store_explicit(&lock_op, op, memory_order_relaxed); + } + + if (!shared) op |= FUTEX_PRIVATE_FLAG; + return FutexWithTimeout(ftx, op, 0 /* value */, use_realtime_clock, abs_timeout, 0 /* bitset */); } diff --git a/libc/bionic/bionic_time_conversions.cpp b/libc/bionic/bionic_time_conversions.cpp index d21e12eb8..9f3c50dff 100644 --- a/libc/bionic/bionic_time_conversions.cpp +++ b/libc/bionic/bionic_time_conversions.cpp @@ -52,23 +52,32 @@ void timeval_from_timespec(timeval& tv, const timespec& ts) { tv.tv_usec = ts.tv_nsec / 1000; } -void monotonic_time_from_realtime_time(timespec& monotonic_time, const timespec& realtime_time) { - monotonic_time = realtime_time; +static void convert_timespec_clocks(timespec& new_time, clockid_t new_clockbase, + const timespec& old_time, clockid_t old_clockbase) { + // get reference clocks + timespec new_clock; + clock_gettime(new_clockbase, &new_clock); + timespec old_clock; + clock_gettime(old_clockbase, &old_clock); - timespec cur_monotonic_time; - clock_gettime(CLOCK_MONOTONIC, &cur_monotonic_time); - timespec cur_realtime_time; - clock_gettime(CLOCK_REALTIME, &cur_realtime_time); + // compute new time by moving old delta to the new clock. + new_time.tv_sec = old_time.tv_sec - old_clock.tv_sec + new_clock.tv_sec; + new_time.tv_nsec = old_time.tv_nsec - old_clock.tv_nsec + new_clock.tv_nsec; - monotonic_time.tv_nsec -= cur_realtime_time.tv_nsec; - monotonic_time.tv_nsec += cur_monotonic_time.tv_nsec; - if (monotonic_time.tv_nsec >= NS_PER_S) { - monotonic_time.tv_nsec -= NS_PER_S; - monotonic_time.tv_sec += 1; - } else if (monotonic_time.tv_nsec < 0) { - monotonic_time.tv_nsec += NS_PER_S; - monotonic_time.tv_sec -= 1; + // correct nsec to second wrap. + if (new_time.tv_nsec >= NS_PER_S) { + new_time.tv_nsec -= NS_PER_S; + new_time.tv_sec += 1; + } else if (new_time.tv_nsec < 0) { + new_time.tv_nsec += NS_PER_S; + new_time.tv_sec -= 1; } - monotonic_time.tv_sec -= cur_realtime_time.tv_sec; - monotonic_time.tv_sec += cur_monotonic_time.tv_sec; +} + +void monotonic_time_from_realtime_time(timespec& monotonic_time, const timespec& realtime_time) { + convert_timespec_clocks(monotonic_time, CLOCK_MONOTONIC, realtime_time, CLOCK_REALTIME); +} + +void realtime_time_from_monotonic_time(timespec& realtime_time, const timespec& monotonic_time) { + convert_timespec_clocks(realtime_time, CLOCK_REALTIME, monotonic_time, CLOCK_MONOTONIC); } diff --git a/libc/private/bionic_constants.h b/libc/private/bionic_constants.h index 05914f400..6274fe284 100644 --- a/libc/private/bionic_constants.h +++ b/libc/private/bionic_constants.h @@ -16,7 +16,7 @@ #pragma once -#define NS_PER_S 1000000000 +#define NS_PER_S 1'000'000'000LL // Size of the shadow call stack. This can be small because these stacks only // contain return addresses. This must be a power of 2 so the mask trick works. diff --git a/libc/private/bionic_time_conversions.h b/libc/private/bionic_time_conversions.h index fb049f238..c6b3c7825 100644 --- a/libc/private/bionic_time_conversions.h +++ b/libc/private/bionic_time_conversions.h @@ -45,6 +45,9 @@ __LIBC_HIDDEN__ void timeval_from_timespec(timeval& tv, const timespec& ts); __LIBC_HIDDEN__ void monotonic_time_from_realtime_time(timespec& monotonic_time, const timespec& realtime_time); +__LIBC_HIDDEN__ void realtime_time_from_monotonic_time(timespec& realtime_time, + const timespec& monotonic_time); + __END_DECLS static inline int check_timespec(const timespec* ts, bool null_allowed) { diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index 749d687f8..1a882beb5 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -2398,6 +2398,20 @@ static void pthread_mutex_timedlock_helper(clockid_t clock, ts.tv_sec = -1; ASSERT_EQ(ETIMEDOUT, lock_function(&m, &ts)); + // check we wait long enough for the lock. + ASSERT_EQ(0, clock_gettime(clock, &ts)); + const int64_t start_ns = ts.tv_sec * NS_PER_S + ts.tv_nsec; + + // add a second to get deadline. + ts.tv_sec += 1; + + ASSERT_EQ(ETIMEDOUT, lock_function(&m, &ts)); + + // The timedlock must have waited at least 1 second before returning. + clock_gettime(clock, &ts); + const int64_t end_ns = ts.tv_sec * NS_PER_S + ts.tv_nsec; + ASSERT_GT(end_ns - start_ns, NS_PER_S); + // If the mutex is unlocked, pthread_mutex_timedlock should succeed. ASSERT_EQ(0, pthread_mutex_unlock(&m)); @@ -2443,7 +2457,11 @@ static void pthread_mutex_timedlock_pi_helper(clockid_t clock, timespec ts; clock_gettime(clock, &ts); + const int64_t start_ns = ts.tv_sec * NS_PER_S + ts.tv_nsec; + + // add a second to get deadline. ts.tv_sec += 1; + ASSERT_EQ(0, lock_function(&m.lock, &ts)); struct ThreadArgs { @@ -2472,6 +2490,12 @@ static void pthread_mutex_timedlock_pi_helper(clockid_t clock, void* result; ASSERT_EQ(0, pthread_join(thread, &result)); ASSERT_EQ(ETIMEDOUT, reinterpret_cast(result)); + + // The timedlock must have waited at least 1 second before returning. + clock_gettime(clock, &ts); + const int64_t end_ns = ts.tv_sec * NS_PER_S + ts.tv_nsec; + ASSERT_GT(end_ns - start_ns, NS_PER_S); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); }