Addition of Conditional Variables

This commit adds working conditional variables, in addition to the mutex and threading implementation. It directly depends on the memory optimization from the previous commit to be able to perform atomic operations on the mutex.

app/src/main/cpp/skyline/common.h

@@ -43,7 +43,7 @@ namespace skyline {
         constexpr u64 MaxSyncHandles = 0x40; //!< The total amount of handles that can be passed to WaitSynchronization
         constexpr handle_t BaseHandleIndex = 0xD000; // The index of the base handle
         constexpr handle_t ThreadSelf = 0xFFFF8000; //!< This is the handle used by threads to refer to themselves
-        constexpr u8 DefaultPriority = 31; //!< The default priority of a process
+        constexpr u8 DefaultPriority = 44; //!< The default priority of a process
         constexpr std::pair<int8_t, int8_t> PriorityAn = {19, -8}; //!< The range of priority for Android, taken from https://medium.com/mindorks/exploring-android-thread-priority-5d0542eebbd1
         constexpr std::pair<u8, u8> PriorityNin = {0, 63}; //!< The range of priority for the Nintendo Switch
         constexpr u32 MtxOwnerMask = 0xBFFFFFFF; //!< The mask of values which contain the owner of a mutex

app/src/main/cpp/skyline/kernel/svc.cpp

@@ -414,42 +414,44 @@ namespace skyline::kernel::svc {
     void ArbitrateLock(DeviceState &state) {
         auto address = state.ctx->registers.x1;
         if (!utils::WordAligned(address)) {
-            state.ctx->registers.w0 = constant::status::InvAddress;
             state.logger->Warn("svcArbitrateLock: 'address' not word aligned: 0x{:X}", address);
+            state.ctx->registers.w0 = constant::status::InvAddress;
             return;
         }
         auto ownerHandle = state.ctx->registers.w0;
         auto requesterHandle = state.ctx->registers.w2;
         if (requesterHandle != state.thread->handle)
             throw exception("svcWaitProcessWideKeyAtomic: Handle doesn't match current thread: 0x{:X} for thread 0x{:X}", requesterHandle, state.thread->handle);
-        state.logger->Debug("svcArbitrateLock: Locking mutex at 0x{:X} for thread {}", address, state.thread->pid);
+        state.logger->Debug("svcArbitrateLock: Locking mutex at 0x{:X}", address);
-        state.process->MutexLock(address, ownerHandle);
+        if (state.process->MutexLock(address, ownerHandle))
-        state.logger->Debug("svcArbitrateLock: Locked mutex at 0x{:X} for thread {}", address, state.thread->pid);
+            state.logger->Debug("svcArbitrateLock: Locked mutex at 0x{:X}", address);
+        else
+            state.logger->Debug("svcArbitrateLock: Owner handle did not match current owner for mutex at 0x{:X}", address);
         state.ctx->registers.w0 = constant::status::Success;
     }
 
     void ArbitrateUnlock(DeviceState &state) {
         auto address = state.ctx->registers.x0;
         if (!utils::WordAligned(address)) {
-            state.ctx->registers.w0 = constant::status::InvAddress;
             state.logger->Warn("svcArbitrateUnlock: 'address' not word aligned: 0x{:X}", address);
+            state.ctx->registers.w0 = constant::status::InvAddress;
             return;
         }
         state.logger->Debug("svcArbitrateUnlock: Unlocking mutex at 0x{:X}", address);
         if (state.process->MutexUnlock(address)) {
-            state.ctx->registers.w0 = constant::status::Success;
             state.logger->Debug("svcArbitrateUnlock: Unlocked mutex at 0x{:X}", address);
+            state.ctx->registers.w0 = constant::status::Success;
         } else {
-            state.ctx->registers.w0 = constant::status::InvAddress;
             state.logger->Debug("svcArbitrateUnlock: A non-owner thread tried to release a mutex at 0x{:X}", address);
+            state.ctx->registers.w0 = constant::status::InvAddress;
         }
     }
 
     void WaitProcessWideKeyAtomic(DeviceState &state) {
         auto mtxAddress = state.ctx->registers.x0;
         if (!utils::WordAligned(mtxAddress)) {
-            state.ctx->registers.w0 = constant::status::InvAddress;
             state.logger->Warn("svcWaitProcessWideKeyAtomic: mutex address not word aligned: 0x{:X}", mtxAddress);
+            state.ctx->registers.w0 = constant::status::InvAddress;
             return;
         }
         auto condAddress = state.ctx->registers.x1;
@@ -457,19 +459,18 @@ namespace skyline::kernel::svc {
         if (handle != state.thread->handle)
             throw exception("svcWaitProcessWideKeyAtomic: Handle doesn't match current thread: 0x{:X} for thread 0x{:X}", handle, state.thread->handle);
         if (!state.process->MutexUnlock(mtxAddress)) {
-            state.ctx->registers.w0 = constant::status::InvAddress;
             state.logger->Debug("WaitProcessWideKeyAtomic: A non-owner thread tried to release a mutex at 0x{:X}", mtxAddress);
+            state.ctx->registers.w0 = constant::status::InvAddress;
             return;
         }
         auto timeout = state.ctx->registers.x3;
         state.logger->Debug("svcWaitProcessWideKeyAtomic: Mutex: 0x{:X}, Conditional-Variable: 0x{:X}, Timeout: {} ns", mtxAddress, condAddress, timeout);
-        if (state.process->ConditionalVariableWait(condAddress, timeout)) {
+        if (state.process->ConditionalVariableWait(condAddress, mtxAddress, timeout)) {
-            state.ctx->registers.w0 = constant::status::Success;
-            state.process->MutexLock(mtxAddress, handle, true);
             state.logger->Debug("svcWaitProcessWideKeyAtomic: Waited for conditional variable and relocked mutex");
+            state.ctx->registers.w0 = constant::status::Success;
         } else {
-            state.ctx->registers.w0 = constant::status::Timeout;
             state.logger->Debug("svcWaitProcessWideKeyAtomic: Wait has timed out");
+            state.ctx->registers.w0 = constant::status::Timeout;
         }
     }
 

app/src/main/cpp/skyline/kernel/types/KProcess.cpp

@@ -165,65 +165,66 @@ namespace skyline::kernel::type {
         return std::nullopt;
     }
 
-    void KProcess::MutexLock(u64 address, handle_t owner, bool alwaysLock) {
+    bool KProcess::MutexLock(u64 address, handle_t owner) {
         std::unique_lock lock(mutexLock);
-        u32 mtxVal = ReadMemory<u32>(address);
+        auto mtx = GetPointer<u32>(address);
-        if (alwaysLock) {
-            if (!mtxVal) {
-                state.logger->Warn("Mutex Value was 0");
-                mtxVal = (constant::MtxOwnerMask & state.thread->handle);
-                WriteMemory<u32>(mtxVal, address);
-                return;
-                // TODO: Replace with atomic CAS
-            }
-        } else {
-            if (mtxVal != (owner | ~constant::MtxOwnerMask))
-                return;
-        }
         auto &mtxWaiters = mutexes[address];
+        u32 mtxExpected = 0;
+        if (__atomic_compare_exchange_n(mtx, &mtxExpected, (constant::MtxOwnerMask & state.thread->handle) | (mtxWaiters.empty() ? 0 : ~constant::MtxOwnerMask), false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
+            return true;
+        if (owner && (__atomic_load_n(mtx, __ATOMIC_SEQ_CST) != (owner | ~constant::MtxOwnerMask)))
+            return false;
         std::shared_ptr<WaitStatus> status;
         for (auto it = mtxWaiters.begin();; ++it) {
             if (it != mtxWaiters.end() && (*it)->priority >= state.thread->priority)
                 continue;
-            status = std::make_shared<WaitStatus>(state.thread->priority, state.thread->pid);
+            status = std::make_shared<WaitStatus>(state.thread->priority, state.thread->handle);
             mtxWaiters.insert(it, status);
             break;
         }
         lock.unlock();
         while (!status->flag);
         lock.lock();
+        status->flag = false;
         for (auto it = mtxWaiters.begin(); it != mtxWaiters.end(); ++it)
-            if ((*it)->pid == state.thread->pid) {
+            if ((*it)->handle == state.thread->handle) {
                 mtxWaiters.erase(it);
                 break;
             }
-        mtxVal = (constant::MtxOwnerMask & state.thread->handle) | (mtxWaiters.empty() ? 0 : ~constant::MtxOwnerMask);
-        WriteMemory<u32>(mtxVal, address);
-        lock.unlock();
-    }
-
-    bool KProcess::MutexUnlock(u64 address) {
-        std::lock_guard lock(mutexLock);
-        u32 mtxVal = ReadMemory<u32>(address);
-        if ((mtxVal & constant::MtxOwnerMask) != state.thread->handle)
-            return false;
-        auto &mtxWaiters = mutexes[address];
-        if (mtxWaiters.empty()) {
-            mtxVal = 0;
-            WriteMemory<u32>(mtxVal, address);
-        } else
-            (*mtxWaiters.begin())->flag = true;
         return true;
     }
 
-    bool KProcess::ConditionalVariableWait(u64 address, u64 timeout) {
+    bool KProcess::MutexUnlock(u64 address) {
+        std::unique_lock lock(mutexLock);
+        auto mtx = GetPointer<u32>(address);
+        auto &mtxWaiters = mutexes[address];
+        u32 mtxDesired{};
+        if (!mtxWaiters.empty())
+            mtxDesired = (*mtxWaiters.begin())->handle | ((mtxWaiters.size() > 1) ? ~constant::MtxOwnerMask : 0);
+        u32 mtxExpected = (constant::MtxOwnerMask & state.thread->handle) | ~constant::MtxOwnerMask;
+        if (!__atomic_compare_exchange_n(mtx, &mtxExpected, mtxDesired, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) {
+            mtxExpected = constant::MtxOwnerMask & state.thread->handle;
+            if (!__atomic_compare_exchange_n(mtx, &mtxExpected, mtxDesired, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
+                return false;
+        }
+        if (mtxDesired) {
+            auto status = (*mtxWaiters.begin());
+            status->flag = true;
+            lock.unlock();
+            while(status->flag);
+            lock.lock();
+        }
+        return true;
+    }
+
+    bool KProcess::ConditionalVariableWait(u64 conditionalAddress, u64 mutexAddress, u64 timeout) {
         std::unique_lock lock(conditionalLock);
-        auto &condWaiters = conditionals[address];
+        auto &condWaiters = conditionals[conditionalAddress];
         std::shared_ptr<WaitStatus> status;
         for (auto it = condWaiters.begin();; ++it) {
             if (it != condWaiters.end() && (*it)->priority >= state.thread->priority)
                 continue;
-            status = std::make_shared<WaitStatus>(state.thread->priority, state.thread->pid);
+            status = std::make_shared<WaitStatus>(state.thread->priority, state.thread->handle, mutexAddress);
             condWaiters.insert(it, status);
             break;
         }
@@ -235,8 +236,12 @@ namespace skyline::kernel::type {
                 timedOut = true;
         }
         lock.lock();
+        if (!status->flag)
+            timedOut = false;
+        else
+            status->flag = false;
         for (auto it = condWaiters.begin(); it != condWaiters.end(); ++it)
-            if ((*it)->pid == state.thread->pid) {
+            if ((*it)->handle == state.thread->handle) {
                 condWaiters.erase(it);
                 break;
             }
@@ -245,10 +250,56 @@ namespace skyline::kernel::type {
     }
 
     void KProcess::ConditionalVariableSignal(u64 address, u64 amount) {
-        std::lock_guard lock(conditionalLock);
+        std::unique_lock condLock(conditionalLock);
         auto &condWaiters = conditionals[address];
-        amount = std::min(condWaiters.size(), amount);
+        u64 count{};
-        for (size_t i = 0; i < amount; ++i)
+        auto iter = condWaiters.begin();
-            condWaiters[i]->flag = true;
+        while (iter != condWaiters.end() && count < amount) {
+            auto &thread = *iter;
+            auto mtx = GetPointer<u32>(thread->mutexAddress);
+            u32 mtxValue = __atomic_load_n(mtx, __ATOMIC_SEQ_CST);
+            while (true) {
+                u32 mtxDesired{};
+                if (!mtxValue)
+                    mtxDesired = (constant::MtxOwnerMask & thread->handle);
+                else if ((mtxValue & constant::MtxOwnerMask) == state.thread->handle)
+                    mtxDesired = mtxValue | (constant::MtxOwnerMask & thread->handle);
+                else if (mtxValue & ~constant::MtxOwnerMask)
+                    mtxDesired = mtxValue | ~constant::MtxOwnerMask;
+                else
+                    break;
+                if (__atomic_compare_exchange_n(mtx, &mtxValue, mtxDesired, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
+                    break;
+            }
+            if (mtxValue && ((mtxValue & constant::MtxOwnerMask) != state.thread->handle)) {
+                std::unique_lock mtxLock(mutexLock);
+                auto &mtxWaiters = mutexes[thread->mutexAddress];
+                std::shared_ptr<WaitStatus> status;
+                for (auto it = mtxWaiters.begin();; ++it) {
+                    if (it != mtxWaiters.end() && (*it)->priority >= thread->priority)
+                        continue;
+                    status = std::make_shared<WaitStatus>(thread->priority, thread->handle);
+                    mtxWaiters.insert(it, status);
+                    break;
+                }
+                mtxLock.unlock();
+                while (!status->flag);
+                mtxLock.lock();
+                status->flag = false;
+                for (auto it = mtxWaiters.begin(); it != mtxWaiters.end(); ++it) {
+                    if ((*it)->handle == thread->handle) {
+                        mtxWaiters.erase(it);
+                        break;
+                    }
+                }
+                mtxLock.unlock();
+            }
+            thread->flag = true;
+            iter++;
+            count++;
+            condLock.unlock();
+            while(thread->flag);
+            condLock.lock();
+        }
     }
 }

app/src/main/cpp/skyline/kernel/types/KProcess.h

@@ -7,7 +7,7 @@
 #include "KSession.h"
 #include "KEvent.h"
 #include <kernel/memory.h>
-#include <condition_variable>
+#include <list>
 
 namespace skyline::kernel::type {
     /**
@@ -91,9 +91,12 @@ namespace skyline::kernel::type {
         struct WaitStatus {
             std::atomic_bool flag{false}; //!< The underlying atomic flag of the thread
             u8 priority; //!< The priority of the thread
-            pid_t pid; //!< The PID of the thread
+            handle_t handle; //!< The handle of the thread
+            u64 mutexAddress{}; //!< The address of the mutex
 
-            WaitStatus(u8 priority, pid_t pid) : priority(priority), pid(pid) {}
+            WaitStatus(u8 priority, handle_t handle) : priority(priority), handle(handle) {}
+
+            WaitStatus(u8 priority, handle_t handle, u64 mutexAddress) : priority(priority), handle(handle), mutexAddress(mutexAddress) {}
         };
 
         handle_t handleIndex = constant::BaseHandleIndex; //!< This is used to keep track of what to map as an handle
@@ -102,11 +105,11 @@ namespace skyline::kernel::type {
         std::unordered_map<handle_t, std::shared_ptr<KObject>> handles; //!< A mapping from a handle_t to it's corresponding KObject which is the actual underlying object
         std::unordered_map<pid_t, std::shared_ptr<KThread>> threads; //!< A mapping from a PID to it's corresponding KThread object
         std::unordered_map<u64, std::vector<std::shared_ptr<WaitStatus>>> mutexes; //!< A map from a mutex's address to a vector of Mutex objects for threads waiting on it
-        std::unordered_map<u64, std::vector<std::shared_ptr<WaitStatus>>> conditionals; //!< A map from a conditional variable's address to a vector of threads waiting on it
+        std::unordered_map<u64, std::list<std::shared_ptr<WaitStatus>>> conditionals; //!< A map from a conditional variable's address to a vector of threads waiting on it
         std::vector<std::shared_ptr<TlsPage>> tlsPages; //!< A vector of all allocated TLS pages
         std::shared_ptr<KPrivateMemory> heap; //!< The kernel memory object backing the allocated heap
-        Mutex mutexLock; //!< This Mutex is to prevent concurrent mutex operations to happen at once
+        Mutex mutexLock; //!< This mutex is to prevent concurrent mutex operations to happen at once
-        Mutex conditionalLock; //!< This Mutex is to prevent concurrent conditional variable operations to happen at once
+        Mutex conditionalLock; //!< This mutex is to prevent concurrent conditional variable operations to happen at once
 
         /**
          * @brief Creates a KThread object for the main thread and opens the process's memory file
@@ -296,9 +299,9 @@ namespace skyline::kernel::type {
          * @brief This locks the Mutex at the specified address
          * @param address The address of the mutex
          * @param owner The handle of the current mutex owner
-         * @param alwaysLock If to return rather than lock if owner tag is not matched
+         * @return If the mutex was successfully locked
          */
-        void MutexLock(u64 address, handle_t owner, bool alwaysLock = false);
+        bool MutexLock(u64 address, handle_t owner);
 
         /**
          * @brief This unlocks the Mutex at the specified address
@@ -308,11 +311,12 @@ namespace skyline::kernel::type {
         bool MutexUnlock(u64 address);
 
         /**
-         * @param address The address of the conditional variable
+         * @param conditionalAddress The address of the conditional variable
+         * @param mutexAddress The address of the mutex
          * @param timeout The amount of time to wait for the conditional variable
          * @return If the conditional variable was successfully waited for or timed out
          */
-        bool ConditionalVariableWait(u64 address, u64 timeout);
+        bool ConditionalVariableWait(u64 conditionalAddress, u64 mutexAddress, u64 timeout);
 
         /**
          * @brief This signals a number of conditional variable waiters