From e3c8e4901c51e2ba172f0e1cec0a07dd56f25311 Mon Sep 17 00:00:00 2001 From: Subv Date: Fri, 5 Dec 2014 23:40:43 -0500 Subject: [PATCH 1/3] Mutex: Properly lock the mutex when a thread enters it Also resume only the next immediate thread waiting for the mutex when it is released, instead of resuming them all. --- src/core/hle/kernel/mutex.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index d07e9761b..17850c1b3 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -27,17 +27,13 @@ public: std::vector waiting_threads; ///< Threads that are waiting for the mutex std::string name; ///< Name of mutex (optional) - ResultVal SyncRequest() override { - // TODO(bunnei): ImplementMe - locked = true; - return MakeResult(false); - } - ResultVal WaitSynchronization() override { - // TODO(bunnei): ImplementMe bool wait = locked; if (locked) { Kernel::WaitCurrentThread(WAITTYPE_MUTEX, GetHandle()); + } else { + // Lock the mutex when the first thread accesses it + locked = true; } return MakeResult(wait); @@ -90,16 +86,17 @@ bool ReleaseMutex(Mutex* mutex) { MutexEraseLock(mutex); // Find the next waiting thread for the mutex... - while (!mutex->waiting_threads.empty()) { + if (mutex->waiting_threads.empty()) { + // Reset mutex lock thread handle, nothing is waiting + mutex->locked = false; + mutex->lock_thread = -1; + } else { + // Resume the next waiting thread and re-lock the mutex std::vector::iterator iter = mutex->waiting_threads.begin(); ReleaseMutexForThread(mutex, *iter); mutex->waiting_threads.erase(iter); } - // Reset mutex lock thread handle, nothing is waiting - mutex->locked = false; - mutex->lock_thread = -1; - return true; } From 64128aa61a7ada0744f801df116dfbe229a50382 Mon Sep 17 00:00:00 2001 From: Subv Date: Sun, 7 Dec 2014 15:44:21 -0500 Subject: [PATCH 2/3] Mutex: Release all held mutexes when a thread exits. --- src/core/hle/kernel/mutex.cpp | 68 +++++++++++++++++++++++----------- src/core/hle/kernel/mutex.h | 6 +++ src/core/hle/kernel/thread.cpp | 4 ++ 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 17850c1b3..01de3c510 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -13,6 +13,9 @@ namespace Kernel { +class Mutex; +void MutexAcquireLock(Mutex* mutex, Handle thread = GetCurrentThreadHandle()); + class Mutex : public Object { public: std::string GetTypeName() const override { return "Mutex"; } @@ -34,6 +37,7 @@ public: } else { // Lock the mutex when the first thread accesses it locked = true; + MutexAcquireLock(this); } return MakeResult(wait); @@ -45,21 +49,46 @@ public: typedef std::multimap MutexMap; static MutexMap g_mutex_held_locks; +/** + * Acquires the specified mutex for the specified thread + * @param mutex Mutex that is to be acquired + * @param thread Thread that will acquired + */ void MutexAcquireLock(Mutex* mutex, Handle thread) { g_mutex_held_locks.insert(std::make_pair(thread, mutex->GetHandle())); mutex->lock_thread = thread; } -void MutexAcquireLock(Mutex* mutex) { - Handle thread = GetCurrentThreadHandle(); +bool ReleaseMutexForThread(Mutex* mutex, Handle thread) { MutexAcquireLock(mutex, thread); + Kernel::ResumeThreadFromWait(thread); + return true; +} + +/** + * Resumes a thread waiting for the specified mutex + * @param mutex The mutex that some thread is waiting on + */ +void ResumeWaitingThread(Mutex* mutex) { + // Find the next waiting thread for the mutex... + if (mutex->waiting_threads.empty()) { + // Reset mutex lock thread handle, nothing is waiting + mutex->locked = false; + mutex->lock_thread = -1; + } + else { + // Resume the next waiting thread and re-lock the mutex + std::vector::iterator iter = mutex->waiting_threads.begin(); + ReleaseMutexForThread(mutex, *iter); + mutex->waiting_threads.erase(iter); + } } void MutexEraseLock(Mutex* mutex) { Handle handle = mutex->GetHandle(); auto locked = g_mutex_held_locks.equal_range(mutex->lock_thread); for (MutexMap::iterator iter = locked.first; iter != locked.second; ++iter) { - if ((*iter).second == handle) { + if (iter->second == handle) { g_mutex_held_locks.erase(iter); break; } @@ -67,6 +96,19 @@ void MutexEraseLock(Mutex* mutex) { mutex->lock_thread = -1; } +void ReleaseThreadMutexes(Handle thread) { + auto locked = g_mutex_held_locks.equal_range(thread); + + // Release every mutex that the thread holds, and resume execution on the waiting threads + for (MutexMap::iterator iter = locked.first; iter != locked.second; ++iter) { + Mutex* mutex = g_object_pool.GetFast(iter->second); + ResumeWaitingThread(mutex); + } + + // Erase all the locks that this thread holds + g_mutex_held_locks.erase(thread); +} + bool LockMutex(Mutex* mutex) { // Mutex alread locked? if (mutex->locked) { @@ -76,27 +118,9 @@ bool LockMutex(Mutex* mutex) { return true; } -bool ReleaseMutexForThread(Mutex* mutex, Handle thread) { - MutexAcquireLock(mutex, thread); - Kernel::ResumeThreadFromWait(thread); - return true; -} - bool ReleaseMutex(Mutex* mutex) { MutexEraseLock(mutex); - - // Find the next waiting thread for the mutex... - if (mutex->waiting_threads.empty()) { - // Reset mutex lock thread handle, nothing is waiting - mutex->locked = false; - mutex->lock_thread = -1; - } else { - // Resume the next waiting thread and re-lock the mutex - std::vector::iterator iter = mutex->waiting_threads.begin(); - ReleaseMutexForThread(mutex, *iter); - mutex->waiting_threads.erase(iter); - } - + ResumeWaitingThread(mutex); return true; } diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index 155449f95..7f4909a6e 100644 --- a/src/core/hle/kernel/mutex.h +++ b/src/core/hle/kernel/mutex.h @@ -24,4 +24,10 @@ ResultCode ReleaseMutex(Handle handle); */ Handle CreateMutex(bool initial_locked, const std::string& name="Unknown"); +/** + * Releases all the mutexes held by the specified thread + * @param thread Thread that is holding the mutexes + */ +void ReleaseThreadMutexes(Handle thread); + } // namespace diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 8d65dc84d..c01d76e4d 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -14,6 +14,7 @@ #include "core/hle/hle.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/thread.h" +#include "core/hle/kernel/mutex.h" #include "core/hle/result.h" #include "core/mem_map.h" @@ -156,6 +157,9 @@ ResultCode StopThread(Handle handle, const char* reason) { Thread* thread = g_object_pool.Get(handle); if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel); + // Release all the mutexes that this thread holds + ReleaseThreadMutexes(handle); + ChangeReadyState(thread, false); thread->status = THREADSTATUS_DORMANT; for (Handle waiting_handle : thread->waiting_threads) { From bc318c464bbe1a33e37312339d25c56021c444e8 Mon Sep 17 00:00:00 2001 From: Subv Date: Sun, 7 Dec 2014 15:57:28 -0500 Subject: [PATCH 3/3] Mutex: Remove some forward declarations Moved Mutex::WaitSynchronization to the end of the file. --- src/core/hle/kernel/mutex.cpp | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 01de3c510..5a173e129 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -13,9 +13,6 @@ namespace Kernel { -class Mutex; -void MutexAcquireLock(Mutex* mutex, Handle thread = GetCurrentThreadHandle()); - class Mutex : public Object { public: std::string GetTypeName() const override { return "Mutex"; } @@ -30,18 +27,7 @@ public: std::vector waiting_threads; ///< Threads that are waiting for the mutex std::string name; ///< Name of mutex (optional) - ResultVal WaitSynchronization() override { - bool wait = locked; - if (locked) { - Kernel::WaitCurrentThread(WAITTYPE_MUTEX, GetHandle()); - } else { - // Lock the mutex when the first thread accesses it - locked = true; - MutexAcquireLock(this); - } - - return MakeResult(wait); - } + ResultVal WaitSynchronization() override; }; //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -54,7 +40,7 @@ static MutexMap g_mutex_held_locks; * @param mutex Mutex that is to be acquired * @param thread Thread that will acquired */ -void MutexAcquireLock(Mutex* mutex, Handle thread) { +void MutexAcquireLock(Mutex* mutex, Handle thread = GetCurrentThreadHandle()) { g_mutex_held_locks.insert(std::make_pair(thread, mutex->GetHandle())); mutex->lock_thread = thread; } @@ -178,4 +164,17 @@ Handle CreateMutex(bool initial_locked, const std::string& name) { return handle; } +ResultVal Mutex::WaitSynchronization() { + bool wait = locked; + if (locked) { + Kernel::WaitCurrentThread(WAITTYPE_MUTEX, GetHandle()); + } + else { + // Lock the mutex when the first thread accesses it + locked = true; + MutexAcquireLock(this); + } + + return MakeResult(wait); +} } // namespace