fix(libkernel): refuse to detach or rejoin a thread that is being joined

core/libs/prx/libkernel/CMakeLists.txt

@@ -587,6 +587,16 @@ if(BUILD_TESTING)
         set_tests_properties(guest_pthread_start PROPERTIES TIMEOUT 60 RUN_SERIAL TRUE ENVIRONMENT_MODIFICATION
             "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
     endif()
+    add_test_executable(guest_pthread_join_detach_tests ${LIBS_INCLUDE_DIR}/tests/GuestPthreadJoinDetach.cpp)
+    target_include_directories(guest_pthread_join_detach_tests PRIVATE ${LIBS_INCLUDE_DIR})
+    target_link_libraries(guest_pthread_join_detach_tests PRIVATE libkernel libc)
+    configure_windows_unwind(guest_pthread_join_detach_tests)
+    add_test(NAME guest_pthread_join_detach COMMAND guest_pthread_join_detach_tests)
+    set_tests_properties(guest_pthread_join_detach PROPERTIES TIMEOUT 20)
+    if(WIN32)
+        set_tests_properties(guest_pthread_join_detach PROPERTIES ENVIRONMENT_MODIFICATION
+            "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
+    endif()
     add_test_executable(guest_pthread_attr_errors_tests ${LIBS_INCLUDE_DIR}/tests/GuestPthreadAttrErrors.cpp)
     target_include_directories(guest_pthread_attr_errors_tests PRIVATE ${LIBS_INCLUDE_DIR})
     target_link_libraries(guest_pthread_attr_errors_tests PRIVATE libkernel libc)

core/libs/prx/libkernel/Pthread/include/Pthread.hpp

@@ -144,6 +144,7 @@ struct PthreadPrivate {
     bool _adopted;
     Pthread* _handleStorage;
     std::atomic<bool> _joined;
+        bool _joining = false;
     std::mutex _join_mtx;
     TimedWait::Condition _join_cv;
     std::atomic<bool> cancelPending{false};

core/libs/prx/libkernel/Pthread/src/Thread.cpp

@@ -886,9 +886,22 @@ int APS5_VABI scePthreadJoin(Pthread thread, void** retval) {
     ThreadCancel::Check();
     {
         std::unique_lock lock(thread->_join_mtx);
-        ThreadCancel::WaitUntil(thread->_join_cv, lock, std::nullopt, [&] { return thread->_finished.load(); });
+        if (thread->_detached) return SCE_KERNEL_ERROR_EINVAL;
+        if (thread->_joining) return SCE_KERNEL_ERROR_EOPNOTSUPP;
+        thread->_joining = true;
+        while (!thread->_finished.load()) {
+            ThreadCancel::Wait(thread->_join_cv, lock, std::nullopt);
+            if (ThreadCancel::Requested()) {
+                thread->_joining = false;
+                lock.unlock();
+                ThreadCancel::Check();
+                lock.lock();
+                thread->_joining = true;
+            }
+        }
+        thread->_joining = false;
+        if (thread->_joined.exchange(true, std::memory_order_acq_rel)) return SCE_KERNEL_ERROR_ESRCH;
     }
-    if (thread->_joined.exchange(true, std::memory_order_acq_rel)) return SCE_KERNEL_ERROR_ESRCH;
 #ifdef _WIN32
     if (WaitForSingleObject(thread->nativeHandle, INFINITE) != WAIT_OBJECT_0)
         throw std::system_error(GetLastError(), std::system_category(), "Joining guest thread");
@@ -905,9 +918,11 @@ int APS5_VABI scePthreadJoin(Pthread thread, void** retval) {
 
 int APS5_VABI scePthreadDetach(Pthread thread) {
     if (!thread) return SCE_KERNEL_ERROR_EINVAL;
-    if (thread->_detached) return SCE_KERNEL_ERROR_EINVAL;
-    if (thread->_joined.load(std::memory_order_acquire)) return SCE_KERNEL_ERROR_EINVAL;
-    thread->_detached = true;
+    {
+        std::lock_guard lock(thread->_join_mtx);
+        if (thread->_detached || thread->_joining || thread->_joined.load(std::memory_order_acquire)) return SCE_KERNEL_ERROR_EINVAL;
+        thread->_detached = true;
+    }
 #ifdef _WIN32
     ReleaseThread(thread);
 #else

core/libs/tests/GuestPthreadJoinDetach.cpp

@@ -0,0 +1,75 @@
+#include "prx/libkernel/Pthread/include/Pthread.hpp"
+#include "prx/libkernel/KernelErrors.hpp"
+#include <atomic>
+#include <chrono>
+#include <cstdio>
+#include <cstdlib>
+#include <mutex>
+#include <thread>
+
+extern "C" int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name);
+extern "C" int APS5_VABI scePthreadJoin(Pthread thread, void** retval);
+extern "C" int APS5_VABI scePthreadDetach(Pthread thread);
+
+static constexpr int SCE_OK = 0;
+
+static void Require(bool condition, const char* message) {
+    if (!condition) {
+        std::fprintf(stderr, "%s\n", message);
+        std::exit(1);
+    }
+}
+
+struct Joinee {
+    Pthread thread = nullptr;
+    std::atomic<bool> release{false};
+};
+
+struct Joiner {
+    Pthread target = nullptr;
+    void* result = nullptr;
+    int status = -1;
+};
+
+static void* APS5_VABI Wait(void* opaque) {
+    auto* joinee = static_cast<Joinee*>(opaque);
+    while (!joinee->release.load()) std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    return opaque;
+}
+
+static void* APS5_VABI Join(void* opaque) {
+    auto* joiner = static_cast<Joiner*>(opaque);
+    joiner->status = scePthreadJoin(joiner->target, &joiner->result);
+    return nullptr;
+}
+
+static bool Joining(Pthread thread) {
+    std::lock_guard lock(thread->_join_mtx);
+    return thread->_joining;
+}
+
+int main() {
+    Joinee joinee;
+    Require(scePthreadCreate(&joinee.thread, nullptr, Wait, &joinee, "joinee") == SCE_OK, "create the joinee");
+    Joiner joiner{joinee.thread};
+    Pthread joinerThread = nullptr;
+    Require(scePthreadCreate(&joinerThread, nullptr, Join, &joiner, "joiner") == SCE_OK, "create the joiner");
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
+    while (!Joining(joinee.thread)) {
+        Require(std::chrono::steady_clock::now() < deadline, "the joiner waits in scePthreadJoin");
+        std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    }
+    Require(scePthreadDetach(joinee.thread) == SCE_KERNEL_ERROR_EINVAL, "detaching a thread with a joiner returns EINVAL");
+    Require(scePthreadJoin(joinee.thread, nullptr) == SCE_KERNEL_ERROR_EOPNOTSUPP, "a second joiner gets EOPNOTSUPP");
+    joinee.release.store(true);
+    Require(scePthreadJoin(joinerThread, nullptr) == SCE_OK, "join the joiner");
+    Require(joiner.status == SCE_OK && joiner.result == &joinee, "the first join completes with the return value");
+
+    Joinee detached;
+    Require(scePthreadCreate(&detached.thread, nullptr, Wait, &detached, "detached") == SCE_OK, "create a thread to detach");
+    Require(scePthreadDetach(detached.thread) == SCE_OK, "detach a thread without a joiner");
+    Require(scePthreadJoin(detached.thread, nullptr) == SCE_KERNEL_ERROR_EINVAL, "joining a detached thread returns EINVAL");
+    detached.release.store(true);
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    return 0;
+}

docs/dev/debt/libkernel.md

@@ -148,7 +148,7 @@
 - [sceKernelRaiseException](../../../core/libs/prx/libkernel/System/src/Exception.cpp) on Windows runs the handler wherever the target thread was interrupted. It waits only while the thread is inside winpthreads or restoring a context through `NtContinue`, and throws if that lasts 1 s, so a handler that calls into other host code the thread was interrupted in, such as the C runtime's heap or a lock of the emulator, can deadlock on it or reenter it. A thread that holds the address-wait lock of `sceKernelSyncOnAddressWait`, `sceKernelSyncOnAddressWake` and `_umtx_op`, or the park queue lock of the guest mutexes, gets the raise as an APC instead, which runs once it has released the lock or while it waits; the other locks of the emulator are not deferred this way
 - [sigaltstack](../../../core/libs/prx/libkernel/System/src/Signals.cpp) (libkernel) records each thread's alternate signal stack and reports it back, following FreeBSD's `kern_sigaltstack` (`EINVAL` for flags other than `SS_DISABLE`, `ENOMEM` below `MINSIGSTKSZ` 2048), but handlers never run on it, so `SS_ONSTACK` and `EPERM` are never reported
 - [sigpending](../../../core/libs/prx/libkernel/System/src/Signals.cpp) (libkernel) reports the intersection of the process-wide pending set of `sigtimedwait` and the blocked mask of `_sigprocmask`, as FreeBSD's `sys_sigpending` intersects the pending list with `td_sigmask`, and returns -1 with `EFAULT` for a null set. Because `_sigprocmask` keeps only the first 32 signals, signals 33 to 128 are never blocked and never appear in the set, and setting `SIG_IGN` (or `SIG_DFL` on a signal whose default action is to ignore) does not clear a signal that was already marked pending. PPSA99420 imports it
-- [scePthreadJoin, scePthreadCreate](../../../core/libs/prx/libkernel/Pthread/src/Thread.cpp) (libkernel) - on a PS5 (firmware 13.60) joining a thread a second time returns `ESRCH` (`0x80020003`), as it does here, but a joined thread is kept until `scePthreadCreate` publishes over the same handle storage, so one host object and its handle are retained per distinct guest handle storage location for the lifetime of the process; a guest that joins into a fresh storage location every time keeps every one of them. A null entry crashed the payload and still throws. Joining the calling thread returns `EDEADLK` and a zeroed attribute creates a default thread, as on the console. `scePthreadDetach` on a joined thread answers `EINVAL`, as FreeBSD does on a thread that is not joinable; the console answer for a detach after a join is unverified. A detached thread is still released when it ends, so joining it after it ended reads freed memory. A handle value copied to a second storage and joined there retains the object under the original storage key, so a later `scePthreadCreate` over that original storage releases an object the copy still points at, and the copy reads freed memory
+- [scePthreadJoin, scePthreadCreate](../../../core/libs/prx/libkernel/Pthread/src/Thread.cpp) (libkernel) - on a PS5 (firmware 13.60) joining a thread a second time returns `ESRCH` (`0x80020003`), as it does here, but a joined thread is kept until `scePthreadCreate` publishes over the same handle storage, so one host object and its handle are retained per distinct guest handle storage location for the lifetime of the process; a guest that joins into a fresh storage location every time keeps every one of them. A null entry crashed the payload and still throws. Joining the calling thread returns `EDEADLK` and a zeroed attribute creates a default thread, as on the console. `scePthreadDetach` on a joined thread answers `EINVAL`, as FreeBSD does on a thread that is not joinable; the console answer for a detach after a join is unverified. Detaching a thread while another thread waits to join it returns `EINVAL`, and a second concurrent join returns `EOPNOTSUPP`, as FreeBSD 11.4's [`thr_detach.c`](https://github.com/freebsd/freebsd-src/blob/releng/11.4/lib/libthr/thread/thr_detach.c) and [`thr_join.c`](https://github.com/freebsd/freebsd-src/blob/releng/11.4/lib/libthr/thread/thr_join.c) check `joiner`; before this PPSA34015 stopped once with `Joining guest thread: Access is denied` when a thread it was joining was released under the join, and which of the two calls released it was not captured. A detached thread is still released when it ends, so joining it after it ended reads freed memory. A handle value copied to a second storage and joined there retains the object under the original storage key, so a later `scePthreadCreate` over that original storage releases an object the copy still points at, and the copy reads freed memory
 - [Thread cancellation](../../../core/libs/prx/libkernel/Pthread/src/Thread.cpp) (libkernel) follows FreeBSD libthr (states and types 0/1 and 0/2, `PTHREAD_CANCELED` 1, the `_pthread_cleanup_info` node layout), not measured on the console. A request is acted on only in `scePthreadTestcancel`, the condition variable, `sem` and `scePthreadSem` waits, `scePthreadJoin` and the sleeps, and their POSIX names; file and socket calls are not cancellation points. A sleep is not interrupted: the thread exits when it ends. The asynchronous type acts like the deferred one, except at the two set calls.
 - [cpuset_setaffinity](../../../core/libs/prx/libkernel/Pthread/Posix/Thread.cpp) (libkernel) - the calling thread, another live guest thread selected by its guest TID, or the calling process is handled; other levels (ROOT/CPUSET) and other scopes (CPUSET/JAIL) throw as unmodelled, invalid levels or scopes return `EINVAL` and unknown thread or process IDs return `ESRCH`, following [FreeBSD 11.4](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/sys/kern/kern_cpuset.c#L1281-L1286) thread selection. Bits outside `sceKernelGetAvailableCpumask` are dropped, and a mask with none of those bits fails with `EDEADLK`, as FreeBSD does when the new set has no CPU in common with the parent set. Guest-TID lookup and the update hold the thread registry lock so a join cannot release the target during access. Affinity is recorded for guest CPU reporting and child inheritance, not applied to host scheduling; PS5 behaviour has not been measured
 - [cpuset_getaffinity](../../../core/libs/prx/libkernel/Pthread/Posix/Thread.cpp) (libkernel) - the calling thread, another live guest thread selected by its guest TID, or the calling process is handled; other levels (ROOT/CPUSET) and other scopes (CPUSET/JAIL) throw as unmodelled, invalid levels or scopes return `EINVAL` and unknown thread or process IDs return `ESRCH`, following [FreeBSD 11.4](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/sys/kern/kern_cpuset.c#L426-L437) guest-TID selection, not measured on PS5. For the process the mask is the union of the affinities of the live guest threads, as [FreeBSD 11.4](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/sys/kern/kern_cpuset.c#L1150-L1156) ORs the mask of every thread in the process; threads the guest did not create through libkernel are not included, and the console result is unverified