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 <noreply@anthropic.com>
This commit is contained in:
parent
908befe81d
commit
76b7f998f5
|
|
@ -240,6 +240,26 @@ if(BUILD_TESTING)
|
|||
set_tests_properties(guest_once PROPERTIES ENVIRONMENT_MODIFICATION
|
||||
"PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
|
||||
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:$<TARGET_FILE_DIR:libkernel>")
|
||||
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:$<TARGET_FILE_DIR:libkernel>")
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -51,20 +51,21 @@ struct PthreadAttrPrivate {
|
|||
struct PthreadPrivate {
|
||||
#ifdef _WIN32
|
||||
void* nativeHandle = nullptr;
|
||||
std::thread::id threadId;
|
||||
std::atomic<unsigned> references{2};
|
||||
#else
|
||||
std::thread _thr;
|
||||
#endif
|
||||
std::thread::id threadId;
|
||||
std::atomic<unsigned> references{2};
|
||||
void* stackAddress = nullptr;
|
||||
std::size_t stackSize = 0;
|
||||
std::atomic<bool> _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
|
||||
|
|
|
|||
|
|
@ -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<std::recursive_timed_mutex> 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<std::timed_mutex> 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -47,17 +47,22 @@ static void RunThread(std::unique_ptr<ThreadArgs> args) {
|
|||
FinishThread(self, entry(arg));
|
||||
}
|
||||
|
||||
#ifdef _WIN32
|
||||
static thread_local PthreadPrivate* currentThread = nullptr;
|
||||
static thread_local std::unique_ptr<PthreadPrivate> 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<ThreadArgs> guest;
|
||||
std::future<bool> 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<PthreadPrivate>();
|
||||
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() {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,52 @@
|
|||
#include "SceTypes.hpp"
|
||||
#include <atomic>
|
||||
#include <cstdint>
|
||||
#include <cstdlib>
|
||||
|
||||
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<int>(0x8002003C);
|
||||
|
||||
static void Require(bool value) { if (!value) std::abort(); }
|
||||
|
||||
struct Context {
|
||||
PthreadMutex mutex = nullptr;
|
||||
std::atomic<bool> acquired{false};
|
||||
};
|
||||
|
||||
static void* APS5_VABI Contender(void* arg) {
|
||||
auto& context = *static_cast<Context*>(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);
|
||||
}
|
||||
|
|
@ -0,0 +1,75 @@
|
|||
#include "SceTypes.hpp"
|
||||
#include <cstdint>
|
||||
#include <cstdlib>
|
||||
|
||||
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<WorkerContext*>(arg);
|
||||
context.selfFromWorker = scePthreadSelf();
|
||||
context.unlockResult = scePthreadMutexUnlock(context.mutex);
|
||||
scePthreadExit(reinterpret_cast<void*>(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<std::intptr_t>(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);
|
||||
}
|
||||
Loading…
Reference in New Issue