diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp index 24066ae02..d2ff1aeb0 100644 --- a/libc/bionic/pthread_mutex.cpp +++ b/libc/bionic/pthread_mutex.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -80,7 +81,7 @@ #define MUTEX_STATE_FROM_BITS(v) FIELD_FROM_BITS(v, MUTEX_STATE_SHIFT, MUTEX_STATE_LEN) #define MUTEX_STATE_TO_BITS(v) FIELD_TO_BITS(v, MUTEX_STATE_SHIFT, MUTEX_STATE_LEN) -#define MUTEX_STATE_UNLOCKED 0 /* must be 0 to match __PTHREAD_MUTEX_INIT_VALUE */ +#define MUTEX_STATE_UNLOCKED 0 /* must be 0 to match PTHREAD_MUTEX_INITIALIZER */ #define MUTEX_STATE_LOCKED_UNCONTENDED 1 /* must be 1 due to atomic dec in unlock operation */ #define MUTEX_STATE_LOCKED_CONTENDED 2 /* must be 1 + LOCKED_UNCONTENDED due to atomic dec */ @@ -122,30 +123,17 @@ #define MUTEX_SHARED_MASK FIELD_MASK(MUTEX_SHARED_SHIFT,1) /* Mutex type: - * * We support normal, recursive and errorcheck mutexes. - * - * The constants defined here *cannot* be changed because they must match - * the C library ABI which defines the following initialization values in - * : - * - * __PTHREAD_MUTEX_INIT_VALUE - * __PTHREAD_RECURSIVE_MUTEX_VALUE - * __PTHREAD_ERRORCHECK_MUTEX_INIT_VALUE */ #define MUTEX_TYPE_SHIFT 14 #define MUTEX_TYPE_LEN 2 #define MUTEX_TYPE_MASK FIELD_MASK(MUTEX_TYPE_SHIFT,MUTEX_TYPE_LEN) -#define MUTEX_TYPE_NORMAL 0 /* Must be 0 to match __PTHREAD_MUTEX_INIT_VALUE */ -#define MUTEX_TYPE_RECURSIVE 1 -#define MUTEX_TYPE_ERRORCHECK 2 - #define MUTEX_TYPE_TO_BITS(t) FIELD_TO_BITS(t, MUTEX_TYPE_SHIFT, MUTEX_TYPE_LEN) -#define MUTEX_TYPE_BITS_NORMAL MUTEX_TYPE_TO_BITS(MUTEX_TYPE_NORMAL) -#define MUTEX_TYPE_BITS_RECURSIVE MUTEX_TYPE_TO_BITS(MUTEX_TYPE_RECURSIVE) -#define MUTEX_TYPE_BITS_ERRORCHECK MUTEX_TYPE_TO_BITS(MUTEX_TYPE_ERRORCHECK) +#define MUTEX_TYPE_BITS_NORMAL MUTEX_TYPE_TO_BITS(PTHREAD_MUTEX_NORMAL) +#define MUTEX_TYPE_BITS_RECURSIVE MUTEX_TYPE_TO_BITS(PTHREAD_MUTEX_RECURSIVE) +#define MUTEX_TYPE_BITS_ERRORCHECK MUTEX_TYPE_TO_BITS(PTHREAD_MUTEX_ERRORCHECK) /* Mutex owner field: * @@ -237,55 +225,66 @@ int pthread_mutexattr_getpshared(const pthread_mutexattr_t* attr, int* pshared) return 0; } -static inline atomic_int* get_mutex_value_pointer(pthread_mutex_t* mutex) { - static_assert(sizeof(atomic_int) == sizeof(mutex->value), - "mutex->value should actually be atomic_int in implementation."); +struct pthread_mutex_internal_t { + atomic_int state; +#if defined(__LP64__) + char __reserved[36]; +#endif +}; - // We prefer casting to atomic_int instead of declaring mutex->value to be atomic_int directly. - // Because using the second method pollutes pthread.h, and causes an error when compiling libcxx. - return reinterpret_cast(&mutex->value); +static_assert(sizeof(pthread_mutex_t) == sizeof(pthread_mutex_internal_t), + "pthread_mutex_t should actually be pthread_mutex_internal_t in implementation."); + +// For binary compatibility with old version of pthread_mutex_t, we can't use more strict alignment +// than 4-byte alignment. +static_assert(alignof(pthread_mutex_t) == 4, + "pthread_mutex_t should fulfill the alignment of pthread_mutex_internal_t."); + +static inline pthread_mutex_internal_t* __get_internal_mutex(pthread_mutex_t* mutex_interface) { + return reinterpret_cast(mutex_interface); } -int pthread_mutex_init(pthread_mutex_t* mutex, const pthread_mutexattr_t* attr) { - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); +int pthread_mutex_init(pthread_mutex_t* mutex_interface, const pthread_mutexattr_t* attr) { + pthread_mutex_internal_t* mutex = __get_internal_mutex(mutex_interface); + + memset(mutex, 0, sizeof(pthread_mutex_internal_t)); if (__predict_true(attr == NULL)) { - atomic_init(mutex_value_ptr, MUTEX_TYPE_BITS_NORMAL); + atomic_init(&mutex->state, MUTEX_TYPE_BITS_NORMAL); return 0; } - int value = 0; + int state = 0; if ((*attr & MUTEXATTR_SHARED_MASK) != 0) { - value |= MUTEX_SHARED_MASK; + state |= MUTEX_SHARED_MASK; } switch (*attr & MUTEXATTR_TYPE_MASK) { case PTHREAD_MUTEX_NORMAL: - value |= MUTEX_TYPE_BITS_NORMAL; + state |= MUTEX_TYPE_BITS_NORMAL; break; case PTHREAD_MUTEX_RECURSIVE: - value |= MUTEX_TYPE_BITS_RECURSIVE; + state |= MUTEX_TYPE_BITS_RECURSIVE; break; case PTHREAD_MUTEX_ERRORCHECK: - value |= MUTEX_TYPE_BITS_ERRORCHECK; + state |= MUTEX_TYPE_BITS_ERRORCHECK; break; default: return EINVAL; } - atomic_init(mutex_value_ptr, value); + atomic_init(&mutex->state, state); return 0; } -static inline int __pthread_normal_mutex_trylock(atomic_int* mutex_value_ptr, int shared) { +static inline __always_inline int __pthread_normal_mutex_trylock(pthread_mutex_internal_t* mutex, + int shared) { const int unlocked = shared | MUTEX_STATE_BITS_UNLOCKED; const int locked_uncontended = shared | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; - int mvalue = unlocked; - if (__predict_true(atomic_compare_exchange_strong_explicit(mutex_value_ptr, &mvalue, - locked_uncontended, - memory_order_acquire, - memory_order_relaxed))) { + int old_state = unlocked; + if (__predict_true(atomic_compare_exchange_strong_explicit(&mutex->state, &old_state, + locked_uncontended, memory_order_acquire, memory_order_relaxed))) { return 0; } return EBUSY; @@ -303,9 +302,11 @@ static inline int __pthread_normal_mutex_trylock(atomic_int* mutex_value_ptr, in * "type" value is zero, so the only bits that will be set are the ones in * the lock state field. */ -static inline int __pthread_normal_mutex_lock(atomic_int* mutex_value_ptr, int shared, - const timespec* abs_timeout_or_null, clockid_t clock) { - if (__predict_true(__pthread_normal_mutex_trylock(mutex_value_ptr, shared) == 0)) { +static inline __always_inline int __pthread_normal_mutex_lock(pthread_mutex_internal_t* mutex, + int shared, + const timespec* abs_timeout_or_null, + clockid_t clock) { + if (__predict_true(__pthread_normal_mutex_trylock(mutex, shared) == 0)) { return 0; } @@ -316,13 +317,13 @@ static inline int __pthread_normal_mutex_lock(atomic_int* mutex_value_ptr, int s // We want to go to sleep until the mutex is available, which requires // promoting it to locked_contended. We need to swap in the new state - // value and then wait until somebody wakes us up. + // and then wait until somebody wakes us up. // An atomic_exchange is used to compete with other threads for the lock. // If it returns unlocked, we have acquired the lock, otherwise another // thread still holds the lock and we should wait again. // If lock is acquired, an acquire fence is needed to make all memory accesses // made by other threads visible to the current CPU. - while (atomic_exchange_explicit(mutex_value_ptr, locked_contended, + while (atomic_exchange_explicit(&mutex->state, locked_contended, memory_order_acquire) != unlocked) { timespec ts; timespec* rel_timeout = NULL; @@ -332,7 +333,7 @@ static inline int __pthread_normal_mutex_lock(atomic_int* mutex_value_ptr, int s return ETIMEDOUT; } } - if (__futex_wait_ex(mutex_value_ptr, shared, locked_contended, rel_timeout) == -ETIMEDOUT) { + if (__futex_wait_ex(&mutex->state, shared, locked_contended, rel_timeout) == -ETIMEDOUT) { return ETIMEDOUT; } } @@ -343,7 +344,8 @@ static inline int __pthread_normal_mutex_lock(atomic_int* mutex_value_ptr, int s * Release a mutex of type NORMAL. The caller is responsible for determining * that we are in fact the owner of this lock. */ -static inline void __pthread_normal_mutex_unlock(atomic_int* mutex_value_ptr, int shared) { +static inline __always_inline void __pthread_normal_mutex_unlock(pthread_mutex_internal_t* mutex, + int shared) { const int unlocked = shared | MUTEX_STATE_BITS_UNLOCKED; const int locked_contended = shared | MUTEX_STATE_BITS_LOCKED_CONTENDED; @@ -352,7 +354,7 @@ static inline void __pthread_normal_mutex_unlock(atomic_int* mutex_value_ptr, in // one of them. // A release fence is required to make previous stores visible to next // lock owner threads. - if (atomic_exchange_explicit(mutex_value_ptr, unlocked, + if (atomic_exchange_explicit(&mutex->state, unlocked, memory_order_release) == locked_contended) { // Wake up one waiting thread. We don't know which thread will be // woken or when it'll start executing -- futexes make no guarantees @@ -372,7 +374,7 @@ static inline void __pthread_normal_mutex_unlock(atomic_int* mutex_value_ptr, in // we call wake, the thread we eventually wake will find an unlocked mutex // and will execute. Either way we have correct behavior and nobody is // orphaned on the wait queue. - __futex_wake_ex(mutex_value_ptr, shared, 1); + __futex_wake_ex(&mutex->state, shared, 1); } } @@ -382,11 +384,12 @@ static inline void __pthread_normal_mutex_unlock(atomic_int* mutex_value_ptr, in * Otherwise, it atomically increments the counter and returns 0. * */ -static inline int __recursive_increment(atomic_int* mutex_value_ptr, int mvalue) { +static inline __always_inline int __recursive_increment(pthread_mutex_internal_t* mutex, + int old_state) { // Detect recursive lock overflow and return EAGAIN. // This is safe because only the owner thread can modify the // counter bits in the mutex value. - if (MUTEX_COUNTER_BITS_WILL_OVERFLOW(mvalue)) { + if (MUTEX_COUNTER_BITS_WILL_OVERFLOW(old_state)) { return EAGAIN; } @@ -395,32 +398,30 @@ static inline int __recursive_increment(atomic_int* mutex_value_ptr, int mvalue) // loop to update the counter. The counter will not overflow in the loop, // as only the owner thread can change it. // The mutex is still locked, so we don't need a release fence. - atomic_fetch_add_explicit(mutex_value_ptr, MUTEX_COUNTER_BITS_ONE, memory_order_relaxed); + atomic_fetch_add_explicit(&mutex->state, MUTEX_COUNTER_BITS_ONE, memory_order_relaxed); return 0; } -static int __pthread_mutex_lock_with_timeout(pthread_mutex_t* mutex, +static int __pthread_mutex_lock_with_timeout(pthread_mutex_internal_t* mutex, const timespec* abs_timeout_or_null, clockid_t clock) { - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); + int old_state, mtype, tid, shared; - int mvalue, mtype, tid, shared; - - mvalue = atomic_load_explicit(mutex_value_ptr, memory_order_relaxed); - mtype = (mvalue & MUTEX_TYPE_MASK); - shared = (mvalue & MUTEX_SHARED_MASK); + old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed); + mtype = (old_state & MUTEX_TYPE_MASK); + shared = (old_state & MUTEX_SHARED_MASK); // Handle common case first. if ( __predict_true(mtype == MUTEX_TYPE_BITS_NORMAL) ) { - return __pthread_normal_mutex_lock(mutex_value_ptr, shared, abs_timeout_or_null, clock); + return __pthread_normal_mutex_lock(mutex, shared, abs_timeout_or_null, clock); } // Do we already own this recursive or error-check mutex? tid = __get_thread()->tid; - if (tid == MUTEX_OWNER_FROM_BITS(mvalue)) { + if (tid == MUTEX_OWNER_FROM_BITS(old_state)) { if (mtype == MUTEX_TYPE_BITS_ERRORCHECK) { return EDEADLK; } - return __recursive_increment(mutex_value_ptr, mvalue); + return __recursive_increment(mutex, old_state); } const int unlocked = mtype | shared | MUTEX_STATE_BITS_UNLOCKED; @@ -429,12 +430,12 @@ static int __pthread_mutex_lock_with_timeout(pthread_mutex_t* mutex, // First, if the mutex is unlocked, try to quickly acquire it. // In the optimistic case where this works, set the state to locked_uncontended. - if (mvalue == unlocked) { - int newval = MUTEX_OWNER_TO_BITS(tid) | locked_uncontended; + if (old_state == unlocked) { + int new_state = MUTEX_OWNER_TO_BITS(tid) | locked_uncontended; // If exchanged successfully, an acquire fence is required to make // all memory accesses made by other threads visible to the current CPU. - if (__predict_true(atomic_compare_exchange_strong_explicit(mutex_value_ptr, &mvalue, - newval, memory_order_acquire, memory_order_relaxed))) { + if (__predict_true(atomic_compare_exchange_strong_explicit(&mutex->state, &old_state, + new_state, memory_order_acquire, memory_order_relaxed))) { return 0; } } @@ -442,33 +443,33 @@ static int __pthread_mutex_lock_with_timeout(pthread_mutex_t* mutex, ScopedTrace trace("Contending for pthread mutex"); while (true) { - if (mvalue == unlocked) { + if (old_state == unlocked) { // NOTE: We put the state to locked_contended since we _know_ there // is contention when we are in this loop. This ensures all waiters // will be unlocked. - int newval = MUTEX_OWNER_TO_BITS(tid) | locked_contended; + int new_state = MUTEX_OWNER_TO_BITS(tid) | locked_contended; // If exchanged successfully, an acquire fence is required to make // all memory accesses made by other threads visible to the current CPU. - if (__predict_true(atomic_compare_exchange_weak_explicit(mutex_value_ptr, - &mvalue, newval, + if (__predict_true(atomic_compare_exchange_weak_explicit(&mutex->state, + &old_state, new_state, memory_order_acquire, memory_order_relaxed))) { return 0; } continue; - } else if (MUTEX_STATE_BITS_IS_LOCKED_UNCONTENDED(mvalue)) { + } else if (MUTEX_STATE_BITS_IS_LOCKED_UNCONTENDED(old_state)) { // We should set it to locked_contended beforing going to sleep. This can make // sure waiters will be woken up eventually. - int newval = MUTEX_STATE_BITS_FLIP_CONTENTION(mvalue); - if (__predict_false(!atomic_compare_exchange_weak_explicit(mutex_value_ptr, - &mvalue, newval, + int new_state = MUTEX_STATE_BITS_FLIP_CONTENTION(old_state); + if (__predict_false(!atomic_compare_exchange_weak_explicit(&mutex->state, + &old_state, new_state, memory_order_relaxed, memory_order_relaxed))) { continue; } - mvalue = newval; + old_state = new_state; } // We are in locked_contended state, sleep until someone wakes us up. @@ -480,54 +481,54 @@ static int __pthread_mutex_lock_with_timeout(pthread_mutex_t* mutex, return ETIMEDOUT; } } - if (__futex_wait_ex(mutex_value_ptr, shared, mvalue, rel_timeout) == -ETIMEDOUT) { + if (__futex_wait_ex(&mutex->state, shared, old_state, rel_timeout) == -ETIMEDOUT) { return ETIMEDOUT; } - mvalue = atomic_load_explicit(mutex_value_ptr, memory_order_relaxed); + old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed); } } -int pthread_mutex_lock(pthread_mutex_t* mutex) { - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); +int pthread_mutex_lock(pthread_mutex_t* mutex_interface) { + pthread_mutex_internal_t* mutex = __get_internal_mutex(mutex_interface); - int mvalue = atomic_load_explicit(mutex_value_ptr, memory_order_relaxed); - int mtype = (mvalue & MUTEX_TYPE_MASK); - int shared = (mvalue & MUTEX_SHARED_MASK); + int old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed); + int mtype = (old_state & MUTEX_TYPE_MASK); + int shared = (old_state & MUTEX_SHARED_MASK); // Avoid slowing down fast path of normal mutex lock operation. if (__predict_true(mtype == MUTEX_TYPE_BITS_NORMAL)) { - if (__predict_true(__pthread_normal_mutex_trylock(mutex_value_ptr, shared) == 0)) { + if (__predict_true(__pthread_normal_mutex_trylock(mutex, shared) == 0)) { return 0; } } return __pthread_mutex_lock_with_timeout(mutex, NULL, 0); } -int pthread_mutex_unlock(pthread_mutex_t* mutex) { - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); +int pthread_mutex_unlock(pthread_mutex_t* mutex_interface) { + pthread_mutex_internal_t* mutex = __get_internal_mutex(mutex_interface); - int mvalue, mtype, tid, shared; + int old_state, mtype, tid, shared; - mvalue = atomic_load_explicit(mutex_value_ptr, memory_order_relaxed); - mtype = (mvalue & MUTEX_TYPE_MASK); - shared = (mvalue & MUTEX_SHARED_MASK); + old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed); + mtype = (old_state & MUTEX_TYPE_MASK); + shared = (old_state & MUTEX_SHARED_MASK); // Handle common case first. if (__predict_true(mtype == MUTEX_TYPE_BITS_NORMAL)) { - __pthread_normal_mutex_unlock(mutex_value_ptr, shared); + __pthread_normal_mutex_unlock(mutex, shared); return 0; } // Do we already own this recursive or error-check mutex? tid = __get_thread()->tid; - if ( tid != MUTEX_OWNER_FROM_BITS(mvalue) ) + if ( tid != MUTEX_OWNER_FROM_BITS(old_state) ) return EPERM; // If the counter is > 0, we can simply decrement it atomically. // Since other threads can mutate the lower state bits (and only the // lower state bits), use a compare_exchange loop to do it. - if (!MUTEX_COUNTER_BITS_IS_ZERO(mvalue)) { + if (!MUTEX_COUNTER_BITS_IS_ZERO(old_state)) { // We still own the mutex, so a release fence is not needed. - atomic_fetch_sub_explicit(mutex_value_ptr, MUTEX_COUNTER_BITS_ONE, memory_order_relaxed); + atomic_fetch_sub_explicit(&mutex->state, MUTEX_COUNTER_BITS_ONE, memory_order_relaxed); return 0; } @@ -538,36 +539,36 @@ int pthread_mutex_unlock(pthread_mutex_t* mutex) { // A release fence is required to make previous stores visible to next // lock owner threads. const int unlocked = mtype | shared | MUTEX_STATE_BITS_UNLOCKED; - mvalue = atomic_exchange_explicit(mutex_value_ptr, unlocked, memory_order_release); - if (MUTEX_STATE_BITS_IS_LOCKED_CONTENDED(mvalue)) { - __futex_wake_ex(mutex_value_ptr, shared, 1); + old_state = atomic_exchange_explicit(&mutex->state, unlocked, memory_order_release); + if (MUTEX_STATE_BITS_IS_LOCKED_CONTENDED(old_state)) { + __futex_wake_ex(&mutex->state, shared, 1); } return 0; } -int pthread_mutex_trylock(pthread_mutex_t* mutex) { - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); +int pthread_mutex_trylock(pthread_mutex_t* mutex_interface) { + pthread_mutex_internal_t* mutex = __get_internal_mutex(mutex_interface); - int mvalue = atomic_load_explicit(mutex_value_ptr, memory_order_relaxed); - int mtype = (mvalue & MUTEX_TYPE_MASK); - int shared = (mvalue & MUTEX_SHARED_MASK); + int old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed); + int mtype = (old_state & MUTEX_TYPE_MASK); + int shared = (old_state & MUTEX_SHARED_MASK); const int unlocked = mtype | shared | MUTEX_STATE_BITS_UNLOCKED; const int locked_uncontended = mtype | shared | MUTEX_STATE_BITS_LOCKED_UNCONTENDED; // Handle common case first. if (__predict_true(mtype == MUTEX_TYPE_BITS_NORMAL)) { - return __pthread_normal_mutex_trylock(mutex_value_ptr, shared); + return __pthread_normal_mutex_trylock(mutex, shared); } // Do we already own this recursive or error-check mutex? pid_t tid = __get_thread()->tid; - if (tid == MUTEX_OWNER_FROM_BITS(mvalue)) { + if (tid == MUTEX_OWNER_FROM_BITS(old_state)) { if (mtype == MUTEX_TYPE_BITS_ERRORCHECK) { return EBUSY; } - return __recursive_increment(mutex_value_ptr, mvalue); + return __recursive_increment(mutex, old_state); } // Same as pthread_mutex_lock, except that we don't want to wait, and @@ -575,9 +576,9 @@ int pthread_mutex_trylock(pthread_mutex_t* mutex) { // lock if it is released / not owned by anyone. No need for a complex loop. // If exchanged successfully, an acquire fence is required to make // all memory accesses made by other threads visible to the current CPU. - mvalue = unlocked; - int newval = MUTEX_OWNER_TO_BITS(tid) | locked_uncontended; - if (__predict_true(atomic_compare_exchange_strong_explicit(mutex_value_ptr, &mvalue, newval, + old_state = unlocked; + int new_state = MUTEX_OWNER_TO_BITS(tid) | locked_uncontended; + if (__predict_true(atomic_compare_exchange_strong_explicit(&mutex->state, &old_state, new_state, memory_order_acquire, memory_order_relaxed))) { return 0; @@ -586,7 +587,7 @@ int pthread_mutex_trylock(pthread_mutex_t* mutex) { } #if !defined(__LP64__) -extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex, unsigned ms) { +extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex_interface, unsigned ms) { timespec abs_timeout; clock_gettime(CLOCK_MONOTONIC, &abs_timeout); abs_timeout.tv_sec += ms / 1000; @@ -596,7 +597,8 @@ extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex, unsigned ms abs_timeout.tv_nsec -= NS_PER_S; } - int error = __pthread_mutex_lock_with_timeout(mutex, &abs_timeout, CLOCK_MONOTONIC); + int error = __pthread_mutex_lock_with_timeout(__get_internal_mutex(mutex_interface), + &abs_timeout, CLOCK_MONOTONIC); if (error == ETIMEDOUT) { error = EBUSY; } @@ -604,18 +606,19 @@ extern "C" int pthread_mutex_lock_timeout_np(pthread_mutex_t* mutex, unsigned ms } #endif -int pthread_mutex_timedlock(pthread_mutex_t* mutex, const timespec* abs_timeout) { - return __pthread_mutex_lock_with_timeout(mutex, abs_timeout, CLOCK_REALTIME); +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); } -int pthread_mutex_destroy(pthread_mutex_t* mutex) { +int pthread_mutex_destroy(pthread_mutex_t* mutex_interface) { // Use trylock to ensure that the mutex is valid and not already locked. - int error = pthread_mutex_trylock(mutex); + int error = pthread_mutex_trylock(mutex_interface); if (error != 0) { return error; } - atomic_int* mutex_value_ptr = get_mutex_value_pointer(mutex); - atomic_store_explicit(mutex_value_ptr, 0xdead10cc, memory_order_relaxed); + pthread_mutex_internal_t* mutex = __get_internal_mutex(mutex_interface); + atomic_store_explicit(&mutex->state, 0xdead10cc, memory_order_relaxed); return 0; } diff --git a/libc/include/pthread.h b/libc/include/pthread.h index 234a43d51..0ad14dd13 100644 --- a/libc/include/pthread.h +++ b/libc/include/pthread.h @@ -36,30 +36,15 @@ #include #include -#if defined(__LP64__) - #define __RESERVED_INITIALIZER , {0} -#else - #define __RESERVED_INITIALIZER -#endif - typedef struct { - int value; -#ifdef __LP64__ - char __reserved[36]; +#if defined(__LP64__) + int32_t __private[10]; +#else + int32_t __private[1]; #endif } pthread_mutex_t; -#define __PTHREAD_MUTEX_INIT_VALUE 0 -#define __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE 0x4000 -#define __PTHREAD_ERRORCHECK_MUTEX_INIT_VALUE 0x8000 - -#define PTHREAD_MUTEX_INITIALIZER {__PTHREAD_MUTEX_INIT_VALUE __RESERVED_INITIALIZER} -#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP {__PTHREAD_ERRORCHECK_MUTEX_INIT_VALUE __RESERVED_INITIALIZER} -#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP {__PTHREAD_RECURSIVE_MUTEX_INIT_VALUE __RESERVED_INITIALIZER} - -/* TODO: remove this namespace pollution. */ -#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP -#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP +typedef long pthread_mutexattr_t; enum { PTHREAD_MUTEX_NORMAL = 0, @@ -72,28 +57,35 @@ enum { PTHREAD_MUTEX_DEFAULT = PTHREAD_MUTEX_NORMAL }; +#define PTHREAD_MUTEX_INITIALIZER { { ((PTHREAD_MUTEX_NORMAL & 3) << 14) } } +#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP { { ((PTHREAD_MUTEX_RECURSIVE & 3) << 14) } } +#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP { { ((PTHREAD_MUTEX_ERRORCHECK & 3) << 14) } } + +/* TODO: remove this namespace pollution. */ +#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP +#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP + typedef struct { #if defined(__LP64__) - char __private[48]; + int32_t __private[12]; #else - char __private[4]; + int32_t __private[1]; #endif -} pthread_cond_t __attribute__((aligned(4))); +} pthread_cond_t; + +typedef long pthread_condattr_t; #define PTHREAD_COND_INITIALIZER { { 0 } } -typedef long pthread_mutexattr_t; -typedef long pthread_condattr_t; - -typedef long pthread_rwlockattr_t; - typedef struct { #if defined(__LP64__) - char __private[56]; + int32_t __private[14]; #else - char __private[40]; + int32_t __private[10]; #endif -} pthread_rwlock_t __attribute__((aligned(4))); +} pthread_rwlock_t; + +typedef long pthread_rwlockattr_t; #define PTHREAD_RWLOCK_INITIALIZER { { 0 } } diff --git a/libc/stdio/fileext.h b/libc/stdio/fileext.h index 209815aee..6cacc0f84 100644 --- a/libc/stdio/fileext.h +++ b/libc/stdio/fileext.h @@ -61,7 +61,11 @@ do { \ _UB(fp)._base = NULL; \ _UB(fp)._size = 0; \ WCIO_INIT(fp); \ - _FLOCK(fp).value = __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE; \ + pthread_mutexattr_t attr; \ + pthread_mutexattr_init(&attr); \ + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); \ + pthread_mutex_init(&_FLOCK(fp), &attr); \ + pthread_mutexattr_destroy(&attr); \ _EXT(fp)->_stdio_handles_locking = true; \ } while (0) diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index b4667f939..632e28552 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -16,10 +16,6 @@ #include -#include "private/ScopeGuard.h" -#include "BionicDeathTest.h" -#include "ScopedSignalHandler.h" - #include #include #include @@ -35,6 +31,11 @@ #include #include +#include "private/bionic_macros.h" +#include "private/ScopeGuard.h" +#include "BionicDeathTest.h" +#include "ScopedSignalHandler.h" + TEST(pthread, pthread_key_create) { pthread_key_t key; ASSERT_EQ(0, pthread_key_create(&key, NULL)); @@ -1221,54 +1222,84 @@ TEST(pthread, pthread_mutexattr_gettype) { ASSERT_EQ(0, pthread_mutexattr_destroy(&attr)); } -static void CreateMutex(pthread_mutex_t& mutex, int mutex_type) { - pthread_mutexattr_t attr; - ASSERT_EQ(0, pthread_mutexattr_init(&attr)); - ASSERT_EQ(0, pthread_mutexattr_settype(&attr, mutex_type)); - ASSERT_EQ(0, pthread_mutex_init(&mutex, &attr)); - ASSERT_EQ(0, pthread_mutexattr_destroy(&attr)); -} +struct PthreadMutex { + pthread_mutex_t lock; + + PthreadMutex(int mutex_type) { + init(mutex_type); + } + + ~PthreadMutex() { + destroy(); + } + + private: + void init(int mutex_type) { + pthread_mutexattr_t attr; + ASSERT_EQ(0, pthread_mutexattr_init(&attr)); + ASSERT_EQ(0, pthread_mutexattr_settype(&attr, mutex_type)); + ASSERT_EQ(0, pthread_mutex_init(&lock, &attr)); + ASSERT_EQ(0, pthread_mutexattr_destroy(&attr)); + } + + void destroy() { + ASSERT_EQ(0, pthread_mutex_destroy(&lock)); + } + + DISALLOW_COPY_AND_ASSIGN(PthreadMutex); +}; TEST(pthread, pthread_mutex_lock_NORMAL) { - pthread_mutex_t lock; - CreateMutex(lock, PTHREAD_MUTEX_NORMAL); + PthreadMutex m(PTHREAD_MUTEX_NORMAL); - ASSERT_EQ(0, pthread_mutex_lock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_destroy(&lock)); + ASSERT_EQ(0, pthread_mutex_lock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); } TEST(pthread, pthread_mutex_lock_ERRORCHECK) { - pthread_mutex_t lock; - CreateMutex(lock, PTHREAD_MUTEX_ERRORCHECK); + PthreadMutex m(PTHREAD_MUTEX_ERRORCHECK); - ASSERT_EQ(0, pthread_mutex_lock(&lock)); - ASSERT_EQ(EDEADLK, pthread_mutex_lock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_trylock(&lock)); - ASSERT_EQ(EBUSY, pthread_mutex_trylock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(EPERM, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_destroy(&lock)); + ASSERT_EQ(0, pthread_mutex_lock(&m.lock)); + ASSERT_EQ(EDEADLK, pthread_mutex_lock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_trylock(&m.lock)); + ASSERT_EQ(EBUSY, pthread_mutex_trylock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); + ASSERT_EQ(EPERM, pthread_mutex_unlock(&m.lock)); } TEST(pthread, pthread_mutex_lock_RECURSIVE) { - pthread_mutex_t lock; - CreateMutex(lock, PTHREAD_MUTEX_RECURSIVE); + PthreadMutex m(PTHREAD_MUTEX_RECURSIVE); - ASSERT_EQ(0, pthread_mutex_lock(&lock)); - ASSERT_EQ(0, pthread_mutex_lock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_trylock(&lock)); - ASSERT_EQ(0, pthread_mutex_unlock(&lock)); - ASSERT_EQ(EPERM, pthread_mutex_unlock(&lock)); - ASSERT_EQ(0, pthread_mutex_destroy(&lock)); + ASSERT_EQ(0, pthread_mutex_lock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_lock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_trylock(&m.lock)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); + ASSERT_EQ(EPERM, pthread_mutex_unlock(&m.lock)); +} + +TEST(pthread, pthread_mutex_init_same_as_static_initializers) { + pthread_mutex_t lock_normal = PTHREAD_MUTEX_INITIALIZER; + PthreadMutex m1(PTHREAD_MUTEX_NORMAL); + ASSERT_EQ(0, memcmp(&lock_normal, &m1.lock, sizeof(pthread_mutex_t))); + pthread_mutex_destroy(&lock_normal); + + pthread_mutex_t lock_errorcheck = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + PthreadMutex m2(PTHREAD_MUTEX_ERRORCHECK); + ASSERT_EQ(0, memcmp(&lock_errorcheck, &m2.lock, sizeof(pthread_mutex_t))); + pthread_mutex_destroy(&lock_errorcheck); + + pthread_mutex_t lock_recursive = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; + PthreadMutex m3(PTHREAD_MUTEX_RECURSIVE); + ASSERT_EQ(0, memcmp(&lock_recursive, &m3.lock, sizeof(pthread_mutex_t))); + ASSERT_EQ(0, pthread_mutex_destroy(&lock_recursive)); } class MutexWakeupHelper { private: - pthread_mutex_t mutex; + PthreadMutex m; enum Progress { LOCK_INITIALIZED, LOCK_WAITING, @@ -1281,17 +1312,19 @@ class MutexWakeupHelper { ASSERT_EQ(LOCK_INITIALIZED, helper->progress); helper->progress = LOCK_WAITING; - ASSERT_EQ(0, pthread_mutex_lock(&helper->mutex)); + ASSERT_EQ(0, pthread_mutex_lock(&helper->m.lock)); ASSERT_EQ(LOCK_RELEASED, helper->progress); - ASSERT_EQ(0, pthread_mutex_unlock(&helper->mutex)); + ASSERT_EQ(0, pthread_mutex_unlock(&helper->m.lock)); helper->progress = LOCK_ACCESSED; } public: - void test(int mutex_type) { - CreateMutex(mutex, mutex_type); - ASSERT_EQ(0, pthread_mutex_lock(&mutex)); + MutexWakeupHelper(int mutex_type) : m(mutex_type) { + } + + void test() { + ASSERT_EQ(0, pthread_mutex_lock(&m.lock)); progress = LOCK_INITIALIZED; pthread_t thread; @@ -1303,27 +1336,26 @@ class MutexWakeupHelper { } usleep(5000); progress = LOCK_RELEASED; - ASSERT_EQ(0, pthread_mutex_unlock(&mutex)); + ASSERT_EQ(0, pthread_mutex_unlock(&m.lock)); ASSERT_EQ(0, pthread_join(thread, NULL)); ASSERT_EQ(LOCK_ACCESSED, progress); - ASSERT_EQ(0, pthread_mutex_destroy(&mutex)); } }; TEST(pthread, pthread_mutex_NORMAL_wakeup) { - MutexWakeupHelper helper; - helper.test(PTHREAD_MUTEX_NORMAL); + MutexWakeupHelper helper(PTHREAD_MUTEX_NORMAL); + helper.test(); } TEST(pthread, pthread_mutex_ERRORCHECK_wakeup) { - MutexWakeupHelper helper; - helper.test(PTHREAD_MUTEX_ERRORCHECK); + MutexWakeupHelper helper(PTHREAD_MUTEX_ERRORCHECK); + helper.test(); } TEST(pthread, pthread_mutex_RECURSIVE_wakeup) { - MutexWakeupHelper helper; - helper.test(PTHREAD_MUTEX_RECURSIVE); + MutexWakeupHelper helper(PTHREAD_MUTEX_RECURSIVE); + helper.test(); } TEST(pthread, pthread_mutex_owner_tid_limit) {