From 76b7f998f5cd0f9a8bdb06246c487938ffef8972 Mon Sep 17 00:00:00 2001 From: QDarkRex Date: Sun, 27 Sep 2026 12:38:59 +0700 Subject: [PATCH] fix(kernel): give every thread a pthread handle and keep mutex ownership consistent - scePthreadSelf returned null on Linux and for threads not created by scePthreadCreate (including the main thread); such threads now get a lazily adopted, non-joinable handle - scePthreadExit on Linux skipped FinishThread, losing the join value - detached threads on Linux were never freed; use the reference count already used on Windows - unlocking a recursive mutex from a non-owner was undefined behaviour; return SCE_KERNEL_ERROR_EPERM - scePthreadCondTimedwait did not restore the mutex owner after waking, so another thread locking during the wait made the waiter's unlock fail with EPERM and left the mutex locked forever - SCE_KERNEL_ERROR_ETIMEDOUT in Cond.cpp was 0x80020062 instead of 0x8002003C Co-Authored-By: Claude Opus 5.5 --- core/libs/CMakeLists.txt | 20 +++++ .../prx/libkernel/Pthread/include/Pthread.hpp | 7 +- core/libs/prx/libkernel/Pthread/src/Cond.cpp | 9 ++- core/libs/prx/libkernel/Pthread/src/Mutex.cpp | 2 + .../libs/prx/libkernel/Pthread/src/Thread.cpp | 45 ++++++++--- core/libs/tests/GuestCondTimedwait.cpp | 52 +++++++++++++ core/libs/tests/GuestThreadSelf.cpp | 75 +++++++++++++++++++ 7 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 core/libs/tests/GuestCondTimedwait.cpp create mode 100644 core/libs/tests/GuestThreadSelf.cpp diff --git a/core/libs/CMakeLists.txt b/core/libs/CMakeLists.txt index 2ad90fd7e..c8abb5269 100644 --- a/core/libs/CMakeLists.txt +++ b/core/libs/CMakeLists.txt @@ -240,6 +240,26 @@ if(BUILD_TESTING) set_tests_properties(guest_once PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() + add_executable(guest_thread_self_tests tests/GuestThreadSelf.cpp) + target_include_directories(guest_thread_self_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_link_libraries(guest_thread_self_tests PRIVATE libkernel libc) + configure_windows_unwind(guest_thread_self_tests) + add_test(NAME guest_thread_self COMMAND guest_thread_self_tests) + set_tests_properties(guest_thread_self PROPERTIES TIMEOUT 20) + if(WIN32) + set_tests_properties(guest_thread_self PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() + add_executable(guest_cond_timedwait_tests tests/GuestCondTimedwait.cpp) + target_include_directories(guest_cond_timedwait_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_link_libraries(guest_cond_timedwait_tests PRIVATE libkernel libc) + configure_windows_unwind(guest_cond_timedwait_tests) + add_test(NAME guest_cond_timedwait COMMAND guest_cond_timedwait_tests) + set_tests_properties(guest_cond_timedwait PROPERTIES TIMEOUT 20) + if(WIN32) + set_tests_properties(guest_cond_timedwait PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() add_executable(guest_signal_tests tests/GuestSignals.cpp) target_include_directories(guest_signal_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(guest_signal_tests PRIVATE libkernel libc) diff --git a/core/libs/prx/libkernel/Pthread/include/Pthread.hpp b/core/libs/prx/libkernel/Pthread/include/Pthread.hpp index e0adf92b8..1e4c313f0 100644 --- a/core/libs/prx/libkernel/Pthread/include/Pthread.hpp +++ b/core/libs/prx/libkernel/Pthread/include/Pthread.hpp @@ -51,20 +51,21 @@ struct PthreadAttrPrivate { struct PthreadPrivate { #ifdef _WIN32 void* nativeHandle = nullptr; - std::thread::id threadId; - std::atomic references{2}; #else std::thread _thr; #endif + std::thread::id threadId; + std::atomic references{2}; void* stackAddress = nullptr; std::size_t stackSize = 0; std::atomic _finished; void* _retval; bool _detached; + bool _adopted; std::mutex _join_mtx; std::condition_variable _join_cv; - PthreadPrivate() : _finished(false), _retval(nullptr), _detached(false) {} + PthreadPrivate() : _finished(false), _retval(nullptr), _detached(false), _adopted(false) {} }; #endif diff --git a/core/libs/prx/libkernel/Pthread/src/Cond.cpp b/core/libs/prx/libkernel/Pthread/src/Cond.cpp index 754a076f3..8d7494f26 100644 --- a/core/libs/prx/libkernel/Pthread/src/Cond.cpp +++ b/core/libs/prx/libkernel/Pthread/src/Cond.cpp @@ -5,7 +5,7 @@ static constexpr int SCE_OK = 0; static constexpr int SCE_KERNEL_ERROR_ENOMEM = 0x8002000C; -static constexpr int SCE_KERNEL_ERROR_ETIMEDOUT = 0x80020062; +static constexpr int SCE_KERNEL_ERROR_ETIMEDOUT = 0x8002003C; extern "C" { @@ -58,12 +58,19 @@ int APS5_VABI scePthreadCondTimedwait(PthreadCond* cond, PthreadMutex* mutex, un auto* c = *cond; if (m->_type == MutexType::Recursive) { std::unique_lock lk(m->_rmtx, std::adopt_lock); + const int savedCount = m->_count; + m->_count = 0; + m->_owner.store(std::thread::id{}, std::memory_order_relaxed); auto res = c->_cv.wait_for(lk, std::chrono::microseconds(usec)); + m->_owner.store(std::this_thread::get_id(), std::memory_order_relaxed); + m->_count = savedCount; lk.release(); return res == std::cv_status::timeout ? SCE_KERNEL_ERROR_ETIMEDOUT : SCE_OK; } std::unique_lock lk(m->_mtx, std::adopt_lock); + m->_owner.store(std::thread::id{}, std::memory_order_relaxed); auto res = c->_cv.wait_for(lk, std::chrono::microseconds(usec)); + m->_owner.store(std::this_thread::get_id(), std::memory_order_relaxed); lk.release(); return res == std::cv_status::timeout ? SCE_KERNEL_ERROR_ETIMEDOUT : SCE_OK; } diff --git a/core/libs/prx/libkernel/Pthread/src/Mutex.cpp b/core/libs/prx/libkernel/Pthread/src/Mutex.cpp index cc9ac1881..e586e4060 100644 --- a/core/libs/prx/libkernel/Pthread/src/Mutex.cpp +++ b/core/libs/prx/libkernel/Pthread/src/Mutex.cpp @@ -81,6 +81,8 @@ int APS5_VABI scePthreadMutexUnlock(PthreadMutex* mutex) { return SCE_KERNEL_ERROR_EPERM; } if (m->_type == MutexType::Recursive) { + if (m->_owner.load(std::memory_order_acquire) != std::this_thread::get_id() || m->_count == 0) + return SCE_KERNEL_ERROR_EPERM; if (--m->_count == 0) m->_owner.store(std::thread::id{}, std::memory_order_relaxed); m->_rmtx.unlock(); return SCE_OK; diff --git a/core/libs/prx/libkernel/Pthread/src/Thread.cpp b/core/libs/prx/libkernel/Pthread/src/Thread.cpp index b594ba7a1..aab144818 100644 --- a/core/libs/prx/libkernel/Pthread/src/Thread.cpp +++ b/core/libs/prx/libkernel/Pthread/src/Thread.cpp @@ -47,17 +47,22 @@ static void RunThread(std::unique_ptr args) { FinishThread(self, entry(arg)); } -#ifdef _WIN32 static thread_local PthreadPrivate* currentThread = nullptr; +static thread_local std::unique_ptr adoptedThread; static void ReleaseThread(PthreadPrivate* thread) { + if (thread->_adopted) + return; if (thread->references.fetch_sub(1, std::memory_order_acq_rel) != 1) return; +#ifdef _WIN32 if (!CloseHandle(thread->nativeHandle)) throw std::system_error(GetLastError(), std::system_category(), "Closing guest thread handle"); +#endif delete thread; } +#ifdef _WIN32 struct NativeThreadArgs { std::unique_ptr guest; std::future start; @@ -136,8 +141,19 @@ int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, Pthread if (detached) ReleaseThread(published); #else - p->_thr = std::thread([args = std::move(args), ready = start.get_future()]() mutable { - if (ready.get()) RunThread(std::move(args)); + auto* self = p.get(); + p->_thr = std::thread([self, args = std::move(args), ready = start.get_future()]() mutable { + if (!ready.get()) return; + currentThread = self; + self->threadId = std::this_thread::get_id(); + struct ThreadGuard { + PthreadPrivate* self; + ~ThreadGuard() { + currentThread = nullptr; + ReleaseThread(self); + } + } guard{self}; + RunThread(std::move(args)); }); try { if (detached) p->_thr.detach(); @@ -146,8 +162,11 @@ int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, Pthread p->_thr.join(); throw; } - *thread = p.release(); + auto* published = p.release(); + *thread = published; start.set_value(true); + if (detached) + ReleaseThread(published); #endif return SCE_OK; } @@ -163,9 +182,11 @@ int APS5_VABI scePthreadJoin(Pthread thread, void** retval) { if (retval) *retval = thread->_retval; ReleaseThread(thread); #else + if (thread == currentThread) + throw std::runtime_error("scePthreadJoin: cannot join current thread"); if (thread->_thr.joinable()) thread->_thr.join(); if (retval) *retval = thread->_retval; - delete thread; + ReleaseThread(thread); #endif return SCE_OK; } @@ -178,6 +199,7 @@ int APS5_VABI scePthreadDetach(Pthread thread) { ReleaseThread(thread); #else if (thread->_thr.joinable()) thread->_thr.detach(); + ReleaseThread(thread); #endif return SCE_OK; } @@ -192,17 +214,22 @@ void APS5_VABI scePthreadExit(void* retval) { ReleaseThread(self); _endthreadex(0); #else + if (!currentThread) + throw std::runtime_error("scePthreadExit: current thread is not registered"); + FinishThread(currentThread, retval); pthread_exit(retval); #endif __builtin_unreachable(); } Pthread APS5_VABI scePthreadSelf() { -#ifdef _WIN32 + if (currentThread) return currentThread; + adoptedThread = std::make_unique(); + adoptedThread->_detached = true; + adoptedThread->_adopted = true; + adoptedThread->threadId = std::this_thread::get_id(); + currentThread = adoptedThread.get(); return currentThread; -#else - return nullptr; -#endif } void APS5_VABI scePthreadYield() { diff --git a/core/libs/tests/GuestCondTimedwait.cpp b/core/libs/tests/GuestCondTimedwait.cpp new file mode 100644 index 000000000..52b734e0a --- /dev/null +++ b/core/libs/tests/GuestCondTimedwait.cpp @@ -0,0 +1,52 @@ +#include "SceTypes.hpp" +#include +#include +#include + +extern "C" { +int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name); +int APS5_VABI scePthreadJoin(Pthread thread, void** retval); +int APS5_VABI scePthreadMutexInit(PthreadMutex* mutex, const PthreadMutexattr* attr, const char* name); +int APS5_VABI scePthreadMutexDestroy(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexLock(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexUnlock(PthreadMutex* mutex); +int APS5_VABI scePthreadCondInit(PthreadCond* cond, const PthreadCondattr* attr, const char* name); +int APS5_VABI scePthreadCondDestroy(PthreadCond* cond); +int APS5_VABI scePthreadCondTimedwait(PthreadCond* cond, PthreadMutex* mutex, unsigned int usec); +} + +static constexpr int SCE_OK = 0; +static constexpr int SCE_KERNEL_ERROR_ETIMEDOUT = static_cast(0x8002003C); + +static void Require(bool value) { if (!value) std::abort(); } + +struct Context { + PthreadMutex mutex = nullptr; + std::atomic acquired{false}; +}; + +static void* APS5_VABI Contender(void* arg) { + auto& context = *static_cast(arg); + Require(scePthreadMutexLock(&context.mutex) == SCE_OK); + context.acquired.store(true); + Require(scePthreadMutexUnlock(&context.mutex) == SCE_OK); + return nullptr; +} + +int main() { + Context context; + PthreadCond cond = nullptr; + Require(scePthreadMutexInit(&context.mutex, nullptr, nullptr) == SCE_OK); + Require(scePthreadCondInit(&cond, nullptr, nullptr) == SCE_OK); + + Require(scePthreadMutexLock(&context.mutex) == SCE_OK); + Pthread thread = nullptr; + Require(scePthreadCreate(&thread, nullptr, Contender, &context, nullptr) == SCE_OK); + while (!context.acquired.load()) + Require(scePthreadCondTimedwait(&cond, &context.mutex, 1000) == SCE_KERNEL_ERROR_ETIMEDOUT); + Require(scePthreadMutexUnlock(&context.mutex) == SCE_OK); + Require(scePthreadJoin(thread, nullptr) == SCE_OK); + + Require(scePthreadCondDestroy(&cond) == SCE_OK); + Require(scePthreadMutexDestroy(&context.mutex) == SCE_OK); +} diff --git a/core/libs/tests/GuestThreadSelf.cpp b/core/libs/tests/GuestThreadSelf.cpp new file mode 100644 index 000000000..585745cbd --- /dev/null +++ b/core/libs/tests/GuestThreadSelf.cpp @@ -0,0 +1,75 @@ +#include "SceTypes.hpp" +#include +#include + +extern "C" { +int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name); +int APS5_VABI scePthreadJoin(Pthread thread, void** retval); +int APS5_VABI scePthreadDetach(Pthread thread); +void APS5_VABI scePthreadExit(void* retval); +Pthread APS5_VABI scePthreadSelf(); +int APS5_VABI scePthreadMutexattrInit(PthreadMutexattr* attr); +int APS5_VABI scePthreadMutexattrDestroy(PthreadMutexattr* attr); +int APS5_VABI scePthreadMutexattrSettype(PthreadMutexattr* attr, int type); +int APS5_VABI scePthreadMutexInit(PthreadMutex* mutex, const PthreadMutexattr* attr, const char* name); +int APS5_VABI scePthreadMutexDestroy(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexLock(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexUnlock(PthreadMutex* mutex); +} + +static constexpr int SCE_OK = 0; +static constexpr int SCE_KERNEL_ERROR_EINVAL = 0x80020016; +static constexpr int SCE_KERNEL_ERROR_EPERM = 0x80020001; +static constexpr int MUTEX_TYPE_RECURSIVE = 2; +static constexpr std::intptr_t WorkerRetval = 0x1234; + +static void Require(bool value) { if (!value) std::abort(); } + +struct WorkerContext { + Pthread thread = nullptr; + Pthread selfFromWorker = nullptr; + PthreadMutex* mutex = nullptr; + int unlockResult = 0; +}; + +static void* APS5_VABI Worker(void* arg) { + auto& context = *static_cast(arg); + context.selfFromWorker = scePthreadSelf(); + context.unlockResult = scePthreadMutexUnlock(context.mutex); + scePthreadExit(reinterpret_cast(WorkerRetval)); + return nullptr; +} + +int main() { + const Pthread mainSelf = scePthreadSelf(); + Require(mainSelf != nullptr); + Require(scePthreadSelf() == mainSelf); + Require(scePthreadJoin(mainSelf, nullptr) == SCE_KERNEL_ERROR_EINVAL); + Require(scePthreadDetach(mainSelf) == SCE_KERNEL_ERROR_EINVAL); + + PthreadMutexattr attr = nullptr; + Require(scePthreadMutexattrInit(&attr) == SCE_OK); + Require(scePthreadMutexattrSettype(&attr, MUTEX_TYPE_RECURSIVE) == SCE_OK); + PthreadMutex mutex = nullptr; + Require(scePthreadMutexInit(&mutex, &attr, nullptr) == SCE_OK); + Require(scePthreadMutexattrDestroy(&attr) == SCE_OK); + + Require(scePthreadMutexLock(&mutex) == SCE_OK); + + WorkerContext context; + context.mutex = &mutex; + Require(scePthreadCreate(&context.thread, nullptr, Worker, &context, nullptr) == SCE_OK); + Require(context.thread != nullptr); + Require(context.thread != mainSelf); + + void* result = nullptr; + Require(scePthreadJoin(context.thread, &result) == SCE_OK); + Require(reinterpret_cast(result) == WorkerRetval); + Require(context.selfFromWorker != nullptr); + Require(context.selfFromWorker == context.thread); + Require(context.selfFromWorker != mainSelf); + Require(context.unlockResult == SCE_KERNEL_ERROR_EPERM); + + Require(scePthreadMutexUnlock(&mutex) == SCE_OK); + Require(scePthreadMutexDestroy(&mutex) == SCE_OK); +}