From 0e714a5b41451e84c5ded93a42c9a4b0a9440691 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 3 Mar 2014 16:42:47 -0800 Subject: [PATCH] Implement POSIX pthread_mutex_timedlock. This replaces the non-standard pthread_mutex_lock_timeout_np, which we have to keep around on LP32 for binary compatibility. Change-Id: I098dc7cd38369f0c1bec1fac35687fbd27392e00 --- libc/bionic/pthread_cond.cpp | 2 +- libc/bionic/pthread_internal.h | 2 +- libc/bionic/pthread_internals.cpp | 2 +- libc/bionic/pthread_mutex.cpp | 227 +++++++++++++++--------------- libc/include/pthread.h | 22 +-- tests/pthread_test.cpp | 23 +++ 6 files changed, 141 insertions(+), 137 deletions(-) diff --git a/libc/bionic/pthread_cond.cpp b/libc/bionic/pthread_cond.cpp index e67afbafb..e570602be 100644 --- a/libc/bionic/pthread_cond.cpp +++ b/libc/bionic/pthread_cond.cpp @@ -185,7 +185,7 @@ int __pthread_cond_timedwait(pthread_cond_t* cond, pthread_mutex_t* mutex, const timespec* tsp; if (abstime != NULL) { - if (__timespec_to_absolute(&ts, abstime, clock) < 0) { + if (__timespec_from_absolute(&ts, abstime, clock) < 0) { return ETIMEDOUT; } tsp = &ts; diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 31ed07c21..3825a4c5c 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -89,7 +89,7 @@ __LIBC_HIDDEN__ void _pthread_internal_remove_locked(pthread_internal_t* thread) __LIBC_HIDDEN__ extern pthread_internal_t* gThreadList; __LIBC_HIDDEN__ extern pthread_mutex_t gThreadListLock; -__LIBC_HIDDEN__ int __timespec_to_absolute(timespec*, const timespec*, clockid_t); +__LIBC_HIDDEN__ int __timespec_from_absolute(timespec*, const timespec*, clockid_t); /* needed by fork.c */ __LIBC_HIDDEN__ extern void __timer_table_start_stop(int); diff --git a/libc/bionic/pthread_internals.cpp b/libc/bionic/pthread_internals.cpp index 09c48dcd3..d4d609946 100644 --- a/libc/bionic/pthread_internals.cpp +++ b/libc/bionic/pthread_internals.cpp @@ -75,7 +75,7 @@ pid_t __pthread_gettid(pthread_t t) { // Initialize 'ts' with the difference between 'abstime' and the current time // according to 'clock'. Returns -1 if abstime already expired, or 0 otherwise. -int __timespec_to_absolute(timespec* ts, const timespec* abstime, clockid_t clock) { +int __timespec_from_absolute(timespec* ts, const timespec* abstime, clockid_t clock) { clock_gettime(clock, ts); ts->tv_sec = abstime->tv_sec - ts->tv_sec; ts->tv_nsec = abstime->tv_nsec - ts->tv_nsec; diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp index 0d992b365..90726052e 100644 --- a/libc/bionic/pthread_mutex.cpp +++ b/libc/bionic/pthread_mutex.cpp @@ -667,140 +667,133 @@ int pthread_mutex_trylock(pthread_mutex_t *mutex) return err; } -/* initialize 'abstime' to the current time according to 'clock' plus 'msecs' - * milliseconds. - */ -static void __timespec_to_relative_msec(timespec* abstime, unsigned msecs, clockid_t clock) { - clock_gettime(clock, abstime); - abstime->tv_sec += msecs/1000; - abstime->tv_nsec += (msecs%1000)*1000000; - if (abstime->tv_nsec >= 1000000000) { - abstime->tv_sec++; - abstime->tv_nsec -= 1000000000; +static int __pthread_mutex_timedlock(pthread_mutex_t* mutex, const timespec* abs_timeout, clockid_t clock) { + timespec ts; + + int mvalue = mutex->value; + int mtype = (mvalue & MUTEX_TYPE_MASK); + int shared = (mvalue & MUTEX_SHARED_MASK); + + // Handle common case first. + if (__predict_true(mtype == MUTEX_TYPE_BITS_NORMAL)) { + const int unlocked = shared | MUTEX_STATE_BITS_UNLOCKED; + const int locked_uncontended = shared | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; + const int locked_contended = shared | MUTEX_STATE_BITS_LOCKED_CONTENDED; + + // Fast path for uncontended lock. Note: MUTEX_TYPE_BITS_NORMAL is 0. + if (__bionic_cmpxchg(unlocked, locked_uncontended, &mutex->value) == 0) { + ANDROID_MEMBAR_FULL(); + return 0; } -} -__LIBC_HIDDEN__ -int pthread_mutex_lock_timeout_np_impl(pthread_mutex_t *mutex, unsigned msecs) -{ - clockid_t clock = CLOCK_MONOTONIC; - timespec abstime; - timespec ts; - int mvalue, mtype, tid, shared; + // Loop while needed. + while (__bionic_swap(locked_contended, &mutex->value) != unlocked) { + if (__timespec_from_absolute(&ts, abs_timeout, clock) < 0) { + return ETIMEDOUT; + } + __futex_wait_ex(&mutex->value, shared, locked_contended, &ts); + } + ANDROID_MEMBAR_FULL(); + return 0; + } - /* compute absolute expiration time */ - __timespec_to_relative_msec(&abstime, msecs, clock); + // Do we already own this recursive or error-check mutex? + pid_t tid = __get_thread()->tid; + if (tid == MUTEX_OWNER_FROM_BITS(mvalue)) { + return _recursive_increment(mutex, mvalue, mtype); + } + // The following implements the same loop as pthread_mutex_lock_impl + // but adds checks to ensure that the operation never exceeds the + // absolute expiration time. + mtype |= shared; + + // First try a quick lock. + if (mvalue == mtype) { + mvalue = MUTEX_OWNER_TO_BITS(tid) | mtype | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; + if (__predict_true(__bionic_cmpxchg(mtype, mvalue, &mutex->value) == 0)) { + ANDROID_MEMBAR_FULL(); + return 0; + } mvalue = mutex->value; - mtype = (mvalue & MUTEX_TYPE_MASK); - shared = (mvalue & MUTEX_SHARED_MASK); + } - /* Handle common case first */ - if ( __predict_true(mtype == MUTEX_TYPE_BITS_NORMAL) ) - { - const int unlocked = shared | MUTEX_STATE_BITS_UNLOCKED; - const int locked_uncontended = shared | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; - const int locked_contended = shared | MUTEX_STATE_BITS_LOCKED_CONTENDED; - - /* fast path for uncontended lock. Note: MUTEX_TYPE_BITS_NORMAL is 0 */ - if (__bionic_cmpxchg(unlocked, locked_uncontended, &mutex->value) == 0) { - ANDROID_MEMBAR_FULL(); - return 0; - } - - /* loop while needed */ - while (__bionic_swap(locked_contended, &mutex->value) != unlocked) { - if (__timespec_to_absolute(&ts, &abstime, clock) < 0) - return EBUSY; - - __futex_wait_ex(&mutex->value, shared, locked_contended, &ts); - } + while (true) { + // If the value is 'unlocked', try to acquire it directly. + // NOTE: put state to 2 since we know there is contention. + if (mvalue == mtype) { // Unlocked. + mvalue = MUTEX_OWNER_TO_BITS(tid) | mtype | MUTEX_STATE_BITS_LOCKED_CONTENDED; + if (__bionic_cmpxchg(mtype, mvalue, &mutex->value) == 0) { ANDROID_MEMBAR_FULL(); return 0; + } + // The value changed before we could lock it. We need to check + // the time to avoid livelocks, reload the value, then loop again. + if (__timespec_from_absolute(&ts, abs_timeout, clock) < 0) { + return ETIMEDOUT; + } + + mvalue = mutex->value; + continue; } - /* Do we already own this recursive or error-check mutex ? */ - tid = __get_thread()->tid; - if ( tid == MUTEX_OWNER_FROM_BITS(mvalue) ) - return _recursive_increment(mutex, mvalue, mtype); - - /* the following implements the same loop than pthread_mutex_lock_impl - * but adds checks to ensure that the operation never exceeds the - * absolute expiration time. - */ - mtype |= shared; - - /* first try a quick lock */ - if (mvalue == mtype) { - mvalue = MUTEX_OWNER_TO_BITS(tid) | mtype | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; - if (__predict_true(__bionic_cmpxchg(mtype, mvalue, &mutex->value) == 0)) { - ANDROID_MEMBAR_FULL(); - return 0; - } + // The value is locked. If 'uncontended', try to switch its state + // to 'contented' to ensure we get woken up later. + if (MUTEX_STATE_BITS_IS_LOCKED_UNCONTENDED(mvalue)) { + int newval = MUTEX_STATE_BITS_FLIP_CONTENTION(mvalue); + if (__bionic_cmpxchg(mvalue, newval, &mutex->value) != 0) { + // This failed because the value changed, reload it. mvalue = mutex->value; + } else { + // This succeeded, update mvalue. + mvalue = newval; + } } - for (;;) { - timespec ts; - - /* if the value is 'unlocked', try to acquire it directly */ - /* NOTE: put state to 2 since we know there is contention */ - if (mvalue == mtype) /* unlocked */ { - mvalue = MUTEX_OWNER_TO_BITS(tid) | mtype | MUTEX_STATE_BITS_LOCKED_CONTENDED; - if (__bionic_cmpxchg(mtype, mvalue, &mutex->value) == 0) { - ANDROID_MEMBAR_FULL(); - return 0; - } - /* the value changed before we could lock it. We need to check - * the time to avoid livelocks, reload the value, then loop again. */ - if (__timespec_to_absolute(&ts, &abstime, clock) < 0) - return EBUSY; - - mvalue = mutex->value; - continue; - } - - /* The value is locked. If 'uncontended', try to switch its state - * to 'contented' to ensure we get woken up later. */ - if (MUTEX_STATE_BITS_IS_LOCKED_UNCONTENDED(mvalue)) { - int newval = MUTEX_STATE_BITS_FLIP_CONTENTION(mvalue); - if (__bionic_cmpxchg(mvalue, newval, &mutex->value) != 0) { - /* this failed because the value changed, reload it */ - mvalue = mutex->value; - } else { - /* this succeeded, update mvalue */ - mvalue = newval; - } - } - - /* check time and update 'ts' */ - if (__timespec_to_absolute(&ts, &abstime, clock) < 0) - return EBUSY; - - /* Only wait to be woken up if the state is '2', otherwise we'll - * simply loop right now. This can happen when the second cmpxchg - * in our loop failed because the mutex was unlocked by another - * thread. - */ - if (MUTEX_STATE_BITS_IS_LOCKED_CONTENDED(mvalue)) { - if (__futex_wait_ex(&mutex->value, shared, mvalue, &ts) == -ETIMEDOUT) { - return EBUSY; - } - mvalue = mutex->value; - } + // Check time and update 'ts'. + if (__timespec_from_absolute(&ts, abs_timeout, clock) < 0) { + return ETIMEDOUT; } - /* NOTREACHED */ + + // Only wait to be woken up if the state is '2', otherwise we'll + // simply loop right now. This can happen when the second cmpxchg + // in our loop failed because the mutex was unlocked by another thread. + if (MUTEX_STATE_BITS_IS_LOCKED_CONTENDED(mvalue)) { + if (__futex_wait_ex(&mutex->value, shared, mvalue, &ts) == -ETIMEDOUT) { + return ETIMEDOUT; + } + mvalue = mutex->value; + } + } + /* NOTREACHED */ } -int pthread_mutex_lock_timeout_np(pthread_mutex_t *mutex, unsigned msecs) -{ - int err = pthread_mutex_lock_timeout_np_impl(mutex, msecs); - if (PTHREAD_DEBUG_ENABLED) { - if (!err) { - pthread_debug_mutex_lock_check(mutex); - } +#if !defined(__LP64__) +extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex, unsigned 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 >= 1000000000) { + abs_timeout.tv_sec++; + abs_timeout.tv_nsec -= 1000000000; + } + + int err = __pthread_mutex_timedlock(mutex, &abs_timeout, CLOCK_MONOTONIC); + if (err == ETIMEDOUT) { + err = EBUSY; + } + if (PTHREAD_DEBUG_ENABLED) { + if (!err) { + pthread_debug_mutex_lock_check(mutex); } - return err; + } + return err; +} +#endif + +int pthread_mutex_timedlock(pthread_mutex_t* mutex, const timespec* abs_timeout) { + return __pthread_mutex_timedlock(mutex, abs_timeout, CLOCK_REALTIME); } int pthread_mutex_destroy(pthread_mutex_t *mutex) diff --git a/libc/include/pthread.h b/libc/include/pthread.h index 6330a6f9d..461bb44cf 100644 --- a/libc/include/pthread.h +++ b/libc/include/pthread.h @@ -174,7 +174,7 @@ int pthread_mutexattr_settype(pthread_mutexattr_t*, int) __nonnull((1)); int pthread_mutex_destroy(pthread_mutex_t*) __nonnull((1)); int pthread_mutex_init(pthread_mutex_t*, const pthread_mutexattr_t*) __nonnull((1)); int pthread_mutex_lock(pthread_mutex_t*) __nonnull((1)); -int pthread_mutex_timedlock(pthread_mutex_t*, struct timespec*) __nonnull((1, 2)); +int pthread_mutex_timedlock(pthread_mutex_t*, const struct timespec*) __nonnull((1, 2)); int pthread_mutex_trylock(pthread_mutex_t*) __nonnull((1)); int pthread_mutex_unlock(pthread_mutex_t*) __nonnull((1)); @@ -245,23 +245,11 @@ int pthread_cond_timedwait_monotonic_np(pthread_cond_t*, pthread_mutex_t*, const int pthread_cond_timedwait_monotonic(pthread_cond_t*, pthread_mutex_t*, const struct timespec*); #define HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC 1 -/* - * Like pthread_cond_timedwait except 'reltime' is relative to the current time. - * TODO: not like glibc; include in LP64? - */ -int pthread_cond_timedwait_relative_np(pthread_cond_t*, pthread_mutex_t*, const struct timespec*); -#define HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE 1 +int pthread_cond_timedwait_relative_np(pthread_cond_t*, pthread_mutex_t*, const struct timespec*) /* TODO: __attribute__((deprecated("use pthread_cond_timedwait instead")))*/; +#define HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE 1 /* TODO: stop defining this to push LP32 off this API sooner. */ +int pthread_cond_timeout_np(pthread_cond_t*, pthread_mutex_t*, unsigned) /* TODO: __attribute__((deprecated("use pthread_cond_timedwait instead")))*/; -/* TODO: not like glibc; include in LP64? */ -int pthread_cond_timeout_np(pthread_cond_t*, pthread_mutex_t*, unsigned); - -/* Like pthread_mutex_lock(), but will wait up to 'msecs' milli-seconds - * before returning. Same return values as pthread_mutex_trylock though, i.e. - * returns EBUSY if the lock could not be acquired after the timeout expired. - * - * TODO: replace with pthread_mutex_timedlock_np for LP64. - */ -int pthread_mutex_lock_timeout_np(pthread_mutex_t*, unsigned); +int pthread_mutex_lock_timeout_np(pthread_mutex_t*, unsigned) __attribute__((deprecated("use pthread_mutex_timedlock instead"))); #endif /* !defined(__LP64__) */ diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index d481a1d86..120bbc7bf 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -653,3 +653,26 @@ TEST(pthread, pthread_cond_broadcast__preserves_condattr_flags) { GTEST_LOG_(INFO) << "This test does nothing.\n"; #endif // __BIONIC__ } + +TEST(pthread, pthread_mutex_timedlock) { + pthread_mutex_t m; + ASSERT_EQ(0, pthread_mutex_init(&m, NULL)); + + // If the mutex is already locked, pthread_mutex_timedlock should time out. + ASSERT_EQ(0, pthread_mutex_lock(&m)); + + timespec ts; + ASSERT_EQ(0, clock_gettime(CLOCK_REALTIME, &ts)); + ts.tv_nsec += 1; + ASSERT_EQ(ETIMEDOUT, pthread_mutex_timedlock(&m, &ts)); + + // If the mutex is unlocked, pthread_mutex_timedlock should succeed. + ASSERT_EQ(0, pthread_mutex_unlock(&m)); + + ASSERT_EQ(0, clock_gettime(CLOCK_REALTIME, &ts)); + ts.tv_nsec += 1; + ASSERT_EQ(0, pthread_mutex_timedlock(&m, &ts)); + + ASSERT_EQ(0, pthread_mutex_unlock(&m)); + ASSERT_EQ(0, pthread_mutex_destroy(&m)); +}