From 3b5b950c895a2db217a3e5c8105cec4498a2534e Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Tue, 25 Feb 2020 16:38:33 -0400 Subject: [PATCH] SVC: Correct SignalEvent, ClearEvent, ResetSignal, WaitSynchronization, CancelSynchronization, ArbitrateLock --- src/core/hle/kernel/mutex.cpp | 71 +++++++----- src/core/hle/kernel/process.cpp | 1 + src/core/hle/kernel/readable_event.cpp | 3 + src/core/hle/kernel/svc.cpp | 1 - src/core/hle/kernel/synchronization.cpp | 116 ++++++++++--------- src/core/hle/kernel/synchronization_object.h | 5 +- src/core/hle/kernel/thread.cpp | 14 ++- src/core/hle/kernel/thread.h | 17 ++- 8 files changed, 136 insertions(+), 92 deletions(-) diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 7869eb32b6..3520c5e498 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -72,42 +72,55 @@ ResultCode Mutex::TryAcquire(VAddr address, Handle holding_thread_handle, return ERR_INVALID_ADDRESS; } - const auto& handle_table = system.Kernel().CurrentProcess()->GetHandleTable(); + auto& kernel = system.Kernel(); std::shared_ptr current_thread = - SharedFrom(system.CurrentScheduler().GetCurrentThread()); - std::shared_ptr holding_thread = handle_table.Get(holding_thread_handle); - std::shared_ptr requesting_thread = handle_table.Get(requesting_thread_handle); + SharedFrom(kernel.CurrentScheduler().GetCurrentThread()); + { + SchedulerLock lock(kernel); + // The mutex address must be 4-byte aligned + if ((address % sizeof(u32)) != 0) { + return ERR_INVALID_ADDRESS; + } - // TODO(Subv): It is currently unknown if it is possible to lock a mutex in behalf of another - // thread. - ASSERT(requesting_thread == current_thread); + const auto& handle_table = kernel.CurrentProcess()->GetHandleTable(); + std::shared_ptr holding_thread = handle_table.Get(holding_thread_handle); + std::shared_ptr requesting_thread = handle_table.Get(requesting_thread_handle); - const u32 addr_value = system.Memory().Read32(address); + // TODO(Subv): It is currently unknown if it is possible to lock a mutex in behalf of another + // thread. + ASSERT(requesting_thread == current_thread); - // If the mutex isn't being held, just return success. - if (addr_value != (holding_thread_handle | Mutex::MutexHasWaitersFlag)) { - return RESULT_SUCCESS; + current_thread->SetSynchronizationResults(nullptr, RESULT_SUCCESS); + + const u32 addr_value = system.Memory().Read32(address); + + // If the mutex isn't being held, just return success. + if (addr_value != (holding_thread_handle | Mutex::MutexHasWaitersFlag)) { + return RESULT_SUCCESS; + } + + if (holding_thread == nullptr) { + return ERR_INVALID_HANDLE; + } + + // Wait until the mutex is released + current_thread->SetMutexWaitAddress(address); + current_thread->SetWaitHandle(requesting_thread_handle); + + current_thread->SetStatus(ThreadStatus::WaitMutex); + + // Update the lock holder thread's priority to prevent priority inversion. + holding_thread->AddMutexWaiter(current_thread); } - if (holding_thread == nullptr) { - LOG_ERROR(Kernel, "Holding thread does not exist! thread_handle={:08X}", - holding_thread_handle); - return ERR_INVALID_HANDLE; + { + SchedulerLock lock(kernel); + auto* owner = current_thread->GetLockOwner(); + if (owner != nullptr) { + owner->RemoveMutexWaiter(current_thread); + } } - - // Wait until the mutex is released - current_thread->SetMutexWaitAddress(address); - current_thread->SetWaitHandle(requesting_thread_handle); - - current_thread->SetStatus(ThreadStatus::WaitMutex); - current_thread->InvalidateWakeupCallback(); - - // Update the lock holder thread's priority to prevent priority inversion. - holding_thread->AddMutexWaiter(current_thread); - - system.PrepareReschedule(); - - return RESULT_SUCCESS; + return current_thread->GetSignalingResult(); } ResultCode Mutex::Release(VAddr address) { diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index 7e26a54f41..cd4b0aa606 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -212,6 +212,7 @@ void Process::UnregisterThread(const Thread* thread) { } ResultCode Process::ClearSignalState() { + SchedulerLock lock(system.Kernel()); if (status == ProcessStatus::Exited) { LOG_ERROR(Kernel, "called on a terminated process instance."); return ERR_INVALID_STATE; diff --git a/src/core/hle/kernel/readable_event.cpp b/src/core/hle/kernel/readable_event.cpp index ef5e19e63b..6e286419e9 100644 --- a/src/core/hle/kernel/readable_event.cpp +++ b/src/core/hle/kernel/readable_event.cpp @@ -6,8 +6,10 @@ #include "common/assert.h" #include "common/logging/log.h" #include "core/hle/kernel/errors.h" +#include "core/hle/kernel/kernel.h" #include "core/hle/kernel/object.h" #include "core/hle/kernel/readable_event.h" +#include "core/hle/kernel/scheduler.h" #include "core/hle/kernel/thread.h" namespace Kernel { @@ -37,6 +39,7 @@ void ReadableEvent::Clear() { } ResultCode ReadableEvent::Reset() { + SchedulerLock lock(kernel); if (!is_signaled) { LOG_TRACE(Kernel, "Handle is not signaled! object_id={}, object_type={}, object_name={}", GetObjectId(), GetTypeName(), GetName()); diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a071b0c09a..0d905c0ca5 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -448,7 +448,6 @@ static ResultCode CancelSynchronization(Core::System& system, Handle thread_hand } thread->CancelWait(); - system.PrepareReschedule(thread->GetProcessorID()); return RESULT_SUCCESS; } diff --git a/src/core/hle/kernel/synchronization.cpp b/src/core/hle/kernel/synchronization.cpp index dc37fad1a7..b36e550a04 100644 --- a/src/core/hle/kernel/synchronization.cpp +++ b/src/core/hle/kernel/synchronization.cpp @@ -10,78 +10,88 @@ #include "core/hle/kernel/synchronization.h" #include "core/hle/kernel/synchronization_object.h" #include "core/hle/kernel/thread.h" +#include "core/hle/kernel/time_manager.h" namespace Kernel { -/// Default thread wakeup callback for WaitSynchronization -static bool DefaultThreadWakeupCallback(ThreadWakeupReason reason, std::shared_ptr thread, - std::shared_ptr object, - std::size_t index) { - ASSERT(thread->GetStatus() == ThreadStatus::WaitSynch); - - if (reason == ThreadWakeupReason::Timeout) { - thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); - return true; - } - - ASSERT(reason == ThreadWakeupReason::Signal); - thread->SetWaitSynchronizationResult(RESULT_SUCCESS); - thread->SetWaitSynchronizationOutput(static_cast(index)); - return true; -} - Synchronization::Synchronization(Core::System& system) : system{system} {} void Synchronization::SignalObject(SynchronizationObject& obj) const { + SchedulerLock lock(system.Kernel()); if (obj.IsSignaled()) { - obj.WakeupAllWaitingThreads(); + for (auto thread : obj.GetWaitingThreads()) { + if (thread->GetSchedulingStatus() == ThreadSchedStatus::Paused) { + thread->SetSynchronizationResults(&obj, RESULT_SUCCESS); + thread->ResumeFromWait(); + } + } } } std::pair Synchronization::WaitFor( std::vector>& sync_objects, s64 nano_seconds) { + auto& kernel = system.Kernel(); auto* const thread = system.CurrentScheduler().GetCurrentThread(); - // Find the first object that is acquirable in the provided list of objects - const auto itr = std::find_if(sync_objects.begin(), sync_objects.end(), - [thread](const std::shared_ptr& object) { - return object->IsSignaled(); - }); + Handle event_handle = InvalidHandle; + { + SchedulerLockAndSleep lock(kernel, event_handle, thread, nano_seconds); + const auto itr = + std::find_if(sync_objects.begin(), sync_objects.end(), + [thread](const std::shared_ptr& object) { + return object->IsSignaled(); + }); - if (itr != sync_objects.end()) { - // We found a ready object, acquire it and set the result value - SynchronizationObject* object = itr->get(); - object->Acquire(thread); - const u32 index = static_cast(std::distance(sync_objects.begin(), itr)); - return {RESULT_SUCCESS, index}; + if (itr != sync_objects.end()) { + // We found a ready object, acquire it and set the result value + SynchronizationObject* object = itr->get(); + object->Acquire(thread); + const u32 index = static_cast(std::distance(sync_objects.begin(), itr)); + lock.CancelSleep(); + return {RESULT_SUCCESS, index}; + } + + if (nano_seconds == 0) { + lock.CancelSleep(); + return {RESULT_TIMEOUT, InvalidHandle}; + } + + /// TODO(Blinkhawk): Check for termination pending + + if (thread->IsSyncCancelled()) { + thread->SetSyncCancelled(false); + lock.CancelSleep(); + return {ERR_SYNCHRONIZATION_CANCELED, InvalidHandle}; + } + + for (auto& object : sync_objects) { + object->AddWaitingThread(SharedFrom(thread)); + } + thread->SetSynchronizationResults(nullptr, RESULT_TIMEOUT); + thread->SetStatus(ThreadStatus::WaitSynch); } - // No objects were ready to be acquired, prepare to suspend the thread. - - // If a timeout value of 0 was provided, just return the Timeout error code instead of - // suspending the thread. - if (nano_seconds == 0) { - return {RESULT_TIMEOUT, InvalidHandle}; + if (event_handle != InvalidHandle) { + auto& time_manager = kernel.TimeManager(); + time_manager.UnscheduleTimeEvent(event_handle); } - if (thread->IsSyncCancelled()) { - thread->SetSyncCancelled(false); - return {ERR_SYNCHRONIZATION_CANCELED, InvalidHandle}; + { + SchedulerLock lock(kernel); + ResultCode signaling_result = thread->GetSignalingResult(); + SynchronizationObject* signaling_object = thread->GetSignalingObject(); + if (signaling_result == RESULT_SUCCESS) { + const auto itr = std::find_if( + sync_objects.begin(), sync_objects.end(), + [signaling_object](const std::shared_ptr& object) { + return object.get() == signaling_object; + }); + ASSERT(itr != sync_objects.end()); + signaling_object->Acquire(thread); + const u32 index = static_cast(std::distance(sync_objects.begin(), itr)); + return {RESULT_SUCCESS, index}; + } + return {signaling_result, -1}; } - - for (auto& object : sync_objects) { - object->AddWaitingThread(SharedFrom(thread)); - } - - thread->SetSynchronizationObjects(std::move(sync_objects)); - thread->SetStatus(ThreadStatus::WaitSynch); - - // Create an event to wake the thread up after the specified nanosecond delay has passed - thread->WakeAfterDelay(nano_seconds); - thread->SetWakeupCallback(DefaultThreadWakeupCallback); - - system.PrepareReschedule(thread->GetProcessorID()); - - return {RESULT_TIMEOUT, InvalidHandle}; } } // namespace Kernel diff --git a/src/core/hle/kernel/synchronization_object.h b/src/core/hle/kernel/synchronization_object.h index 741c31faf1..0a0d069e0e 100644 --- a/src/core/hle/kernel/synchronization_object.h +++ b/src/core/hle/kernel/synchronization_object.h @@ -12,6 +12,7 @@ namespace Kernel { class KernelCore; +class Synchronization; class Thread; /// Class that represents a Kernel object that a thread can be waiting on @@ -53,7 +54,7 @@ public: * Wake up all threads waiting on this object that can be awoken, in priority order, * and set the synchronization result and output of the thread. */ - void WakeupAllWaitingThreads(); + void /* deprecated */ WakeupAllWaitingThreads(); /** * Wakes up a single thread waiting on this object. @@ -62,7 +63,7 @@ public: void WakeupWaitingThread(std::shared_ptr thread); /// Obtains the highest priority thread that is ready to run from this object's waiting list. - std::shared_ptr GetHighestPriorityReadyThread() const; + std::shared_ptr /* deprecated */ GetHighestPriorityReadyThread() const; /// Get a const reference to the waiting threads list for debug use const std::vector>& GetWaitingThreads() const; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index e6bb7c6663..5fef3945be 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -139,12 +139,13 @@ ResultCode Thread::Start() { } void Thread::CancelWait() { + SchedulerLock lock(kernel); if (GetSchedulingStatus() != ThreadSchedStatus::Paused) { is_sync_cancelled = true; return; } is_sync_cancelled = false; - SetWaitSynchronizationResult(ERR_SYNCHRONIZATION_CANCELED); + SetSynchronizationResults(nullptr, ERR_SYNCHRONIZATION_CANCELED); ResumeFromWait(); } @@ -258,13 +259,16 @@ void Thread::SetPriority(u32 priority) { } void Thread::SetWaitSynchronizationResult(ResultCode result) { - context_32.cpu_registers[0] = result.raw; - context_64.cpu_registers[0] = result.raw; + UNREACHABLE(); } void Thread::SetWaitSynchronizationOutput(s32 output) { - context_32.cpu_registers[1] = output; - context_64.cpu_registers[1] = output; + UNREACHABLE(); +} + +void Thread::SetSynchronizationResults(SynchronizationObject* object, ResultCode result) { + signaling_object = object; + signaling_result = result; } s32 Thread::GetSynchronizationObjectIndex(std::shared_ptr object) const { diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 29fe5483b4..a8ae1a66f2 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -259,13 +259,23 @@ public: * Sets the result after the thread awakens (from svcWaitSynchronization) * @param result Value to set to the returned result */ - void SetWaitSynchronizationResult(ResultCode result); + void /*deprecated*/ SetWaitSynchronizationResult(ResultCode result); /** * Sets the output parameter value after the thread awakens (from svcWaitSynchronization) * @param output Value to set to the output parameter */ - void SetWaitSynchronizationOutput(s32 output); + void /*deprecated*/ SetWaitSynchronizationOutput(s32 output); + + void SetSynchronizationResults(SynchronizationObject* object, ResultCode result); + + SynchronizationObject* GetSignalingObject() const { + return signaling_object; + } + + ResultCode GetSignalingResult() const { + return signaling_result; + } /** * Retrieves the index that this particular object occupies in the list of objects @@ -565,6 +575,9 @@ private: /// passed to WaitSynchronization. ThreadSynchronizationObjects wait_objects; + SynchronizationObject* signaling_object; + ResultCode signaling_result{RESULT_SUCCESS}; + /// List of threads that are waiting for a mutex that is held by this thread. MutexWaitingThreads wait_mutex_threads;