From 44e09e5f21915391672558940842b92e3a64cb1b Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Mon, 7 Oct 2019 18:57:13 -0400 Subject: [PATCH] Kernel: Correct Results in Condition Variables and Mutexes --- src/core/hle/kernel/kernel.cpp | 13 +++++++------ src/core/hle/kernel/mutex.cpp | 1 + src/core/hle/kernel/svc.cpp | 27 +++++++++------------------ 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 77edbcd1f1..002c5af2bb 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -15,6 +15,7 @@ #include "core/core_timing_util.h" #include "core/hle/kernel/address_arbiter.h" #include "core/hle/kernel/client_port.h" +#include "core/hle/kernel/errors.h" #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/process.h" @@ -60,12 +61,8 @@ static void ThreadWakeupCallback(u64 thread_handle, [[maybe_unused]] s64 cycles_ if (thread->HasWakeupCallback()) { resume = thread->InvokeWakeupCallback(ThreadWakeupReason::Timeout, thread, nullptr, 0); } - } - - if (thread->GetMutexWaitAddress() != 0 || thread->GetCondVarWaitAddress() != 0 || - thread->GetWaitHandle() != 0) { - ASSERT(thread->GetStatus() == ThreadStatus::WaitMutex || - thread->GetStatus() == ThreadStatus::WaitCondVar); + } else if (thread->GetStatus() == ThreadStatus::WaitMutex || + thread->GetStatus() == ThreadStatus::WaitCondVar) { thread->SetMutexWaitAddress(0); thread->SetCondVarWaitAddress(0); thread->SetWaitHandle(0); @@ -85,6 +82,10 @@ static void ThreadWakeupCallback(u64 thread_handle, [[maybe_unused]] s64 cycles_ } if (resume) { + if (thread->GetStatus() == ThreadStatus::WaitCondVar || + thread->GetStatus() == ThreadStatus::WaitArb) { + thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); + } thread->ResumeFromWait(); } } diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index eb919246cd..663d0f4b6e 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -139,6 +139,7 @@ ResultCode Mutex::Release(VAddr address) { thread->SetCondVarWaitAddress(0); thread->SetMutexWaitAddress(0); thread->SetWaitHandle(0); + thread->SetWaitSynchronizationResult(RESULT_SUCCESS); system.PrepareReschedule(); diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index bd67fc96d7..823d1d4032 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1677,18 +1677,20 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var // Atomically read the value of the mutex. u32 mutex_val = 0; + u32 update_val = 0; + const VAddr mutex_address = thread->GetMutexWaitAddress(); do { - monitor.SetExclusive(current_core, thread->GetMutexWaitAddress()); + monitor.SetExclusive(current_core, mutex_address); // If the mutex is not yet acquired, acquire it. - mutex_val = Memory::Read32(thread->GetMutexWaitAddress()); + mutex_val = Memory::Read32(mutex_address); if (mutex_val != 0) { - monitor.ClearExclusive(); - break; + update_val = mutex_val | Mutex::MutexHasWaitersFlag; + } else { + update_val = thread->GetWaitHandle(); } - } while (!monitor.ExclusiveWrite32(current_core, thread->GetMutexWaitAddress(), - thread->GetWaitHandle())); + } while (!monitor.ExclusiveWrite32(current_core, mutex_address, update_val)); if (mutex_val == 0) { // We were able to acquire the mutex, resume this thread. ASSERT(thread->GetStatus() == ThreadStatus::WaitCondVar); @@ -1702,20 +1704,9 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var thread->SetLockOwner(nullptr); thread->SetMutexWaitAddress(0); thread->SetWaitHandle(0); + thread->SetWaitSynchronizationResult(RESULT_SUCCESS); system.PrepareReschedule(thread->GetProcessorID()); } else { - // Atomically signal that the mutex now has a waiting thread. - do { - monitor.SetExclusive(current_core, thread->GetMutexWaitAddress()); - - // Ensure that the mutex value is still what we expect. - u32 value = Memory::Read32(thread->GetMutexWaitAddress()); - // TODO(Subv): When this happens, the kernel just clears the exclusive state and - // retries the initial read for this thread. - ASSERT_MSG(mutex_val == value, "Unhandled synchronization primitive case"); - } while (!monitor.ExclusiveWrite32(current_core, thread->GetMutexWaitAddress(), - mutex_val | Mutex::MutexHasWaitersFlag)); - // The mutex is already owned by some other thread, make this thread wait on it. const Handle owner_handle = static_cast(mutex_val & Mutex::MutexOwnerMask); const auto& handle_table = system.Kernel().CurrentProcess()->GetHandleTable();