From bb032b935481a93badd3be9180538f075fb8f4b4 Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Thu, 6 Nov 2025 23:25:42 +0000 Subject: [PATCH 1/7] Fix crash-causing pthread race condition & rework pthread implementation When the app tries to use a mutex function such as pthread_mutex_lock, we run mutex_static_initializer if the mutex isn't initialized. mutex_static_initializer internally calls pthread_mutex_init, which in turn sets up the wrapped native mutex of the shim mutex struct. A problem arises in the following scenario: thread 1 calls pthread_mutex_init; thread 2 calls pthread_mutex_lock. In this case, pthread_mutex_lock thinks the mutex isn't initialized, so it calls the static initializer which in turn calls pthread_mutex_init, setting the wrapped native mutex pointer. pthread_mutex_init runs at the same time but on a different thread, messing up the internal state. The solution is two-pronged: 1. use an atomic integer to determine whether a mutex is initialized; 2. protect pthread_mutex_init exposed to the app, as well as pthread_static_initializer, with the same mutex internally, thus preventing them from racing. --- src/pthreads.cpp | 101 +++++++++++++++++++++++++---------------------- src/pthreads.h | 68 ++++++++++++++++--------------- 2 files changed, 89 insertions(+), 80 deletions(-) diff --git a/src/pthreads.cpp b/src/pthreads.cpp index 46b79eb..f7ded4d 100644 --- a/src/pthreads.cpp +++ b/src/pthreads.cpp @@ -12,6 +12,8 @@ using namespace shim; +static std::mutex global_shim_init_mutex; + thread_local bionic::pthread_cleanup_holder bionic::cleanup; bionic::pthread_cleanup_holder::~pthread_cleanup_holder() { @@ -22,7 +24,9 @@ bionic::pthread_cleanup_holder::~pthread_cleanup_holder() { } } -void bionic::mutex_static_initializer(shim::pthread_mutex_t *mutex, shim::pthread_mutex_t &old) { +void bionic::mutex_static_initializer(shim::pthread_mutex_t *mutex) { + std::lock_guard lk(global_shim_init_mutex); + if (is_mutex_initialized(mutex)) return; @@ -31,7 +35,6 @@ void bionic::mutex_static_initializer(shim::pthread_mutex_t *mutex, shim::pthrea #else auto init_value = (size_t) mutex->wrapped; #endif - shim::pthread_mutex_t n = old; pthread_mutexattr_t attr; pthread_mutexattr_init(&attr); @@ -39,48 +42,21 @@ void bionic::mutex_static_initializer(shim::pthread_mutex_t *mutex, shim::pthrea pthread_mutexattr_settype(&attr, (int) mutex_type::RECURSIVE); else if (init_value == errorcheck_mutex_init_value) pthread_mutexattr_settype(&attr, (int) mutex_type::ERRORCHECK); - if (pthread_mutex_init(&n, &attr) != 0) + if (pthread_mutex_init_internal(mutex, &attr) != 0) handle_runtime_error("Failed to init mutex"); pthread_mutexattr_destroy(&attr); - - if(!detail::update_wrapper(mutex, old, n)) { - // Concurrent creation, we need to destroy the new one - detail::destroy_c_wrapped(&n, &::pthread_mutex_destroy); - } else { - old = n; - } } -void bionic::cond_static_initializer(shim::pthread_cond_t *cond, shim::pthread_cond_t &old) { - if (is_cond_initialized(&old)) - return; - shim::pthread_cond_t n = old; - if(pthread_cond_init(&n, nullptr) == 0) { - if(!detail::update_wrapper(cond, old, n)) { - // Concurrent creation, we need to destroy the new one - detail::destroy_c_wrapped(&n, &::pthread_cond_destroy); - } else { - old = n; - } - } else { - handle_runtime_error("Failed to init cond"); - } +void bionic::cond_static_initializer(shim::pthread_cond_t *cond) { + std::lock_guard lk(global_shim_init_mutex); + if (!is_cond_initialized(cond)) + pthread_cond_init(cond, nullptr); } -void bionic::rwlock_static_initializer(shim::pthread_rwlock_t *rwlock, shim::pthread_rwlock_t &old) { - if (is_rwlock_initialized(&old)) - return; - shim::pthread_rwlock_t n = old; - if(pthread_rwlock_init(&n, nullptr) == 0) { - if(!detail::update_wrapper(rwlock, old, n)) { - // Concurrent creation, we need to destroy the new one - detail::destroy_c_wrapped(&n, &::pthread_rwlock_destroy); - } else { - old = n; - } - } else { - handle_runtime_error("Failed to init rwlock"); - } +void bionic::rwlock_static_initializer(shim::pthread_rwlock_t *rwlock) { + std::lock_guard lk(global_shim_init_mutex); + if (!is_rwlock_initialized(rwlock)) + pthread_rwlock_init(rwlock, nullptr); } int bionic::to_host_mutex_type(bionic::mutex_type type) { @@ -218,11 +194,11 @@ int shim::pthread_getattr_np(pthread_t th, pthread_attr_t* attr) { } int shim::pthread_attr_init(pthread_attr_t *attr) { - *attr = pthread_attr_t{false, false, 0, nullptr, 0, 0, bionic::sched_policy::OTHER, 0}; + *attr = pthread_attr_t{false, false, 0, nullptr, 0, 0, bionic::sched_policy::OTHER, 0, {0,0}}; return 0; } -int shim::pthread_attr_destroy(shim::pthread_attr_t *attr) { +int shim::pthread_attr_destroy(shim::pthread_attr_t *) { return 0; } @@ -249,7 +225,7 @@ int shim::pthread_attr_getschedparam(shim::pthread_attr_t *attr, shim::bionic::s } int shim::pthread_attr_setstacksize(shim::pthread_attr_t *attr, size_t value) { - if (value < PTHREAD_STACK_MIN) + if ((long)value < PTHREAD_STACK_MIN) return 0; attr->stack_size = value; return 0; @@ -278,22 +254,37 @@ int shim::pthread_setname_np(pthread_t thread, const char* name) { #endif } -int shim::pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr) { +int shim::pthread_mutex_init_internal(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr) { host_mutexattr hattr (attr); int ret = detail::make_c_wrapped(mutex, &::pthread_mutex_init, &hattr.attr); + if (ret == 0) { + mark_mutex_initialized(mutex); + } return bionic::translate_errno_from_host(ret); } -int shim::pthread_mutex_destroy(pthread_mutex_t *wrapper) { - auto mutex = detail::load_wrapper(wrapper); +int shim::pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr) { + std::lock_guard lk(global_shim_init_mutex); + return pthread_mutex_init_internal(mutex, attr); +} + +int shim::pthread_mutex_destroy(pthread_mutex_t *mutex) { if (!bionic::is_mutex_initialized(mutex)) return 0; - int ret = detail::destroy_c_wrapped(&mutex.value, &::pthread_mutex_destroy); + int ret = detail::destroy_c_wrapped(mutex, &::pthread_mutex_destroy); + mark_mutex_destroyed(mutex); return bionic::translate_errno_from_host(ret); } int shim::pthread_mutex_lock(pthread_mutex_t *mutex) { - int ret = ::pthread_mutex_lock(bionic::to_host(mutex)); + auto host_mutex = bionic::to_host(mutex); + auto check_value = reinterpret_cast<::pthread_mutex_t *>(mutex->check.load()); + int ret = 0; + + if (host_mutex == check_value) [[likely]] { + ret = ::pthread_mutex_lock(host_mutex); + } + return bionic::translate_errno_from_host(ret); } @@ -313,6 +304,8 @@ int shim::pthread_mutexattr_init(pthread_mutexattr_t *attr) { } int shim::pthread_mutexattr_destroy(pthread_mutexattr_t *attr) { + // TODO: have a unique state and detect usage of destroyed mutex attrs + *attr = pthread_mutexattr_t{bionic::mutex_type::NORMAL}; return 0; } @@ -360,12 +353,25 @@ int shim::pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, c return bionic::translate_errno_from_host(ret); } +int shim::pthread_cond_timedwait_monotonic_np(pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *ts) { + int ret; + #if __linux__ + ret = ::pthread_cond_clockwait(bionic::to_host(cond), bionic::to_host(mutex), CLOCK_MONOTONIC, ts); + #else + ret = shim::pthread_cond_timedwait(cond, mutex, ts); + #endif + + return bionic::translate_errno_from_host(ret); +} + int shim::pthread_condattr_init(pthread_condattr_t *attr) { *attr = {false, bionic::clock_type::MONOTONIC}; return 0; } int shim::pthread_condattr_destroy(pthread_condattr_t *attr) { + // TODO: have a unique state and detect usage of destroyed condattr + *attr = {false, bionic::clock_type::MONOTONIC}; return 0; } @@ -494,8 +500,7 @@ void shim::add_pthread_shimmed_symbols(std::vector &list) { {"pthread_cond_broadcast", pthread_cond_broadcast}, {"pthread_cond_signal", pthread_cond_signal}, {"pthread_cond_timedwait", pthread_cond_timedwait}, - // TODO figure out how to implement this correctly, this will use the default clock of the cond variable - {"pthread_cond_timedwait_monotonic_np", pthread_cond_timedwait}, + {"pthread_cond_timedwait_monotonic_np", pthread_cond_timedwait_monotonic_np}, {"pthread_condattr_init", pthread_condattr_init}, {"pthread_condattr_destroy", pthread_condattr_destroy}, {"pthread_condattr_setclock", pthread_condattr_setclock}, diff --git a/src/pthreads.h b/src/pthreads.h index f84680c..aeff9b4 100644 --- a/src/pthreads.h +++ b/src/pthreads.h @@ -12,14 +12,13 @@ namespace shim { namespace bionic { + // EXPECTED: Size: 40 bytes, alignment 8 bytes struct pthread_mutex_t { -#if defined(__LP64__) - size_t init_value; - ::pthread_mutex_t *wrapped; - int64_t priv[2]; -#else - ::pthread_mutex_t *wrapped; -#endif + size_t init_value = 0; + ::pthread_mutex_t *wrapped = nullptr; + std::atomic_int64_t is_initialized = 0; + std::atomic_int64_t check = 0; + int64_t priv = 0; }; constexpr size_t mutex_init_value = 0; @@ -27,16 +26,21 @@ namespace shim { constexpr size_t errorcheck_mutex_init_value = 0x8000; inline bool is_mutex_initialized(pthread_mutex_t const *m) { -#if defined(__LP64__) - return (m->wrapped != nullptr); -#else - return ((size_t) m->wrapped != mutex_init_value && - (size_t) m->wrapped != recursive_mutex_init_value && - (size_t) m->wrapped != errorcheck_mutex_init_value); -#endif + return m->is_initialized.load(); } - void mutex_static_initializer(pthread_mutex_t *mutex, pthread_mutex_t &old); + inline void mark_mutex_initialized(pthread_mutex_t *m) { + m->check.store(reinterpret_cast(m->wrapped)); + + m->is_initialized.store(1); + } + + inline void mark_mutex_destroyed(pthread_mutex_t *m) { + // TODO: have a separate state and detect it + m->is_initialized.store(0); + } + + void mutex_static_initializer(pthread_mutex_t *mutex); struct pthread_cond_t { ::pthread_cond_t *wrapped; @@ -46,12 +50,10 @@ namespace shim { }; inline bool is_cond_initialized(pthread_cond_t const *m) { - auto wrapped_ptr = reinterpret_cast *>(m); - auto val = wrapped_ptr->load(std::memory_order_relaxed); - return val.wrapped != nullptr; + return m->wrapped != nullptr; } - void cond_static_initializer(pthread_cond_t *cond, pthread_cond_t &old); + void cond_static_initializer(pthread_cond_t *cond); struct pthread_rwlock_t { ::pthread_rwlock_t *wrapped; @@ -64,25 +66,25 @@ namespace shim { return m->wrapped != nullptr; } - void rwlock_static_initializer(pthread_rwlock_t *rwlock, pthread_rwlock_t &old); + void rwlock_static_initializer(pthread_rwlock_t *rwlock); template <> - inline auto to_host(pthread_mutex_t const *o) { - auto m = detail::load_wrapper(o); - mutex_static_initializer(const_cast(o), m.value); - return m.value.wrapped; + inline auto to_host(pthread_mutex_t const *m) { + if (!is_mutex_initialized(m)) + mutex_static_initializer(const_cast(m)); + return m->wrapped; } template <> - inline auto to_host(pthread_cond_t const *o) { - auto m = detail::load_wrapper(o); - cond_static_initializer(const_cast(o), m.value); - return m.value.wrapped; + inline auto to_host(pthread_cond_t const *m) { + if (m->wrapped == nullptr) + cond_static_initializer(const_cast(m)); + return m->wrapped; } template <> - inline auto to_host(pthread_rwlock_t const *o) { - auto m = detail::load_wrapper(o); - rwlock_static_initializer(const_cast(o), m.value); - return m.value.wrapped; + inline auto to_host(pthread_rwlock_t const *m) { + if (m->wrapped == nullptr) + rwlock_static_initializer(const_cast(m)); + return m->wrapped; } enum class mutex_type : uint32_t { @@ -207,6 +209,7 @@ namespace shim { int pthread_setname_np(pthread_t thread, const char* name); + int pthread_mutex_init_internal(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr); int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr); int pthread_mutex_destroy(pthread_mutex_t *mutex); int pthread_mutex_lock(pthread_mutex_t *mutex); @@ -224,6 +227,7 @@ namespace shim { int pthread_cond_broadcast(pthread_cond_t *cond); int pthread_cond_signal(pthread_cond_t *cond); int pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *ts); + int pthread_cond_timedwait_monotonic_np(pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *ts); int pthread_condattr_init(pthread_condattr_t *attr); int pthread_condattr_destroy(pthread_condattr_t *attr); From f1505e0e801aa4bfe44e11791154457f2b9c572d Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Fri, 7 Nov 2025 00:10:37 +0000 Subject: [PATCH 2/7] fix pthread_mutex_t 32-bit ABI compat --- src/pthreads.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pthreads.h b/src/pthreads.h index aeff9b4..35a1095 100644 --- a/src/pthreads.h +++ b/src/pthreads.h @@ -12,13 +12,21 @@ namespace shim { namespace bionic { - // EXPECTED: Size: 40 bytes, alignment 8 bytes struct pthread_mutex_t { +#if defined(__LP64__) + // EXPECTED: Size: 40 bytes, alignment 8 bytes size_t init_value = 0; ::pthread_mutex_t *wrapped = nullptr; std::atomic_int64_t is_initialized = 0; std::atomic_int64_t check = 0; int64_t priv = 0; +#else + // EXPECTED: Size: 24 bytes, alignment 4 bytes + size_t init_value = 0; + ::pthread_mutex_t *wrapped = nullptr; + std::atomic_int32_t is_initialized = 0; + std::atomic_int32_t check = 0; +#endif }; constexpr size_t mutex_init_value = 0; From ed09a0d6163d9fc81dfcb53c9a58f35d2fcd1902 Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Fri, 7 Nov 2025 01:39:27 +0000 Subject: [PATCH 3/7] restore 32-bit ABI compat --- src/pthreads.cpp | 6 +++++- src/pthreads.h | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/pthreads.cpp b/src/pthreads.cpp index f7ded4d..d37c75d 100644 --- a/src/pthreads.cpp +++ b/src/pthreads.cpp @@ -278,13 +278,17 @@ int shim::pthread_mutex_destroy(pthread_mutex_t *mutex) { int shim::pthread_mutex_lock(pthread_mutex_t *mutex) { auto host_mutex = bionic::to_host(mutex); +#if defined(__LP64__) auto check_value = reinterpret_cast<::pthread_mutex_t *>(mutex->check.load()); + int ret = 0; if (host_mutex == check_value) [[likely]] { ret = ::pthread_mutex_lock(host_mutex); } - +#else + int ret = ::pthread_mutex_lock(mutex->wrapped); +#endif return bionic::translate_errno_from_host(ret); } diff --git a/src/pthreads.h b/src/pthreads.h index 35a1095..cd64c01 100644 --- a/src/pthreads.h +++ b/src/pthreads.h @@ -21,11 +21,8 @@ namespace shim { std::atomic_int64_t check = 0; int64_t priv = 0; #else - // EXPECTED: Size: 24 bytes, alignment 4 bytes - size_t init_value = 0; - ::pthread_mutex_t *wrapped = nullptr; - std::atomic_int32_t is_initialized = 0; - std::atomic_int32_t check = 0; + // EXPECTED: Size: 4 bytes, alignment 4 bytes + ::pthread_mutex_t* wrapped = nullptr; #endif }; @@ -34,18 +31,27 @@ namespace shim { constexpr size_t errorcheck_mutex_init_value = 0x8000; inline bool is_mutex_initialized(pthread_mutex_t const *m) { +#if defined(__LP64__) return m->is_initialized.load(); +#else + return ((size_t) m->wrapped != mutex_init_value && + (size_t) m->wrapped != recursive_mutex_init_value && + (size_t) m->wrapped != errorcheck_mutex_init_value); +#endif } inline void mark_mutex_initialized(pthread_mutex_t *m) { +#if defined(__LP64__) m->check.store(reinterpret_cast(m->wrapped)); - m->is_initialized.store(1); +#endif } inline void mark_mutex_destroyed(pthread_mutex_t *m) { +#if defined(__LP64__) // TODO: have a separate state and detect it m->is_initialized.store(0); +#endif } void mutex_static_initializer(pthread_mutex_t *mutex); From 499996ad7589fa7158d5e8c137d8187b10d5e98e Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Fri, 7 Nov 2025 22:49:10 +0000 Subject: [PATCH 4/7] fix: force pthread_mutex_t to have proper alignment regardless of platform preference --- src/pthreads.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pthreads.h b/src/pthreads.h index cd64c01..3335621 100644 --- a/src/pthreads.h +++ b/src/pthreads.h @@ -12,7 +12,13 @@ namespace shim { namespace bionic { - struct pthread_mutex_t { +#if defined(__LP64__) + #define LIBC_SHIM_BYTE_ALIGNMENT 8 +#else + #define LIBC_SHIM_BYTE_ALIGNMENT 4 +#endif + + struct alignas(LIBC_SHIM_BYTE_ALIGNMENT) pthread_mutex_t { #if defined(__LP64__) // EXPECTED: Size: 40 bytes, alignment 8 bytes size_t init_value = 0; @@ -26,6 +32,8 @@ namespace shim { #endif }; + #undef LIBC_SHIM_BYTE_ALIGNMENT + constexpr size_t mutex_init_value = 0; constexpr size_t recursive_mutex_init_value = 0x4000; constexpr size_t errorcheck_mutex_init_value = 0x8000; From 214f3c00b51c3932e1fc9270d06b6ad197de4f9d Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Sat, 15 Nov 2025 14:05:37 +0000 Subject: [PATCH 5/7] (hopefully) fix pthread_mutex_t on OSX --- src/pthreads.cpp | 22 +++++++++++++--------- src/pthreads.h | 36 +++++++++++++++++------------------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/pthreads.cpp b/src/pthreads.cpp index d37c75d..c6ed756 100644 --- a/src/pthreads.cpp +++ b/src/pthreads.cpp @@ -277,18 +277,22 @@ int shim::pthread_mutex_destroy(pthread_mutex_t *mutex) { } int shim::pthread_mutex_lock(pthread_mutex_t *mutex) { - auto host_mutex = bionic::to_host(mutex); -#if defined(__LP64__) - auto check_value = reinterpret_cast<::pthread_mutex_t *>(mutex->check.load()); - - int ret = 0; +#ifdef __LP64__ + bionic::MutexState current_state = mutex->state.load(std::memory_order_acquire); - if (host_mutex == check_value) [[likely]] { - ret = ::pthread_mutex_lock(host_mutex); + if (current_state == bionic::MutexState::Uninitialized) { + bionic::mutex_static_initializer(mutex); + } else if (current_state == bionic::MutexState::Destroyed) { + handle_runtime_error("Attempt to lock a mutex that has been destroyed."); + return EINVAL; } -#else - int ret = ::pthread_mutex_lock(mutex->wrapped); #endif + + auto host_mutex = bionic::to_host(mutex); + int ret = 0; + + ret = ::pthread_mutex_lock(host_mutex); + return bionic::translate_errno_from_host(ret); } diff --git a/src/pthreads.h b/src/pthreads.h index 3335621..57bb892 100644 --- a/src/pthreads.h +++ b/src/pthreads.h @@ -11,36 +11,36 @@ namespace shim { namespace bionic { + enum class MutexState : int64_t { + Uninitialized = 0, + Initialized = 1, + Destroyed = 2, + }; + struct pthread_mutex_t { #if defined(__LP64__) - #define LIBC_SHIM_BYTE_ALIGNMENT 8 -#else - #define LIBC_SHIM_BYTE_ALIGNMENT 4 -#endif - - struct alignas(LIBC_SHIM_BYTE_ALIGNMENT) pthread_mutex_t { -#if defined(__LP64__) - // EXPECTED: Size: 40 bytes, alignment 8 bytes + // EXPECTED: Size: 32 bytes, alignment 8 or 16 bytes + // NOTE: On OSX and likely other ARM platforms, + // alignment will make this structure larger than it is, + // so it should be at least a little below spec. + // (Bionic is normally 40 bytes with 8 byte alignment) size_t init_value = 0; ::pthread_mutex_t *wrapped = nullptr; - std::atomic_int64_t is_initialized = 0; - std::atomic_int64_t check = 0; - int64_t priv = 0; + std::atomic state{MutexState::Uninitialized}; + char __private[4]; #else // EXPECTED: Size: 4 bytes, alignment 4 bytes ::pthread_mutex_t* wrapped = nullptr; #endif }; - #undef LIBC_SHIM_BYTE_ALIGNMENT - constexpr size_t mutex_init_value = 0; constexpr size_t recursive_mutex_init_value = 0x4000; constexpr size_t errorcheck_mutex_init_value = 0x8000; inline bool is_mutex_initialized(pthread_mutex_t const *m) { #if defined(__LP64__) - return m->is_initialized.load(); + return m->state.load(std::memory_order_acquire) == MutexState::Initialized; #else return ((size_t) m->wrapped != mutex_init_value && (size_t) m->wrapped != recursive_mutex_init_value && @@ -48,17 +48,15 @@ namespace shim { #endif } - inline void mark_mutex_initialized(pthread_mutex_t *m) { + inline __attribute__((always_inline)) void mark_mutex_initialized(pthread_mutex_t *m) { #if defined(__LP64__) - m->check.store(reinterpret_cast(m->wrapped)); - m->is_initialized.store(1); + m->state.store(MutexState::Initialized, std::memory_order_release); #endif } inline void mark_mutex_destroyed(pthread_mutex_t *m) { #if defined(__LP64__) - // TODO: have a separate state and detect it - m->is_initialized.store(0); + m->state.store(MutexState::Destroyed, std::memory_order_release); #endif } From f067f262636e26bbdb44c81ffadd4d1a6a003163 Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:56:16 +0000 Subject: [PATCH 6/7] more accurately handle use of destroyed mutex --- src/pthreads.cpp | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/pthreads.cpp b/src/pthreads.cpp index c6ed756..7e74b07 100644 --- a/src/pthreads.cpp +++ b/src/pthreads.cpp @@ -268,6 +268,14 @@ int shim::pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t * return pthread_mutex_init_internal(mutex, attr); } +int handle_using_destroyed_mutex(const char* type) { + // Using an uninitialized mutex is non-crashing on NDK < 28 + // MCPE targets NDK 28 exactly, so this is valid. + handle_runtime_error("Attempt to %s a destroyed mutex.", type); + // In other cases, return EBUSY. + return bionic::translate_errno_from_host(EBUSY); +} + int shim::pthread_mutex_destroy(pthread_mutex_t *mutex) { if (!bionic::is_mutex_initialized(mutex)) return 0; @@ -282,21 +290,31 @@ int shim::pthread_mutex_lock(pthread_mutex_t *mutex) { if (current_state == bionic::MutexState::Uninitialized) { bionic::mutex_static_initializer(mutex); - } else if (current_state == bionic::MutexState::Destroyed) { - handle_runtime_error("Attempt to lock a mutex that has been destroyed."); - return EINVAL; + } else if (current_state == bionic::MutexState::Destroyed) [[unlikely]] { + return handle_using_destroyed_mutex("lock"); } #endif auto host_mutex = bionic::to_host(mutex); - int ret = 0; - - ret = ::pthread_mutex_lock(host_mutex); + int ret = ::pthread_mutex_lock(host_mutex); return bionic::translate_errno_from_host(ret); } int shim::pthread_mutex_unlock(pthread_mutex_t *mutex) { +#ifndef __LP64__ + if (mutex == nullptr) { + return bionic::translate_errno_from_host(EINVAL); + } +#endif + +#ifdef __LP64__ + bionic::MutexState current_state = mutex->state.load(std::memory_order_acquire); + if (current_state == bionic::MutexState::Destroyed) [[unlikely]] { + return handle_using_destroyed_mutex("unlock"); + } +#endif + int ret = ::pthread_mutex_unlock(bionic::to_host(mutex)); return bionic::translate_errno_from_host(ret); } @@ -319,7 +337,7 @@ int shim::pthread_mutexattr_destroy(pthread_mutexattr_t *attr) { int shim::pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type) { if (type < 0 || type > (int) bionic::mutex_type::END) - return EINVAL; + return bionic::translate_errno_from_host(EINVAL); attr->type = (bionic::mutex_type) type; return 0; } From b3771aba0ca7c1f63745998bd34b9b3e9727e558 Mon Sep 17 00:00:00 2001 From: V3L0C1T13S <51764975+V3L0C1T13S@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:58:11 +0000 Subject: [PATCH 7/7] add guard for nullptr mutex on 32-bit ABI --- src/pthreads.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pthreads.cpp b/src/pthreads.cpp index 7e74b07..5827800 100644 --- a/src/pthreads.cpp +++ b/src/pthreads.cpp @@ -285,6 +285,12 @@ int shim::pthread_mutex_destroy(pthread_mutex_t *mutex) { } int shim::pthread_mutex_lock(pthread_mutex_t *mutex) { +#ifndef __LP64__ + if (mutex == nullptr) { + return bionic::translate_errno_from_host(EINVAL); + } +#endif + #ifdef __LP64__ bionic::MutexState current_state = mutex->state.load(std::memory_order_acquire);