From 2c6d41cdd46e869a92d0955c9d6ff0abd97c0639 Mon Sep 17 00:00:00 2001 From: georgemoralis Date: Thu, 9 Apr 2026 17:46:36 +0300 Subject: [PATCH] Races and ThreadSafe issues (#4239) * used atomics for thread safety * equeue: Save filter/ident before std:move since we access them out of the locked loop * fixed kqueues memory leak * clean storage objects effectively * fixed memory leak * fix some races * fixed race condition --- src/common/object_pool.h | 4 +++- src/common/slab_heap.h | 4 +++- src/core/file_sys/fs.cpp | 1 + src/core/libraries/kernel/equeue.cpp | 19 ++++++++++--------- src/core/libraries/kernel/threads/condvar.cpp | 4 ++-- src/core/libraries/kernel/threads/mutex.cpp | 4 ++-- .../libraries/kernel/threads/thread_state.cpp | 11 +++++++---- 7 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/common/object_pool.h b/src/common/object_pool.h index 9e25e0c4c..bb94dc748 100644 --- a/src/common/object_pool.h +++ b/src/common/object_pool.h @@ -77,7 +77,9 @@ private: } void Release() { - std::destroy_n(storage.get(), used_objects); + for (size_t i = 0; i < used_objects; i++) { + storage[i].object.~T(); + } used_objects = 0; } diff --git a/src/common/slab_heap.h b/src/common/slab_heap.h index 7648ebea3..17c539c4d 100644 --- a/src/common/slab_heap.h +++ b/src/common/slab_heap.h @@ -1,9 +1,10 @@ -// SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project +// SPDX-FileCopyrightText: Copyright 2024-2026 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include +#include #include "common/assert.h" #include "common/spin_lock.h" @@ -152,6 +153,7 @@ public: } void Free(T* obj) { + std::destroy_at(obj); BaseHeap::Free(obj); } diff --git a/src/core/file_sys/fs.cpp b/src/core/file_sys/fs.cpp index 8327b51de..25e107069 100644 --- a/src/core/file_sys/fs.cpp +++ b/src/core/file_sys/fs.cpp @@ -303,6 +303,7 @@ File* HandleTable::GetResolver(int d) { } File* HandleTable::GetFile(const std::filesystem::path& host_name) { + std::scoped_lock lock{m_mutex}; for (auto* file : m_files) { if (file != nullptr && file->m_host_name == host_name) { return file; diff --git a/src/core/libraries/kernel/equeue.cpp b/src/core/libraries/kernel/equeue.cpp index 68a6dae24..29ab897fc 100644 --- a/src/core/libraries/kernel/equeue.cpp +++ b/src/core/libraries/kernel/equeue.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project +// SPDX-FileCopyrightText: Copyright 2024-2026 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include @@ -49,6 +49,10 @@ static void TimerCallback(OrbisKernelEqueue eq, const OrbisKernelEvent& kevent) // Events are uniquely identified by id and filter. bool EqueueInternal::AddEvent(EqueueEvent& event) { + // Save id and filter before event is moved into m_events. + const u64 id = event.event.ident; + const auto filter = event.event.filter; + { std::scoped_lock lock{m_mutex}; @@ -71,8 +75,6 @@ bool EqueueInternal::AddEvent(EqueueEvent& event) { } // First, check if there's already an event with the same id and filter. - u64 id = event.event.ident; - OrbisKernelEvent::Filter filter = event.event.filter; const auto& find_it = std::ranges::find_if(m_events, [id, filter](auto& ev) { return ev.event.ident == id && ev.event.filter == filter; }); @@ -106,12 +108,10 @@ bool EqueueInternal::AddEvent(EqueueEvent& event) { } // Schedule callbacks for timer events - if (event.event.filter == OrbisKernelEvent::Timer) { - return this->ScheduleEvent(event.event.ident, OrbisKernelEvent::Filter::Timer, - TimerCallback); - } else if (event.event.filter == OrbisKernelEvent::HrTimer) { - return this->ScheduleEvent(event.event.ident, OrbisKernelEvent::Filter::HrTimer, - HrTimerCallback); + if (filter == OrbisKernelEvent::Filter::Timer) { + return this->ScheduleEvent(id, OrbisKernelEvent::Filter::Timer, TimerCallback); + } else if (filter == OrbisKernelEvent::Filter::HrTimer) { + return this->ScheduleEvent(id, OrbisKernelEvent::Filter::HrTimer, HrTimerCallback); } return true; @@ -454,6 +454,7 @@ int PS4_SYSV_ABI sceKernelDeleteEqueue(OrbisKernelEqueue eq) { auto* handles = Common::Singleton::Instance(); handles->DeleteHandle(eq); + delete kqueues[eq]; kqueues.erase(eq); return ORBIS_OK; } diff --git a/src/core/libraries/kernel/threads/condvar.cpp b/src/core/libraries/kernel/threads/condvar.cpp index 9d429ed7d..e347abe0e 100644 --- a/src/core/libraries/kernel/threads/condvar.cpp +++ b/src/core/libraries/kernel/threads/condvar.cpp @@ -30,8 +30,8 @@ static int CondInit(PthreadCondT* cond, const PthreadCondAttrT* cond_attr, const if (name) { cvp->name = name; } else { - static int CondId = 0; - cvp->name = fmt::format("Cond{}", CondId++); + static std::atomic CondId{0}; + cvp->name = fmt::format("Cond{}", CondId.fetch_add(1)); } if (cond_attr == nullptr || *cond_attr == nullptr) { diff --git a/src/core/libraries/kernel/threads/mutex.cpp b/src/core/libraries/kernel/threads/mutex.cpp index 51d2d3bcd..65b13f799 100644 --- a/src/core/libraries/kernel/threads/mutex.cpp +++ b/src/core/libraries/kernel/threads/mutex.cpp @@ -60,8 +60,8 @@ static s32 MutexInit(PthreadMutexT* mutex, const PthreadMutexAttr* mutex_attr, c if (name) { pmutex->name = name; } else { - static s32 MutexId = 0; - pmutex->name = fmt::format("Mutex{}", MutexId++); + static std::atomic MutexId{0}; + pmutex->name = fmt::format("Mutex{}", MutexId.fetch_add(1)); } pmutex->m_flags = PthreadMutexFlags(attr->m_type); diff --git a/src/core/libraries/kernel/threads/thread_state.cpp b/src/core/libraries/kernel/threads/thread_state.cpp index a8ad0e322..413ba5f87 100644 --- a/src/core/libraries/kernel/threads/thread_state.cpp +++ b/src/core/libraries/kernel/threads/thread_state.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project +// SPDX-FileCopyrightText: Copyright 2024-2026 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include @@ -78,8 +78,10 @@ Pthread* ThreadState::Alloc(Pthread* curthread) { } if (!free_threads.empty()) { std::scoped_lock lk{free_thread_lock}; - thread = free_threads.back(); - free_threads.pop_back(); + if (!free_threads.empty()) { + thread = free_threads.back(); + free_threads.pop_back(); + } } } if (thread == nullptr) { @@ -123,9 +125,10 @@ void ThreadState::Free(Pthread* curthread, Pthread* thread) { TcbDtor(thread->tcb); } thread->tcb = nullptr; + auto* sleepqueue = thread->sleepqueue; std::destroy_at(thread); if (free_threads.size() >= MaxCachedThreads) { - delete thread->sleepqueue; + delete sleepqueue; thread_heap.Free(thread); total_threads.fetch_sub(1); } else {