From 241053ad9f470b7e536ad942da8b907c6f9ba334 Mon Sep 17 00:00:00 2001 From: Ondra97 <47064087+Ondra97@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:34:42 +0200 Subject: [PATCH] fix(libkernel): preserve condition signals across cancellation --- .../prx/libkernel/Pthread/include/Pthread.hpp | 3 +- core/libs/prx/libkernel/Pthread/src/Cond.cpp | 59 +++--- .../libs/tests/GuestPthreadCancelRecovery.cpp | 186 ++++++++++++++++++ docs/dev/debt/libkernel.md | 2 +- 4 files changed, 223 insertions(+), 27 deletions(-) diff --git a/core/libs/prx/libkernel/Pthread/include/Pthread.hpp b/core/libs/prx/libkernel/Pthread/include/Pthread.hpp index e22426757f30..6b1c2c0ff1f5 100644 --- a/core/libs/prx/libkernel/Pthread/include/Pthread.hpp +++ b/core/libs/prx/libkernel/Pthread/include/Pthread.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -61,7 +62,7 @@ struct PthreadCondattrPrivate { }; struct PthreadCondPrivate { - TimedWait::Condition _cv; + std::shared_ptr _cv = std::make_shared(); int _clockid = 0; }; diff --git a/core/libs/prx/libkernel/Pthread/src/Cond.cpp b/core/libs/prx/libkernel/Pthread/src/Cond.cpp index d576c64ff034..c177d2a5ad64 100644 --- a/core/libs/prx/libkernel/Pthread/src/Cond.cpp +++ b/core/libs/prx/libkernel/Pthread/src/Cond.cpp @@ -2,6 +2,7 @@ #include "prx/libkernel/Pthread/include/Mutex.hpp" #include "prx/libkernel/Pthread/include/Cond.hpp" #include "prx/libkernel/Pthread/include/Cancel.hpp" +#include "prx/libkernel/Pthread/include/ThreadLifecycle.hpp" #include "prx/libkernel/Pthread/Posix/Common.hpp" #include "prx/libkernel/KernelErrors.hpp" #include "prx/libkernel/Time/include/Time.hpp" @@ -13,6 +14,7 @@ #include #include #include +#include namespace { @@ -31,21 +33,20 @@ PthreadMutex adaptiveInitializer() { return reinterpret_cast(std::uintptr_t{1}); } -PthreadCond resolveCond(PthreadCond* cond) { +std::shared_ptr resolveCond(PthreadCond* cond, int* clockId = nullptr) { if (!cond) throw std::invalid_argument("Condition variable pointer is null"); - std::atomic_ref slot(*cond); - if (const auto current = slot.load(std::memory_order_acquire); current && current != destroyedCond()) - return current; std::lock_guard lock(condInitializationMutex); - const auto current = slot.load(std::memory_order_acquire); + std::atomic_ref slot(*cond); + auto current = slot.load(std::memory_order_acquire); if (current == destroyedCond()) throw std::runtime_error("Condition variable has been destroyed"); - if (current) - return current; - auto* created = new PthreadCondPrivate(); - slot.store(created, std::memory_order_release); - return created; + if (!current) { + current = new PthreadCondPrivate(); + slot.store(current, std::memory_order_release); + } + if (clockId) *clockId = current->_clockid; + return current->_cv; } int mutexOwnership(PthreadMutex* mutex, PthreadMutex* owned) { @@ -73,8 +74,16 @@ struct WakingLock { } }; -int waitUntil(PthreadCond* cond, PthreadMutex* mutex, std::optional deadlineNanos, const void* caller) { - auto* c = resolveCond(cond); +int finishWait(std::shared_ptr& condition, bool timedOut) { + if (ThreadCancel::Requested()) { + condition->NotifyOne(); + condition.reset(); + scePthreadExit(GUEST_CANCELED); + } + return timedOut ? sceTimedOut : 0; +} + +int waitUntil(std::shared_ptr condition, PthreadMutex* mutex, std::optional deadlineNanos, const void* caller) { PthreadMutex m = nullptr; if (const int error = mutexOwnership(mutex, &m)) return error; @@ -92,32 +101,32 @@ int waitUntil(PthreadCond* cond, PthreadMutex* mutex, std::optional_owner.store(std::thread::id{}, std::memory_order_release); for (int level = 1; level < previousCount; ++level) m->_rmtx.unlock(); - timedOut = !ThreadCancel::Wait(c->_cv, lock, deadlineNanos); + timedOut = !ThreadCancel::Wait(*condition, lock, deadlineNanos); for (int level = 1; level < previousCount; ++level) m->_rmtx.lock(); m->_owner.store(std::this_thread::get_id(), std::memory_order_release); m->_count = previousCount; lock.release(); - ThreadCancel::Check(); - return timedOut ? sceTimedOut : 0; + return finishWait(condition, timedOut); } WakingLock waking{m, m->_mtx}; std::unique_lock lock(waking, std::adopt_lock); m->_owner.store(std::thread::id{}, std::memory_order_release); - timedOut = !ThreadCancel::Wait(c->_cv, lock, deadlineNanos); + timedOut = !ThreadCancel::Wait(*condition, lock, deadlineNanos); m->_owner.store(std::this_thread::get_id(), std::memory_order_release); lock.release(); - ThreadCancel::Check(); - return timedOut ? sceTimedOut : 0; + return finishWait(condition, timedOut); } } int CondOperations::AbsoluteTimedwait(PthreadCond* cond, PthreadMutex* mutex, const KernelTimespec* abstime) { KernelUseconds usec = 0; - if (!PosixThread::RelativeMicroseconds(resolveCond(cond)->_clockid, abstime, &usec)) + int clockId = 0; + auto condition = resolveCond(cond, &clockId); + if (!PosixThread::RelativeMicroseconds(clockId, abstime, &usec)) throw std::invalid_argument("Invalid absolute condition variable timeout"); - return waitUntil(cond, mutex, TimedWait::DeadlineNanos(usec), __builtin_return_address(0)); + return waitUntil(std::move(condition), mutex, TimedWait::DeadlineNanos(usec), __builtin_return_address(0)); } extern "C" { @@ -169,27 +178,27 @@ int APS5_VABI scePthreadCondDestroy(PthreadCond* cond) { } int APS5_VABI scePthreadCondSignal(PthreadCond* cond) { - resolveCond(cond)->_cv.NotifyOne(); + resolveCond(cond)->NotifyOne(); return 0; } int APS5_VABI scePthreadCondBroadcast(PthreadCond* cond) { - resolveCond(cond)->_cv.NotifyAll(); + resolveCond(cond)->NotifyAll(); return 0; } int APS5_VABI scePthreadCondSignalto(PthreadCond* cond, Pthread thread) { (void)thread; - resolveCond(cond)->_cv.NotifyAll(); + resolveCond(cond)->NotifyAll(); return 0; } int APS5_VABI scePthreadCondWait(PthreadCond* cond, PthreadMutex* mutex) { - return waitUntil(cond, mutex, std::nullopt, __builtin_return_address(0)); + return waitUntil(resolveCond(cond), mutex, std::nullopt, __builtin_return_address(0)); } int APS5_VABI scePthreadCondTimedwait(PthreadCond* cond, PthreadMutex* mutex, KernelUseconds usec) { - return waitUntil(cond, mutex, TimedWait::DeadlineNanos(usec), __builtin_return_address(0)); + return waitUntil(resolveCond(cond), mutex, TimedWait::DeadlineNanos(usec), __builtin_return_address(0)); } } diff --git a/core/libs/tests/GuestPthreadCancelRecovery.cpp b/core/libs/tests/GuestPthreadCancelRecovery.cpp index 59e6056c8516..297f761a92d6 100644 --- a/core/libs/tests/GuestPthreadCancelRecovery.cpp +++ b/core/libs/tests/GuestPthreadCancelRecovery.cpp @@ -1,10 +1,21 @@ #include "SceTypes.hpp" +#include "prx/libkernel/Pthread/include/Pthread.hpp" #include #include +#include #include #include #include #include +#include +#include + +#ifdef _WIN32 +#include +#else +#include +#include +#endif extern "C" { int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name); @@ -17,6 +28,9 @@ int APS5_VABI scePthreadKeyCreate(PthreadKey* key, pthread_key_destructor_func_t int APS5_VABI scePthreadKeyDelete(PthreadKey key); int APS5_VABI scePthreadSetspecific(PthreadKey key, void* value); int APS5_VABI scePthreadMutexInit(PthreadMutex* mutex, const PthreadMutexattr* attr, const char* name); +int APS5_VABI scePthreadMutexattrInit(PthreadMutexattr* attr); +int APS5_VABI scePthreadMutexattrDestroy(PthreadMutexattr* attr); +int APS5_VABI scePthreadMutexattrSettype(PthreadMutexattr* attr, int type); int APS5_VABI scePthreadMutexDestroy(PthreadMutex* mutex); int APS5_VABI scePthreadMutexLock(PthreadMutex* mutex); int APS5_VABI scePthreadMutexUnlock(PthreadMutex* mutex); @@ -24,6 +38,10 @@ int APS5_VABI scePthreadCondInit(PthreadCond* cond, const PthreadCondattr* attr, int APS5_VABI scePthreadCondDestroy(PthreadCond* cond); int APS5_VABI scePthreadCondSignal(PthreadCond* cond); int APS5_VABI scePthreadCondTimedwait(PthreadCond* cond, PthreadMutex* mutex, KernelUseconds usec); +int APS5_VABI scePthreadCondWait(PthreadCond* cond, PthreadMutex* mutex); +int APS5_VABI pthread_cond_wait_nid_postfix(PthreadCond* cond, PthreadMutex* mutex); +int APS5_VABI pthread_cond_timedwait_nid_postfix(PthreadCond* cond, PthreadMutex* mutex, const KernelTimespec* abstime); +int APS5_VABI clock_gettime_nid_postfix(int clockId, KernelTimespec* time); int APS5_VABI scePthreadSemInit(PthreadSem* sem, int flag, unsigned int value, const char* name); int APS5_VABI scePthreadSemDestroy(PthreadSem* sem); int APS5_VABI scePthreadSemWait(PthreadSem* sem); @@ -226,6 +244,166 @@ static void ConditionRecovery() { Require(scePthreadMutexDestroy(&mutex) == 0); } +enum class ConditionWaitMode { SceTimed, Sce, PosixTimed, Posix }; + +struct SignaledCondition { + PthreadMutex mutex = nullptr; + PthreadCond condition = nullptr; + PthreadCond* slot = &condition; + ConditionWaitMode mode = ConditionWaitMode::SceTimed; + int lockCount = 1; + bool cancellationDisabled = false; + bool released = false; + std::atomic ready{0}; + std::array results{-1, -1}; + std::array cleaned{}; +}; + +struct SignaledConditionWorker { + SignaledCondition* context; + int index; +}; + +static void APS5_VABI UnlockSignaledCondition(void* value) { + auto& worker = *static_cast(value); + auto& context = *worker.context; + for (int level = 0; level < context.lockCount; ++level) Require(scePthreadMutexUnlock(&context.mutex) == 0); + Require(scePthreadMutexUnlock(&context.mutex) == static_cast(0x80020001)); + context.cleaned[worker.index] = true; +} + +static void* APS5_VABI SignaledConditionWaiter(void* value) { + auto& worker = *static_cast(value); + auto& context = *worker.context; + if (context.cancellationDisabled) Require(scePthreadSetcancelstate(CANCEL_DISABLE, nullptr) == 0); + for (int level = 0; level < context.lockCount; ++level) Require(scePthreadMutexLock(&context.mutex) == 0); + std::uintptr_t cleanup[8]; + __pthread_cleanup_push_imp_nid_postfix(UnlockSignaledCondition, &worker, cleanup); + context.ready.fetch_add(1); + int result = 0; + do { + switch (context.mode) { + case ConditionWaitMode::SceTimed: + result = scePthreadCondTimedwait(context.slot, &context.mutex, 2000000); + break; + case ConditionWaitMode::Sce: + result = scePthreadCondWait(context.slot, &context.mutex); + break; + case ConditionWaitMode::PosixTimed: { + KernelTimespec deadline{}; + Require(clock_gettime_nid_postfix(0, &deadline) == 0); + deadline.tv_sec += 2; + result = pthread_cond_timedwait_nid_postfix(context.slot, &context.mutex, &deadline); + break; + } + case ConditionWaitMode::Posix: + result = pthread_cond_wait_nid_postfix(context.slot, &context.mutex); + break; + } + } while (result == 0 && !context.released); + context.results[worker.index] = result; + __pthread_cleanup_pop_imp_nid_postfix(1); + return nullptr; +} + +static bool ConditionRegistered(Pthread thread) { + const std::lock_guard lock(thread->cancelLock); + return thread->cancelWait != nullptr; +} + +static void InitializeSignaledCondition(SignaledCondition& context, bool recursive) { + PthreadMutexattr attr = nullptr; + Require(scePthreadMutexattrInit(&attr) == 0); + Require(scePthreadMutexattrSettype(&attr, recursive ? 2 : 1) == 0); + Require(scePthreadMutexInit(&context.mutex, &attr, nullptr) == 0); + Require(scePthreadMutexattrDestroy(&attr) == 0); + context.lockCount = recursive ? 3 : 1; + Require(scePthreadCondInit(context.slot, nullptr, nullptr) == 0); +} + +static void CanceledConditionSignal(ConditionWaitMode mode, bool recursive) { + SignaledCondition context; + context.mode = mode; + InitializeSignaledCondition(context, recursive); + std::array workers{{{&context, 0}, {&context, 1}}}; + std::array threads{}; + for (int index = 0; index < 2; ++index) { + Require(scePthreadCreate(&threads[index], nullptr, SignaledConditionWaiter, &workers[index], nullptr) == 0); + while (context.ready.load() <= index) std::this_thread::yield(); + Require(scePthreadMutexLock(&context.mutex) == 0); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(1); + while (!ConditionRegistered(threads[index]) && std::chrono::steady_clock::now() < deadline) { + Require(scePthreadMutexUnlock(&context.mutex) == 0); + std::this_thread::yield(); + Require(scePthreadMutexLock(&context.mutex) == 0); + } + Require(ConditionRegistered(threads[index])); + Require(scePthreadMutexUnlock(&context.mutex) == 0); + } + Require(scePthreadMutexLock(&context.mutex) == 0); + context.released = true; + Require(scePthreadCondSignal(&context.condition) == 0); + int selected = -1; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(1); + while (selected < 0 && std::chrono::steady_clock::now() < deadline) { + for (int index = 0; index < 2; ++index) if (!ConditionRegistered(threads[index])) selected = index; + std::this_thread::yield(); + } + Require(selected >= 0); + Require(scePthreadCancel(threads[selected]) == 0); + Require(scePthreadMutexUnlock(&context.mutex) == 0); + Join(threads[selected], CANCELED); + Join(threads[1 - selected], nullptr); + Require(context.results[selected] == -1 && context.results[1 - selected] == 0); + Require(context.cleaned[0] && context.cleaned[1]); + Require(scePthreadCondDestroy(&context.condition) == 0); + Require(scePthreadMutexDestroy(&context.mutex) == 0); +} + +static void DestroyedSignaledCondition(ConditionWaitMode mode, bool recursive, bool cancel, bool cancellationDisabled) { + SignaledCondition context; + context.mode = mode; + context.cancellationDisabled = cancellationDisabled; +#ifdef _WIN32 + SYSTEM_INFO system{}; + GetSystemInfo(&system); + const auto bytes = static_cast(system.dwPageSize); + context.slot = static_cast(VirtualAlloc(nullptr, bytes, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE)); + Require(context.slot != nullptr); +#else + const auto bytes = static_cast(sysconf(_SC_PAGESIZE)); + context.slot = static_cast(mmap(nullptr, bytes, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); + Require(context.slot != MAP_FAILED); +#endif + *context.slot = nullptr; + InitializeSignaledCondition(context, recursive); + const std::weak_ptr retained = (*context.slot)->_cv; + SignaledConditionWorker worker{&context, 0}; + Pthread thread = nullptr; + Require(scePthreadCreate(&thread, nullptr, SignaledConditionWaiter, &worker, nullptr) == 0); + while (context.ready.load() == 0) std::this_thread::yield(); + Require(scePthreadMutexLock(&context.mutex) == 0); + context.released = true; + Require(scePthreadCondSignal(context.slot) == 0); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(1); + while (ConditionRegistered(thread) && std::chrono::steady_clock::now() < deadline) std::this_thread::yield(); + Require(!ConditionRegistered(thread)); + Require(scePthreadCondDestroy(context.slot) == 0); + Require(!retained.expired()); +#ifdef _WIN32 + Require(VirtualFree(context.slot, 0, MEM_RELEASE) != 0); +#else + Require(munmap(context.slot, bytes) == 0); +#endif + if (cancel) Require(scePthreadCancel(thread) == 0); + Require(scePthreadMutexUnlock(&context.mutex) == 0); + Join(thread, cancel && !cancellationDisabled ? CANCELED : nullptr); + Require(context.cleaned[0]); + Require(context.results[0] == (cancel && !cancellationDisabled ? -1 : 0)); + Require(retained.expired()); + Require(scePthreadMutexDestroy(&context.mutex) == 0); +} + static void* APS5_VABI HeldTarget(void*) { CheckDefaults(); targetRelease.wait(false); @@ -274,6 +452,14 @@ int main() { } Require(scePthreadKeyDelete(key) == 0); ConditionRecovery(); + for (const auto mode : {ConditionWaitMode::SceTimed, ConditionWaitMode::Sce, ConditionWaitMode::PosixTimed, ConditionWaitMode::Posix}) { + for (bool recursive : {false, true}) { + CanceledConditionSignal(mode, recursive); + DestroyedSignaledCondition(mode, recursive, true, false); + DestroyedSignaledCondition(mode, recursive, false, false); + DestroyedSignaledCondition(mode, recursive, true, true); + } + } disabled = true; ConditionRecovery(); JoinRecovery(); diff --git a/docs/dev/debt/libkernel.md b/docs/dev/debt/libkernel.md index efa915680902..a2afebc98fbd 100644 --- a/docs/dev/debt/libkernel.md +++ b/docs/dev/debt/libkernel.md @@ -51,7 +51,7 @@ - [sceKernelSyncOnAddressWait](../../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - the only known caller passes a null timeout and a name string as the fourth argument; the timeout is assumed to point to microseconds like the other kernel waits, and the name is ignored - [sceKernelSyncOnAddressWait8 / sceKernelSyncOnAddressWait16 / sceKernelSyncOnAddressWait32 / sceKernelSyncOnAddressWait64](../../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - libc's `__std_atomic_wait_direct_8/16/32/64` pass a microsecond timeout or null and ignore the result; the by-value comparand, return values and the 2- and 8-byte alignment of Wait16 and Wait64 are assumed - [scePthreadRwlockDestroy, pthread_rwlock_destroy](../../../core/libs/prx/libkernel/Pthread/src/Rwlock.cpp) (libkernel) - measured on a PS5 (firmware 13.60): destroying a rwlock returns 0 whether it is free or held for reading or writing, and leaves 1 in the handle; every call on that handle then returns `EINVAL` until it is initialized again, and a zero handle is initialized on first use by the `scePthreadRwlock` calls as by the POSIX ones. A rwlock destroyed while held is not freed, so its memory leaks. Destroying a rwlock while another thread is acquiring it is not guarded -- [scePthreadCondWait / scePthreadCondTimedwait](../../../core/libs/prx/libkernel/Pthread/src/Cond.cpp) (libkernel) - before waiting, FreeBSD libthr's [`cond_wait_common`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_cond.c#L1992-L2029) checks ownership through [`_mutex_owned`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_mutex.c#L1166-L1178): an unowned mutex returns `EPERM`, while its destroyed marker returns `EINVAL`. An untouched adaptive static initializer (slot value 1) therefore returns `EPERM`; the test covers this, an uninitialized slot, a mutex owned by another thread, and the destroyed marker for both timed and untimed sce and POSIX waits. A recursive mutex locked several times is released completely for the wait and relocked to the same depth afterwards, as FreeBSD libthr does (`_mutex_cv_unlock`, `_mutex_cv_lock`). These FreeBSD-derived behaviors remain unverified on console hardware +- [scePthreadCondWait / scePthreadCondTimedwait](../../../core/libs/prx/libkernel/Pthread/src/Cond.cpp) (libkernel) - before waiting, FreeBSD libthr's [`cond_wait_common`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_cond.c#L1992-L2029) checks ownership through [`_mutex_owned`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_mutex.c#L1166-L1178): an unowned mutex returns `EPERM`, while its destroyed marker returns `EINVAL`. An untouched adaptive static initializer (slot value 1) therefore returns `EPERM`; the test covers this, an uninitialized slot, a mutex owned by another thread, and the destroyed marker for both timed and untimed sce and POSIX waits. A recursive mutex locked several times is released completely for the wait and relocked to the same depth afterwards, as FreeBSD libthr does (`_mutex_cv_unlock`, `_mutex_cv_lock`). These FreeBSD-derived behaviors remain unverified on console hardware. Cancellation follows the [POSIX condition wait contract](https://pubs.opengroup.org/onlinepubs/9799919799/functions/pthread_cond_clockwait.html): the original mutex and recursive depth are restored before cleanup, and a canceled waiter forwards a wake to the remaining waiters. The host condition state is retained across mutex reacquisition so the guest can destroy and free its condition after notification; cancellation ordering and signal replacement are not measured on a console - [sceKernelAioSubmitReadCommandsMultiple / sceKernelAioSubmitWriteCommandsMultiple / sceKernelAioWaitRequests / sceKernelAioPollRequests / sceKernelAioCancelRequest / sceKernelAioCancelRequests / sceKernelAioDeleteRequests](../../../core/libs/prx/libkernel/Aio/src/Aio.cpp) (libkernel) - behaviour from shadPS4 (one id per request for the Multiple submits, cancel turns any request aborted and id 0 reports processing, wait mode 2 returns once one request completed, poll copies every state without waiting); the 128 requests or ids per batch limit is from SharpProspero, with an unknown error, so larger batches throw; wait modes other than 1 (and) and 2 (or) throw; null pointers, negative counts and invalid ids return the codes the single-request siblings use, checked before any state is written - [scePthreadAttr setters and getters](../../../core/libs/prx/libkernel/Pthread/src/Attr.cpp) (libkernel) - measured on a PS5 (firmware 13.60): a null or uninitialized attribute returns `EINVAL`, as does a null output of the stack size, detach state and scheduling parameter getters; a policy other than 1 and 3, an inheritance other than 0 and 4, or a priority outside 256 to 767 (or a null parameter) returns `ENOTSUP` (`0x8002002D`); a detach state other than 0 and 1 and a stack size below 16384 return `EINVAL`; any affinity mask is accepted. A null attribute pointer to `scePthreadAttrInit` and a null output of `scePthreadAttrGetaffinity` crashed the payload, so they still throw. A new attribute holds a 64 KiB stack size on the console and 1 MiB here, which the host code that runs on guest threads needs - [sceKernelGetAvailableCpumask](../../../core/libs/prx/libkernel/Pthread/src/Thread.cpp) (libkernel) - returns the default thread affinity, 0x1FFF (CPUs 0 to 12), as KytyPS5 does. Titles pin threads within it (PPSA02664 passes 0x1FFB, Hades II PPSA36082 0x3 and 0x3F); whether CPU 13 is available to a title is not confirmed