SVC: Correct SignalEvent, ClearEvent, ResetSignal, WaitSynchronization, CancelSynchronization, ArbitrateLock

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<Thread> current_thread =
-        SharedFrom(system.CurrentScheduler().GetCurrentThread());
-    std::shared_ptr<Thread> holding_thread = handle_table.Get<Thread>(holding_thread_handle);
-    std::shared_ptr<Thread> requesting_thread = handle_table.Get<Thread>(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<Thread> holding_thread = handle_table.Get<Thread>(holding_thread_handle);
+        std::shared_ptr<Thread> requesting_thread = handle_table.Get<Thread>(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);
 
-    if (holding_thread == nullptr) {
-        LOG_ERROR(Kernel, "Holding thread does not exist! thread_handle={:08X}",
-                  holding_thread_handle);
-        return ERR_INVALID_HANDLE;
-    }
+        const u32 addr_value = system.Memory().Read32(address);
 
-    // Wait until the mutex is released
-    current_thread->SetMutexWaitAddress(address);
-    current_thread->SetWaitHandle(requesting_thread_handle);
+        // If the mutex isn't being held, just return success.
+        if (addr_value != (holding_thread_handle | Mutex::MutexHasWaitersFlag)) {
+            return RESULT_SUCCESS;
+        }
 
-    current_thread->SetStatus(ThreadStatus::WaitMutex);
-    current_thread->InvalidateWakeupCallback();
+        if (holding_thread == nullptr) {
+            return ERR_INVALID_HANDLE;
+        }
 
-    // Update the lock holder thread's priority to prevent priority inversion.
-    holding_thread->AddMutexWaiter(current_thread);
+        // Wait until the mutex is released
+        current_thread->SetMutexWaitAddress(address);
+        current_thread->SetWaitHandle(requesting_thread_handle);
 
-    system.PrepareReschedule();
+        current_thread->SetStatus(ThreadStatus::WaitMutex);
 
-    return RESULT_SUCCESS;
+        // Update the lock holder thread's priority to prevent priority inversion.
+        holding_thread->AddMutexWaiter(current_thread);
+    }
+
+    {
+        SchedulerLock lock(kernel);
+        auto* owner = current_thread->GetLockOwner();
+        if (owner != nullptr) {
+            owner->RemoveMutexWaiter(current_thread);
+        }
+    }
+    return current_thread->GetSignalingResult();
 }
 
 ResultCode Mutex::Release(VAddr address) {

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;

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());

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;
 }
 

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> thread,
-                                        std::shared_ptr<SynchronizationObject> 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<u32>(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<ResultCode, Handle> Synchronization::WaitFor(
     std::vector<std::shared_ptr<SynchronizationObject>>& 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<SynchronizationObject>& 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<s32>(std::distance(sync_objects.begin(), itr));
-        return {RESULT_SUCCESS, index};
+    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<SynchronizationObject>& 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<s32>(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<SynchronizationObject>& object) {
+                    return object.get() == signaling_object;
+                });
+            ASSERT(itr != sync_objects.end());
+            signaling_object->Acquire(thread);
+            const u32 index = static_cast<s32>(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

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> thread);
 
     /// Obtains the highest priority thread that is ready to run from this object's waiting list.
-    std::shared_ptr<Thread> GetHighestPriorityReadyThread() const;
+    std::shared_ptr<Thread> /* deprecated */ GetHighestPriorityReadyThread() const;
 
     /// Get a const reference to the waiting threads list for debug use
     const std::vector<std::shared_ptr<Thread>>& GetWaitingThreads() const;

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<SynchronizationObject> object) const {

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;