Use FUTEX_WAIT_BITSET to avoid converting timeouts.
Add unittests for pthread APIs with timeout parameter.
Bug: 17569991
Change-Id: I6b3b9b2feae03680654cd64c3112ce7644632c87
diff --git a/libc/bionic/__cxa_guard.cpp b/libc/bionic/__cxa_guard.cpp
index 5b34b58..97284d5 100644
--- a/libc/bionic/__cxa_guard.cpp
+++ b/libc/bionic/__cxa_guard.cpp
@@ -109,7 +109,7 @@
}
}
- __futex_wait_ex(&gv->state, false, CONSTRUCTION_UNDERWAY_WITH_WAITER, NULL);
+ __futex_wait_ex(&gv->state, false, CONSTRUCTION_UNDERWAY_WITH_WAITER, false, nullptr);
old_value = atomic_load_explicit(&gv->state, memory_order_relaxed);
}
}
diff --git a/libc/bionic/bionic_time_conversions.cpp b/libc/bionic/bionic_time_conversions.cpp
index 75e8d49..f3ca46a 100644
--- a/libc/bionic/bionic_time_conversions.cpp
+++ b/libc/bionic/bionic_time_conversions.cpp
@@ -52,18 +52,12 @@
tv.tv_usec = ts.tv_nsec / 1000;
}
-// Initializes 'ts' with the difference between 'abs_ts' and the current time
-// according to 'clock'. Returns false if abstime already expired, true otherwise.
-bool timespec_from_absolute_timespec(timespec& ts, const timespec& abs_ts, clockid_t clock) {
- clock_gettime(clock, &ts);
- ts.tv_sec = abs_ts.tv_sec - ts.tv_sec;
- ts.tv_nsec = abs_ts.tv_nsec - ts.tv_nsec;
- if (ts.tv_nsec < 0) {
- ts.tv_sec--;
- ts.tv_nsec += NS_PER_S;
+void absolute_timespec_from_timespec(timespec& abs_ts, const timespec& ts, clockid_t clock) {
+ clock_gettime(clock, &abs_ts);
+ abs_ts.tv_sec += ts.tv_sec;
+ abs_ts.tv_nsec += ts.tv_nsec;
+ if (abs_ts.tv_nsec >= NS_PER_S) {
+ abs_ts.tv_nsec -= NS_PER_S;
+ abs_ts.tv_sec++;
}
- if (ts.tv_nsec < 0 || ts.tv_sec < 0) {
- return false;
- }
- return true;
}
diff --git a/libc/bionic/pthread_barrier.cpp b/libc/bionic/pthread_barrier.cpp
index 3227daf..1bcd12a 100644
--- a/libc/bionic/pthread_barrier.cpp
+++ b/libc/bionic/pthread_barrier.cpp
@@ -118,7 +118,7 @@
// threads have left the barrier. Use acquire operation here to synchronize with
// the last thread leaving the previous cycle, so we can read correct wait_count below.
while(atomic_load_explicit(&barrier->state, memory_order_acquire) == RELEASE) {
- __futex_wait_ex(&barrier->state, barrier->pshared, RELEASE, nullptr);
+ __futex_wait_ex(&barrier->state, barrier->pshared, RELEASE, false, nullptr);
}
uint32_t prev_wait_count = atomic_load_explicit(&barrier->wait_count, memory_order_relaxed);
@@ -152,7 +152,7 @@
// Use acquire operation here to synchronize between the last thread entering the
// barrier with all threads leaving the barrier.
while (atomic_load_explicit(&barrier->state, memory_order_acquire) == WAIT) {
- __futex_wait_ex(&barrier->state, barrier->pshared, WAIT, nullptr);
+ __futex_wait_ex(&barrier->state, barrier->pshared, WAIT, false, nullptr);
}
}
// Use release operation here to make it not reordered with previous operations.
@@ -173,7 +173,7 @@
// Use acquire operation here to synchronize with the last thread leaving the barrier.
// So we can read correct wait_count below.
while (atomic_load_explicit(&barrier->state, memory_order_acquire) == RELEASE) {
- __futex_wait_ex(&barrier->state, barrier->pshared, RELEASE, nullptr);
+ __futex_wait_ex(&barrier->state, barrier->pshared, RELEASE, false, nullptr);
}
if (atomic_load_explicit(&barrier->wait_count, memory_order_relaxed) != 0) {
return EBUSY;
diff --git a/libc/bionic/pthread_cond.cpp b/libc/bionic/pthread_cond.cpp
index 4a69da5..adbce07 100644
--- a/libc/bionic/pthread_cond.cpp
+++ b/libc/bionic/pthread_cond.cpp
@@ -111,8 +111,8 @@
return COND_IS_SHARED(atomic_load_explicit(&state, memory_order_relaxed));
}
- int get_clock() {
- return COND_GET_CLOCK(atomic_load_explicit(&state, memory_order_relaxed));
+ bool use_realtime_clock() {
+ return COND_GET_CLOCK(atomic_load_explicit(&state, memory_order_relaxed)) == CLOCK_REALTIME;
}
#if defined(__LP64__)
@@ -170,12 +170,17 @@
return 0;
}
-static int __pthread_cond_timedwait_relative(pthread_cond_internal_t* cond, pthread_mutex_t* mutex,
- const timespec* rel_timeout_or_null) {
- unsigned int old_state = atomic_load_explicit(&cond->state, memory_order_relaxed);
+static int __pthread_cond_timedwait(pthread_cond_internal_t* cond, pthread_mutex_t* mutex,
+ bool use_realtime_clock, const timespec* abs_timeout_or_null) {
+ int result = check_timespec(abs_timeout_or_null);
+ if (result != 0) {
+ return result;
+ }
+ unsigned int old_state = atomic_load_explicit(&cond->state, memory_order_relaxed);
pthread_mutex_unlock(mutex);
- int status = __futex_wait_ex(&cond->state, cond->process_shared(), old_state, rel_timeout_or_null);
+ int status = __futex_wait_ex(&cond->state, cond->process_shared(), old_state,
+ use_realtime_clock, abs_timeout_or_null);
pthread_mutex_lock(mutex);
if (status == -ETIMEDOUT) {
@@ -184,21 +189,6 @@
return 0;
}
-static int __pthread_cond_timedwait(pthread_cond_internal_t* cond, pthread_mutex_t* mutex,
- const timespec* abs_timeout_or_null, clockid_t clock) {
- timespec ts;
- timespec* rel_timeout = NULL;
-
- if (abs_timeout_or_null != NULL) {
- rel_timeout = &ts;
- if (!timespec_from_absolute_timespec(*rel_timeout, *abs_timeout_or_null, clock)) {
- return ETIMEDOUT;
- }
- }
-
- return __pthread_cond_timedwait_relative(cond, mutex, rel_timeout);
-}
-
int pthread_cond_broadcast(pthread_cond_t* cond_interface) {
return __pthread_cond_pulse(__get_internal_cond(cond_interface), INT_MAX);
}
@@ -209,14 +199,14 @@
int pthread_cond_wait(pthread_cond_t* cond_interface, pthread_mutex_t* mutex) {
pthread_cond_internal_t* cond = __get_internal_cond(cond_interface);
- return __pthread_cond_timedwait(cond, mutex, NULL, cond->get_clock());
+ return __pthread_cond_timedwait(cond, mutex, false, nullptr);
}
int pthread_cond_timedwait(pthread_cond_t *cond_interface, pthread_mutex_t * mutex,
const timespec *abstime) {
pthread_cond_internal_t* cond = __get_internal_cond(cond_interface);
- return __pthread_cond_timedwait(cond, mutex, abstime, cond->get_clock());
+ return __pthread_cond_timedwait(cond, mutex, cond->use_realtime_clock(), abstime);
}
#if !defined(__LP64__)
@@ -225,8 +215,7 @@
pthread_mutex_t* mutex,
const timespec* abs_timeout) {
- return __pthread_cond_timedwait(__get_internal_cond(cond_interface), mutex, abs_timeout,
- CLOCK_MONOTONIC);
+ return __pthread_cond_timedwait(__get_internal_cond(cond_interface), mutex, false, abs_timeout);
}
extern "C" int pthread_cond_timedwait_monotonic_np(pthread_cond_t* cond_interface,
@@ -238,8 +227,13 @@
extern "C" int pthread_cond_timedwait_relative_np(pthread_cond_t* cond_interface,
pthread_mutex_t* mutex,
const timespec* rel_timeout) {
-
- return __pthread_cond_timedwait_relative(__get_internal_cond(cond_interface), mutex, rel_timeout);
+ timespec ts;
+ timespec* abs_timeout = nullptr;
+ if (rel_timeout != nullptr) {
+ absolute_timespec_from_timespec(ts, *rel_timeout, CLOCK_REALTIME);
+ abs_timeout = &ts;
+ }
+ return __pthread_cond_timedwait(__get_internal_cond(cond_interface), mutex, true, abs_timeout);
}
extern "C" int pthread_cond_timeout_np(pthread_cond_t* cond_interface,
diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp
index 851fc3d..23dc3b0 100644
--- a/libc/bionic/pthread_mutex.cpp
+++ b/libc/bionic/pthread_mutex.cpp
@@ -296,11 +296,15 @@
*/
static inline __always_inline int __pthread_normal_mutex_lock(pthread_mutex_internal_t* mutex,
uint16_t shared,
- const timespec* abs_timeout_or_null,
- clockid_t clock) {
+ bool use_realtime_clock,
+ const timespec* abs_timeout_or_null) {
if (__predict_true(__pthread_normal_mutex_trylock(mutex, shared) == 0)) {
return 0;
}
+ int result = check_timespec(abs_timeout_or_null);
+ if (result != 0) {
+ return result;
+ }
ScopedTrace trace("Contending for pthread mutex");
@@ -317,15 +321,8 @@
// made by other threads visible to the current CPU.
while (atomic_exchange_explicit(&mutex->state, locked_contended,
memory_order_acquire) != unlocked) {
- timespec ts;
- timespec* rel_timeout = NULL;
- if (abs_timeout_or_null != NULL) {
- rel_timeout = &ts;
- if (!timespec_from_absolute_timespec(*rel_timeout, *abs_timeout_or_null, clock)) {
- return ETIMEDOUT;
- }
- }
- if (__futex_wait_ex(&mutex->state, shared, locked_contended, rel_timeout) == -ETIMEDOUT) {
+ if (__futex_wait_ex(&mutex->state, shared, locked_contended, use_realtime_clock,
+ abs_timeout_or_null) == -ETIMEDOUT) {
return ETIMEDOUT;
}
}
@@ -396,14 +393,15 @@
pthread_mutex_internal_t* mutex,
uint16_t shared,
uint16_t old_state,
- const timespec* rel_timeout) {
+ bool use_realtime_clock,
+ const timespec* abs_timeout) {
// __futex_wait always waits on a 32-bit value. But state is 16-bit. For a normal mutex, the owner_tid
// field in mutex is not used. On 64-bit devices, the __pad field in mutex is not used.
// But when a recursive or errorcheck mutex is used on 32-bit devices, we need to add the
// owner_tid value in the value argument for __futex_wait, otherwise we may always get EAGAIN error.
#if defined(__LP64__)
- return __futex_wait_ex(&mutex->state, shared, old_state, rel_timeout);
+ return __futex_wait_ex(&mutex->state, shared, old_state, use_realtime_clock, abs_timeout);
#else
// This implementation works only when the layout of pthread_mutex_internal_t matches below expectation.
@@ -412,19 +410,21 @@
static_assert(offsetof(pthread_mutex_internal_t, owner_tid) == 2, "");
uint32_t owner_tid = atomic_load_explicit(&mutex->owner_tid, memory_order_relaxed);
- return __futex_wait_ex(&mutex->state, shared, (owner_tid << 16) | old_state, rel_timeout);
+ return __futex_wait_ex(&mutex->state, shared, (owner_tid << 16) | old_state,
+ use_realtime_clock, abs_timeout);
#endif
}
static int __pthread_mutex_lock_with_timeout(pthread_mutex_internal_t* mutex,
- const timespec* abs_timeout_or_null, clockid_t clock) {
+ bool use_realtime_clock,
+ const timespec* abs_timeout_or_null) {
uint16_t old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed);
uint16_t mtype = (old_state & MUTEX_TYPE_MASK);
uint16_t shared = (old_state & MUTEX_SHARED_MASK);
// Handle common case first.
if ( __predict_true(mtype == MUTEX_TYPE_BITS_NORMAL) ) {
- return __pthread_normal_mutex_lock(mutex, shared, abs_timeout_or_null, clock);
+ return __pthread_normal_mutex_lock(mutex, shared, use_realtime_clock, abs_timeout_or_null);
}
// Do we already own this recursive or error-check mutex?
@@ -484,16 +484,13 @@
old_state = new_state;
}
- // We are in locked_contended state, sleep until someone wakes us up.
- timespec ts;
- timespec* rel_timeout = NULL;
- if (abs_timeout_or_null != NULL) {
- rel_timeout = &ts;
- if (!timespec_from_absolute_timespec(*rel_timeout, *abs_timeout_or_null, clock)) {
- return ETIMEDOUT;
- }
+ int result = check_timespec(abs_timeout_or_null);
+ if (result != 0) {
+ return result;
}
- if (__recursive_or_errorcheck_mutex_wait(mutex, shared, old_state, rel_timeout) == -ETIMEDOUT) {
+ // We are in locked_contended state, sleep until someone wakes us up.
+ if (__recursive_or_errorcheck_mutex_wait(mutex, shared, old_state, use_realtime_clock,
+ abs_timeout_or_null) == -ETIMEDOUT) {
return ETIMEDOUT;
}
old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed);
@@ -518,7 +515,7 @@
return 0;
}
}
- return __pthread_mutex_lock_with_timeout(mutex, NULL, 0);
+ return __pthread_mutex_lock_with_timeout(mutex, false, nullptr);
}
int pthread_mutex_unlock(pthread_mutex_t* mutex_interface) {
@@ -613,17 +610,12 @@
#if !defined(__LP64__)
extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex_interface, unsigned ms) {
+ timespec ts;
+ timespec_from_ms(ts, ms);
timespec abs_timeout;
- clock_gettime(CLOCK_MONOTONIC, &abs_timeout);
- abs_timeout.tv_sec += ms / 1000;
- abs_timeout.tv_nsec += (ms % 1000) * 1000000;
- if (abs_timeout.tv_nsec >= NS_PER_S) {
- abs_timeout.tv_sec++;
- abs_timeout.tv_nsec -= NS_PER_S;
- }
-
+ absolute_timespec_from_timespec(abs_timeout, ts, CLOCK_MONOTONIC);
int error = __pthread_mutex_lock_with_timeout(__get_internal_mutex(mutex_interface),
- &abs_timeout, CLOCK_MONOTONIC);
+ false, &abs_timeout);
if (error == ETIMEDOUT) {
error = EBUSY;
}
@@ -633,7 +625,7 @@
int pthread_mutex_timedlock(pthread_mutex_t* mutex_interface, const timespec* abs_timeout) {
return __pthread_mutex_lock_with_timeout(__get_internal_mutex(mutex_interface),
- abs_timeout, CLOCK_REALTIME);
+ true, abs_timeout);
}
int pthread_mutex_destroy(pthread_mutex_t* mutex_interface) {
diff --git a/libc/bionic/pthread_once.cpp b/libc/bionic/pthread_once.cpp
index 7688a23..f48eadc 100644
--- a/libc/bionic/pthread_once.cpp
+++ b/libc/bionic/pthread_once.cpp
@@ -79,7 +79,7 @@
}
// The initialization is underway, wait for its finish.
- __futex_wait_ex(once_control_ptr, 0, old_value, NULL);
+ __futex_wait_ex(once_control_ptr, 0, old_value, false, nullptr);
old_value = atomic_load_explicit(once_control_ptr, memory_order_acquire);
}
}
diff --git a/libc/bionic/pthread_rwlock.cpp b/libc/bionic/pthread_rwlock.cpp
index 934210e..b1c48c8 100644
--- a/libc/bionic/pthread_rwlock.cpp
+++ b/libc/bionic/pthread_rwlock.cpp
@@ -294,9 +294,13 @@
}
while (true) {
- int ret = __pthread_rwlock_tryrdlock(rwlock);
- if (ret == 0 || ret == EAGAIN) {
- return ret;
+ int result = __pthread_rwlock_tryrdlock(rwlock);
+ if (result == 0 || result == EAGAIN) {
+ return result;
+ }
+ result = check_timespec(abs_timeout_or_null);
+ if (result != 0) {
+ return result;
}
int old_state = atomic_load_explicit(&rwlock->state, memory_order_relaxed);
@@ -304,16 +308,6 @@
continue;
}
- timespec ts;
- timespec* rel_timeout = NULL;
-
- if (abs_timeout_or_null != NULL) {
- rel_timeout = &ts;
- if (!timespec_from_absolute_timespec(*rel_timeout, *abs_timeout_or_null, CLOCK_REALTIME)) {
- return ETIMEDOUT;
- }
- }
-
rwlock->pending_lock.lock();
rwlock->pending_reader_count++;
@@ -327,10 +321,10 @@
int old_serial = rwlock->pending_reader_wakeup_serial;
rwlock->pending_lock.unlock();
- int futex_ret = 0;
+ int futex_result = 0;
if (!__can_acquire_read_lock(old_state, rwlock->writer_nonrecursive_preferred)) {
- futex_ret = __futex_wait_ex(&rwlock->pending_reader_wakeup_serial, rwlock->pshared,
- old_serial, rel_timeout);
+ futex_result = __futex_wait_ex(&rwlock->pending_reader_wakeup_serial, rwlock->pshared,
+ old_serial, true, abs_timeout_or_null);
}
rwlock->pending_lock.lock();
@@ -341,7 +335,7 @@
}
rwlock->pending_lock.unlock();
- if (futex_ret == -ETIMEDOUT) {
+ if (futex_result == -ETIMEDOUT) {
return ETIMEDOUT;
}
}
@@ -372,9 +366,13 @@
return EDEADLK;
}
while (true) {
- int ret = __pthread_rwlock_trywrlock(rwlock);
- if (ret == 0) {
- return ret;
+ int result = __pthread_rwlock_trywrlock(rwlock);
+ if (result == 0) {
+ return result;
+ }
+ result = check_timespec(abs_timeout_or_null);
+ if (result != 0) {
+ return result;
}
int old_state = atomic_load_explicit(&rwlock->state, memory_order_relaxed);
@@ -382,16 +380,6 @@
continue;
}
- timespec ts;
- timespec* rel_timeout = NULL;
-
- if (abs_timeout_or_null != NULL) {
- rel_timeout = &ts;
- if (!timespec_from_absolute_timespec(*rel_timeout, *abs_timeout_or_null, CLOCK_REALTIME)) {
- return ETIMEDOUT;
- }
- }
-
rwlock->pending_lock.lock();
rwlock->pending_writer_count++;
@@ -401,10 +389,10 @@
int old_serial = rwlock->pending_writer_wakeup_serial;
rwlock->pending_lock.unlock();
- int futex_ret = 0;
+ int futex_result = 0;
if (!__can_acquire_write_lock(old_state)) {
- futex_ret = __futex_wait_ex(&rwlock->pending_writer_wakeup_serial, rwlock->pshared,
- old_serial, rel_timeout);
+ futex_result = __futex_wait_ex(&rwlock->pending_writer_wakeup_serial, rwlock->pshared,
+ old_serial, true, abs_timeout_or_null);
}
rwlock->pending_lock.lock();
@@ -415,7 +403,7 @@
}
rwlock->pending_lock.unlock();
- if (futex_ret == -ETIMEDOUT) {
+ if (futex_result == -ETIMEDOUT) {
return ETIMEDOUT;
}
}
@@ -427,7 +415,7 @@
if (__predict_true(__pthread_rwlock_tryrdlock(rwlock) == 0)) {
return 0;
}
- return __pthread_rwlock_timedrdlock(rwlock, NULL);
+ return __pthread_rwlock_timedrdlock(rwlock, nullptr);
}
int pthread_rwlock_timedrdlock(pthread_rwlock_t* rwlock_interface, const timespec* abs_timeout) {
@@ -446,7 +434,7 @@
if (__predict_true(__pthread_rwlock_trywrlock(rwlock) == 0)) {
return 0;
}
- return __pthread_rwlock_timedwrlock(rwlock, NULL);
+ return __pthread_rwlock_timedwrlock(rwlock, nullptr);
}
int pthread_rwlock_timedwrlock(pthread_rwlock_t* rwlock_interface, const timespec* abs_timeout) {
diff --git a/libc/bionic/semaphore.cpp b/libc/bionic/semaphore.cpp
index ff84443..79b5d63 100644
--- a/libc/bionic/semaphore.cpp
+++ b/libc/bionic/semaphore.cpp
@@ -220,7 +220,7 @@
return 0;
}
- __futex_wait_ex(sem_count_ptr, shared, shared | SEMCOUNT_MINUS_ONE, NULL);
+ __futex_wait_ex(sem_count_ptr, shared, shared | SEMCOUNT_MINUS_ONE, false, nullptr);
}
}
@@ -235,36 +235,29 @@
}
// Check it as per POSIX.
- if (abs_timeout == NULL || abs_timeout->tv_sec < 0 || abs_timeout->tv_nsec < 0 || abs_timeout->tv_nsec >= NS_PER_S) {
- errno = EINVAL;
+ int result = check_timespec(abs_timeout);
+ if (result != 0) {
+ errno = result;
return -1;
}
unsigned int shared = SEM_GET_SHARED(sem_count_ptr);
while (true) {
- // POSIX mandates CLOCK_REALTIME here.
- timespec ts;
- if (!timespec_from_absolute_timespec(ts, *abs_timeout, CLOCK_REALTIME)) {
- errno = ETIMEDOUT;
- return -1;
- }
-
// Try to grab the semaphore. If the value was 0, this will also change it to -1.
if (__sem_dec(sem_count_ptr) > 0) {
- break;
+ return 0;
}
// Contention detected. Wait for a wakeup event.
- int ret = __futex_wait_ex(sem_count_ptr, shared, shared | SEMCOUNT_MINUS_ONE, &ts);
+ int result = __futex_wait_ex(sem_count_ptr, shared, shared | SEMCOUNT_MINUS_ONE, true, abs_timeout);
// Return in case of timeout or interrupt.
- if (ret == -ETIMEDOUT || ret == -EINTR) {
- errno = -ret;
+ if (result == -ETIMEDOUT || result == -EINTR) {
+ errno = -result;
return -1;
}
}
- return 0;
}
int sem_post(sem_t* sem) {