Kernel: Convert Mutex to not use Handles

src/core/hle/kernel/mutex.cpp

@@ -13,59 +13,30 @@
 
 namespace Kernel {
 
-class Mutex : public WaitObject {
-public:
-    std::string GetTypeName() const override { return "Mutex"; }
-    std::string GetName() const override { return name; }
-
-    static const HandleType HANDLE_TYPE = HandleType::Mutex;
-    HandleType GetHandleType() const override { return HANDLE_TYPE; }
-
-    bool initial_locked;                        ///< Initial lock state when mutex was created
-    bool locked;                                ///< Current locked state
-    std::string name;                           ///< Name of mutex (optional)
-    SharedPtr<Thread> holding_thread;           ///< Thread that has acquired the mutex
-
-    bool ShouldWait() override;
-    void Acquire() override;
-};
-
-////////////////////////////////////////////////////////////////////////////////////////////////////
-
 typedef std::multimap<SharedPtr<Thread>, SharedPtr<Mutex>> 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 acquire the mutex
- */
-static void MutexAcquireLock(Mutex* mutex, Thread* thread) {
-    g_mutex_held_locks.insert(std::make_pair(thread, mutex));
-    mutex->holding_thread = thread;
-}
-
 /**
  * Resumes a thread waiting for the specified mutex
  * @param mutex The mutex that some thread is waiting on
  */
 static void ResumeWaitingThread(Mutex* mutex) {
-    // Find the next waiting thread for the mutex...
-    auto next_thread = mutex->WakeupNextThread();
-    if (next_thread != nullptr) {
-        MutexAcquireLock(mutex, next_thread);
-    } else {
     // Reset mutex lock thread handle, nothing is waiting
     mutex->locked = false;
     mutex->holding_thread = nullptr;
+
+    // Find the next waiting thread for the mutex...
+    auto next_thread = mutex->WakeupNextThread();
+    if (next_thread != nullptr) {
+        mutex->Acquire(next_thread);
     }
 }
 
 void ReleaseThreadMutexes(Thread* thread) {
-    auto locked = g_mutex_held_locks.equal_range(thread);
+    auto locked_range = g_mutex_held_locks.equal_range(thread);
     
     // Release every mutex that the thread holds, and resume execution on the waiting threads
-    for (auto iter = locked.first; iter != locked.second; ++iter) {
+    for (auto iter = locked_range.first; iter != locked_range.second; ++iter) {
         ResumeWaitingThread(iter->second.get());
     }
 
@@ -73,62 +44,21 @@ void ReleaseThreadMutexes(Thread* thread) {
     g_mutex_held_locks.erase(thread);
 }
 
-static bool ReleaseMutex(Mutex* mutex) {
-    if (mutex->locked) {
-        auto locked = g_mutex_held_locks.equal_range(mutex->holding_thread);
+ResultVal<SharedPtr<Mutex>> Mutex::Create(bool initial_locked, std::string name) {
+    SharedPtr<Mutex> mutex(new Mutex);
+    // TOOD(yuriks): Don't create Handle (see Thread::Create())
+    CASCADE_RESULT(auto unused, Kernel::g_handle_table.Create(mutex));
 
-        for (MutexMap::iterator iter = locked.first; iter != locked.second; ++iter) {
-            if (iter->second == mutex) {
-                g_mutex_held_locks.erase(iter);
-                break;
-            }
-        }
-
-        ResumeWaitingThread(mutex);
-    }
-    return true;
-}
-
-ResultCode ReleaseMutex(Handle handle) {
-    Mutex* mutex = Kernel::g_handle_table.Get<Mutex>(handle).get();
-    if (mutex == nullptr) return InvalidHandle(ErrorModule::Kernel);
-
-    if (!ReleaseMutex(mutex)) {
-        // TODO(yuriks): Verify error code, this one was pulled out of thin air. I'm not even sure
-        // what error condition this is supposed to be signaling.
-        return ResultCode(ErrorDescription::AlreadyDone, ErrorModule::Kernel,
-                ErrorSummary::NothingHappened, ErrorLevel::Temporary);
-    }
-    return RESULT_SUCCESS;
-}
-
-/**
- * Creates a mutex
- * @param handle Reference to handle for the newly created mutex
- * @param initial_locked Specifies if the mutex should be locked initially
- * @param name Optional name of mutex
- * @return Pointer to new Mutex object
- */
-static Mutex* CreateMutex(Handle& handle, bool initial_locked, const std::string& name) {
-    Mutex* mutex = new Mutex;
-    // TODO(yuriks): Fix error reporting
-    handle = Kernel::g_handle_table.Create(mutex).ValueOr(INVALID_HANDLE);
-
-    mutex->locked = mutex->initial_locked = initial_locked;
-    mutex->name = name;
+    mutex->initial_locked = initial_locked;
+    mutex->locked = false;
+    mutex->name = std::move(name);
     mutex->holding_thread = nullptr;
 
     // Acquire mutex with current thread if initialized as locked...
-    if (mutex->locked)
-        MutexAcquireLock(mutex, GetCurrentThread());
+    if (initial_locked)
+        mutex->Acquire();
 
-    return mutex;
-}
-
-Handle CreateMutex(bool initial_locked, const std::string& name) {
-    Handle handle;
-    Mutex* mutex = CreateMutex(handle, initial_locked, name);
-    return handle;
+    return MakeResult<SharedPtr<Mutex>>(mutex);
 }
 
 bool Mutex::ShouldWait() {
@@ -136,9 +66,34 @@ bool Mutex::ShouldWait() {
 }
 
 void Mutex::Acquire() {
+    Acquire(GetCurrentThread());
+}
+
+void Mutex::Acquire(Thread* thread) {
     _assert_msg_(Kernel, !ShouldWait(), "object unavailable!");
+    if (locked)
+        return;
+
     locked = true;
-    MutexAcquireLock(this, GetCurrentThread());
+
+    g_mutex_held_locks.insert(std::make_pair(thread, this));
+    holding_thread = thread;
+}
+
+void Mutex::Release() {
+    if (!locked)
+        return;
+
+    auto locked_range = g_mutex_held_locks.equal_range(holding_thread);
+
+    for (MutexMap::iterator iter = locked_range.first; iter != locked_range.second; ++iter) {
+        if (iter->second == this) {
+            g_mutex_held_locks.erase(iter);
+            break;
+        }
+    }
+
+    ResumeWaitingThread(this);
 }
 
 } // namespace

src/core/hle/kernel/mutex.h

@@ -4,25 +4,51 @@
 
 #pragma once
 
+#include <string>
+
 #include "common/common_types.h"
 
 #include "core/hle/kernel/kernel.h"
 
 namespace Kernel {
 
-/**
- * Releases a mutex
- * @param handle Handle to mutex to release
- */
-ResultCode ReleaseMutex(Handle handle);
+class Thread;
 
-/**
- * Creates a mutex
+class Mutex : public WaitObject {
+public:
+    /**
+     * Creates a mutex.
      * @param initial_locked Specifies if the mutex should be locked initially
      * @param name Optional name of mutex
- * @return Handle to newly created object
+     * @return Pointer to new Mutex object
      */
-Handle CreateMutex(bool initial_locked, const std::string& name="Unknown");
+    static ResultVal<SharedPtr<Mutex>> Create(bool initial_locked, std::string name = "Unknown");
+
+    std::string GetTypeName() const override { return "Mutex"; }
+    std::string GetName() const override { return name; }
+
+    static const HandleType HANDLE_TYPE = HandleType::Mutex;
+    HandleType GetHandleType() const override { return HANDLE_TYPE; }
+
+    bool initial_locked;                        ///< Initial lock state when mutex was created
+    bool locked;                                ///< Current locked state
+    std::string name;                           ///< Name of mutex (optional)
+    SharedPtr<Thread> holding_thread;           ///< Thread that has acquired the mutex
+
+    bool ShouldWait() override;
+    void Acquire() override;
+
+    /**
+    * Acquires the specified mutex for the specified thread
+    * @param mutex Mutex that is to be acquired
+    * @param thread Thread that will acquire the mutex
+    */
+    void Acquire(Thread* thread);
+    void Release();
+
+private:
+    Mutex() = default;
+};
 
 /**
  * Releases all the mutexes held by the specified thread

src/core/hle/service/apt_s.cpp

@@ -10,6 +10,7 @@
 #include "core/hle/kernel/event.h"
 #include "core/hle/kernel/mutex.h"
 #include "core/hle/kernel/shared_memory.h"
+#include "core/hle/kernel/thread.h"
 #include "core/hle/service/apt_s.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////

src/core/hle/service/apt_u.cpp

@@ -10,6 +10,7 @@
 #include "core/hle/kernel/event.h"
 #include "core/hle/kernel/mutex.h"
 #include "core/hle/kernel/shared_memory.h"
+#include "core/hle/kernel/thread.h"
 #include "core/hle/service/apt_u.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -28,7 +29,7 @@ static const VAddr SHARED_FONT_VADDR = 0x18000000;
 /// Handle to shared memory region designated to for shared system font
 static Kernel::SharedPtr<Kernel::SharedMemory> shared_font_mem;
 
-static Handle lock_handle = 0;
+static Kernel::SharedPtr<Kernel::Mutex> lock;
 static Handle notification_event_handle = 0; ///< APT notification event handle
 static Handle pause_event_handle = 0; ///< APT pause event handle
 static std::vector<u8> shared_font;
@@ -76,8 +77,8 @@ void Initialize(Service::Interface* self) {
     Kernel::ClearEvent(notification_event_handle);
     Kernel::SignalEvent(pause_event_handle); // Fire start event
 
-    _assert_msg_(KERNEL, (0 != lock_handle), "Cannot initialize without lock");
-    Kernel::ReleaseMutex(lock_handle);
+    _assert_msg_(KERNEL, (nullptr != lock), "Cannot initialize without lock");
+    lock->Release();
 
     cmd_buff[1] = RESULT_SUCCESS.raw; // No error
 }
@@ -103,10 +104,9 @@ void GetLockHandle(Service::Interface* self) {
     u32* cmd_buff = Kernel::GetCommandBuffer();
     u32 flags = cmd_buff[1]; // TODO(bunnei): Figure out the purpose of the flag field
 
-    if (0 == lock_handle) {
+    if (nullptr == lock) {
         // TODO(bunnei): Verify if this is created here or at application boot?
-        lock_handle = Kernel::CreateMutex(false, "APT_U:Lock");
-        Kernel::ReleaseMutex(lock_handle);
+        lock = Kernel::Mutex::Create(false, "APT_U:Lock").MoveFrom();
     }
     cmd_buff[1] = RESULT_SUCCESS.raw; // No error
 
@@ -116,7 +116,7 @@ void GetLockHandle(Service::Interface* self) {
     cmd_buff[3] = 0;
     cmd_buff[4] = 0;
 
-    cmd_buff[5] = lock_handle;
+    cmd_buff[5] = Kernel::g_handle_table.Create(lock).MoveFrom();
     LOG_TRACE(Service_APT, "called handle=0x%08X", cmd_buff[5]);
 }
 
@@ -520,7 +520,7 @@ Interface::Interface() {
         shared_font_mem = nullptr;
     }
 
-    lock_handle = 0;
+    lock = nullptr;
 
     Register(FunctionTable, ARRAY_SIZE(FunctionTable));
 }

src/core/hle/svc.cpp

@@ -364,18 +364,32 @@ static Result SetThreadPriority(Handle handle, s32 priority) {
 }
 
 /// Create a mutex
-static Result CreateMutex(Handle* mutex, u32 initial_locked) {
-    *mutex = Kernel::CreateMutex((initial_locked != 0));
+static Result CreateMutex(Handle* handle, u32 initial_locked) {
+    using Kernel::Mutex;
+
+    auto mutex_res = Mutex::Create(initial_locked != 0);
+    if (mutex_res.Failed())
+        return mutex_res.Code().raw;
+    SharedPtr<Mutex> mutex = mutex_res.MoveFrom();
+
+    *handle = Kernel::g_handle_table.Create(mutex).MoveFrom();
     LOG_TRACE(Kernel_SVC, "called initial_locked=%s : created handle=0x%08X",
-        initial_locked ? "true" : "false", *mutex);
+        initial_locked ? "true" : "false", *handle);
     return 0;
 }
 
 /// Release a mutex
 static Result ReleaseMutex(Handle handle) {
+    using Kernel::Mutex;
+
     LOG_TRACE(Kernel_SVC, "called handle=0x%08X", handle);
-    ResultCode res = Kernel::ReleaseMutex(handle);
-    return res.raw;
+
+    SharedPtr<Mutex> mutex = Kernel::g_handle_table.Get<Mutex>(handle);
+    if (mutex == nullptr)
+        return InvalidHandle(ErrorModule::Kernel).raw;
+
+    mutex->Release();
+    return RESULT_SUCCESS.raw;
 }
 
 /// Get the ID for the specified thread.