From db3a5251664ae77f5e67dba571a11d208e448a86 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Mon, 8 Jan 2018 14:14:30 -0500
Subject: Kernel: Actually wake up the requested number of threads in
 Semaphore::Release.

Also properly keep track of data in guest memory, this fixes managing the semaphore from userland.

It was found that Semaphores are actually Condition Variables, with Release(1) and Release(-1) being equivalent to notify_one and notify_all. We should change the name of the class to reflect this.
---
 src/citra_qt/debugger/wait_tree.cpp |  5 ++---
 src/core/hle/kernel/semaphore.cpp   | 23 +++++++++++------------
 src/core/hle/kernel/semaphore.h     |  9 ++++-----
 src/core/hle/kernel/svc.cpp         |  2 +-
 4 files changed, 18 insertions(+), 21 deletions(-)

(limited to 'src')

diff --git a/src/citra_qt/debugger/wait_tree.cpp b/src/citra_qt/debugger/wait_tree.cpp
index ebcc02894..6d15e43aa 100644
--- a/src/citra_qt/debugger/wait_tree.cpp
+++ b/src/citra_qt/debugger/wait_tree.cpp
@@ -273,9 +273,8 @@ std::vector<std::unique_ptr<WaitTreeItem>> WaitTreeSemaphore::GetChildren() cons
     std::vector<std::unique_ptr<WaitTreeItem>> list(WaitTreeWaitObject::GetChildren());
 
     const auto& semaphore = static_cast<const Kernel::Semaphore&>(object);
-    list.push_back(
-        std::make_unique<WaitTreeText>(tr("available count = %1").arg(semaphore.available_count)));
-    list.push_back(std::make_unique<WaitTreeText>(tr("max count = %1").arg(semaphore.max_count)));
+    list.push_back(std::make_unique<WaitTreeText>(
+        tr("available count = %1").arg(semaphore.GetAvailableCount())));
     return list;
 }
 
diff --git a/src/core/hle/kernel/semaphore.cpp b/src/core/hle/kernel/semaphore.cpp
index 9c58aa42f..b555bb28e 100644
--- a/src/core/hle/kernel/semaphore.cpp
+++ b/src/core/hle/kernel/semaphore.cpp
@@ -18,9 +18,6 @@ ResultVal<SharedPtr<Semaphore>> Semaphore::Create(VAddr guest_addr, VAddr mutex_
                                                   std::string name) {
     SharedPtr<Semaphore> semaphore(new Semaphore);
 
-    // When the semaphore is created, some slots are reserved for other threads,
-    // and the rest is reserved for the caller thread;
-    semaphore->available_count = Memory::Read32(guest_addr);
     semaphore->name = std::move(name);
     semaphore->guest_addr = guest_addr;
     semaphore->mutex_addr = mutex_addr;
@@ -32,34 +29,36 @@ ResultVal<SharedPtr<Semaphore>> Semaphore::Create(VAddr guest_addr, VAddr mutex_
 }
 
 bool Semaphore::ShouldWait(Thread* thread) const {
-    return available_count <= 0;
+    return GetAvailableCount() <= 0;
 }
 
 void Semaphore::Acquire(Thread* thread) {
-    if (available_count <= 0)
+    if (GetAvailableCount() <= 0)
         return;
 
-    --available_count;
-    UpdateGuestState();
+    SetAvailableCount(GetAvailableCount() - 1);
 }
 
 ResultCode Semaphore::Release(s32 target) {
-    ++available_count;
-    UpdateGuestState();
-
     if (target == -1) {
         // When -1, wake up all waiting threads
+        SetAvailableCount(GetWaitingThreads().size());
         WakeupAllWaitingThreads();
     } else {
         // Otherwise, wake up just a single thread
+        SetAvailableCount(target);
         WakeupWaitingThread(GetHighestPriorityReadyThread());
     }
 
     return RESULT_SUCCESS;
 }
 
-void Semaphore::UpdateGuestState() {
-    Memory::Write32(guest_addr, available_count);
+s32 Semaphore::GetAvailableCount() const {
+    return Memory::Read32(guest_addr);
+}
+
+void Semaphore::SetAvailableCount(s32 value) const {
+    Memory::Write32(guest_addr, value);
 }
 
 } // namespace Kernel
diff --git a/src/core/hle/kernel/semaphore.h b/src/core/hle/kernel/semaphore.h
index e80230cac..7254eb26d 100644
--- a/src/core/hle/kernel/semaphore.h
+++ b/src/core/hle/kernel/semaphore.h
@@ -13,6 +13,7 @@
 
 namespace Kernel {
 
+// TODO(Subv): This is actually a Condition Variable.
 class Semaphore final : public WaitObject {
 public:
     /**
@@ -39,8 +40,9 @@ public:
         return HANDLE_TYPE;
     }
 
-    s32 max_count;       ///< Maximum number of simultaneous holders the semaphore can have
-    s32 available_count; ///< Number of free slots left in the semaphore
+    s32 GetAvailableCount() const;
+    void SetAvailableCount(s32 value) const;
+
     std::string name;    ///< Name of semaphore (optional)
     VAddr guest_addr;    ///< Address of the guest semaphore value
     VAddr mutex_addr; ///< (optional) Address of guest mutex value associated with this semaphore,
@@ -59,9 +61,6 @@ public:
 private:
     Semaphore();
     ~Semaphore() override;
-
-    /// Updates the state of the object tracking this semaphore in guest memory
-    void UpdateGuestState();
 };
 
 } // namespace Kernel
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index a3ac3d782..d18d7a182 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -501,7 +501,7 @@ static ResultCode WaitProcessWideKeyAtomic(VAddr mutex_addr, VAddr semaphore_add
         semaphore->name = Common::StringFromFormat("semaphore-%llx", semaphore_addr);
     }
 
-    ASSERT(semaphore->available_count == 0);
+    ASSERT(semaphore->GetAvailableCount() == 0);
     ASSERT(semaphore->mutex_addr == mutex_addr);
 
     auto wakeup_callback = [mutex, nano_seconds](ThreadWakeupReason reason,
-- 
cgit v1.2.3