Fix some races in SvcThreadSync and change the way how yield works

Ryujinx.Core/OsHle/Handles/KProcessScheduler.cs

@@ -1,6 +1,7 @@
 using Ryujinx.Core.Logging;
 using System;
 using System.Collections.Concurrent;
+using System.Threading;
 
 namespace Ryujinx.Core.OsHle.Handles
 {
@@ -10,7 +11,7 @@ namespace Ryujinx.Core.OsHle.Handles
 
         private ThreadQueue WaitingToRun;
 
-        private int ActiveCores;
+        private KThread[] CoreThreads;
 
         private object SchedLock;
 
@@ -24,6 +25,8 @@ namespace Ryujinx.Core.OsHle.Handles
 
             WaitingToRun = new ThreadQueue();
 
+            CoreThreads = new KThread[4];
+
             SchedLock = new object();
         }
 
@@ -38,7 +41,7 @@ namespace Ryujinx.Core.OsHle.Handles
                     return;
                 }
 
-                if (AddActiveCore(Thread))
+                if (TryAddToCore(Thread))
                 {
                     Thread.Thread.Execute();
 
@@ -74,7 +77,7 @@ namespace Ryujinx.Core.OsHle.Handles
                 {
                     Log.PrintDebug(LogClass.KernelScheduler, $"Nothing to run on core {ActualCore}!");
 
-                    RemoveActiveCore(ActualCore);
+                    CoreThreads[ActualCore] = null;
 
                     return;
                 }
@@ -104,44 +107,38 @@ namespace Ryujinx.Core.OsHle.Handles
             }
         }
 
-        public void EnterWait(KThread Thread)
+        public void EnterWait(KThread Thread, int TimeoutMs = Timeout.Infinite)
         {
-            if (!AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
-            {
-                throw new InvalidOperationException();
-            }
+            SchedulerThread SchedThread = AllThreads[Thread];
 
             Suspend(Thread);
 
-            SchedThread.WaitSync.WaitOne();
+            SchedThread.WaitSync.WaitOne(TimeoutMs);
 
             TryResumingExecution(SchedThread);
         }
 
-        public bool EnterWait(KThread Thread, int Timeout)
+        public void WakeUp(KThread Thread)
         {
-            if (!AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
-            {
-                throw new InvalidOperationException();
-            }
-
-            Suspend(Thread);
-
-            bool Result = SchedThread.WaitSync.WaitOne(Timeout);
-
-            TryResumingExecution(SchedThread);
-
-            return Result;
+            AllThreads[Thread].WaitSync.Set();
         }
 
-        public void WakeUp(KThread Thread)
+        public void TryToRun(KThread Thread)
         {
-            if (!AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
+            lock (SchedLock)
             {
-                throw new InvalidOperationException();
+                if (AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
+                {
+                    if (WaitingToRun.HasThread(SchedThread) && TryAddToCore(Thread))
+                    {
+                        RunThread(SchedThread);
+                    }
+                    else
+                    {
+                        SetReschedule(Thread.ProcessorId);
+                    }
+                }
             }
-
-            SchedThread.WaitSync.Set();
         }
 
         public void Suspend(KThread Thread)
@@ -150,6 +147,8 @@ namespace Ryujinx.Core.OsHle.Handles
             {
                 PrintDbgThreadInfo(Thread, "suspended.");
 
+                AllThreads[Thread].NeedsReschedule = false;
+
                 int ActualCore = Thread.ActualCore;
 
                 SchedulerThread SchedThread = WaitingToRun.Pop(ActualCore);
@@ -158,84 +157,76 @@ namespace Ryujinx.Core.OsHle.Handles
                 {
                     SchedThread.Thread.ActualCore = ActualCore;
 
+                    CoreThreads[ActualCore] = SchedThread.Thread;
+
                     RunThread(SchedThread);
                 }
                 else
                 {
                     Log.PrintDebug(LogClass.KernelScheduler, $"Nothing to run on core {Thread.ActualCore}!");
 
-                    RemoveActiveCore(ActualCore);
+                    CoreThreads[ActualCore] = null;
+                }
+            }
+        }
+
+        public void SetReschedule(int Core)
+        {
+            lock (SchedLock)
+            {
+                KThread Thread = CoreThreads[Core];
+
+                if (Thread != null && AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
+                {
+                    SchedThread.NeedsReschedule = true;
                 }
             }
         }
 
-        public void Yield(KThread Thread)
+        public void Reschedule(KThread Thread)
         {
-            PrintDbgThreadInfo(Thread, "yielded execution.");
+            SchedulerThread SchedThread = AllThreads[Thread];
 
-            if (IsActive(Thread))
+            bool NeedsReschedule;
+
+            lock (SchedLock)
             {
+                NeedsReschedule = SchedThread.NeedsReschedule;
+
+                SchedThread.NeedsReschedule = false;
+            }
+
+            if (NeedsReschedule)
+            {
+                PrintDbgThreadInfo(Thread, "yielded execution.");
+
                 lock (SchedLock)
                 {
                     int ActualCore = Thread.ActualCore;
 
-                    SchedulerThread SchedThread = WaitingToRun.Pop(ActualCore, Thread.ActualPriority);
+                    SchedulerThread NewThread = WaitingToRun.Pop(ActualCore, Thread.ActualPriority);
 
-                    if (SchedThread == null)
+                    if (NewThread == null)
                     {
                         PrintDbgThreadInfo(Thread, "resumed because theres nothing better to run.");
 
                         return;
                     }
 
-                    if (SchedThread != null)
-                    {
-                        SchedThread.Thread.ActualCore = ActualCore;
-
-                        RunThread(SchedThread);
-                    }
-                }
-            }
-            else
-            {
-                //Just stop running the thread if it's not active,
-                //and run whatever is waiting to run with the higuest priority.
-                Suspend(Thread);
-            }
-
-            Resume(Thread);
-        }
+                    NewThread.Thread.ActualCore = ActualCore;
 
-        public bool TryRunning(KThread Thread)
-        {
-            //Failing to get the thread here is fine,
-            //the thread may not have been started yet.
-            if (AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
-            {
-                lock (SchedLock)
-                {
-                    if (WaitingToRun.HasThread(SchedThread) && AddActiveCore(Thread))
-                    {
-                        WaitingToRun.Remove(SchedThread);
-
-                        RunThread(SchedThread);
+                    CoreThreads[ActualCore] = NewThread.Thread;
 
-                        return true;
-                    }
+                    RunThread(NewThread);
                 }
-            }
 
-            return false;
+                TryResumingExecution(SchedThread);
+            }
         }
 
         public void Resume(KThread Thread)
         {
-            if (!AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
-            {
-                throw new InvalidOperationException();
-            }
-
-            TryResumingExecution(SchedThread);
+            TryResumingExecution(AllThreads[Thread]);
         }
 
         private void TryResumingExecution(SchedulerThread SchedThread)
@@ -248,7 +239,7 @@ namespace Ryujinx.Core.OsHle.Handles
 
             lock (SchedLock)
             {
-                if (AddActiveCore(Thread))
+                if (TryAddToCore(Thread))
                 {
                     PrintDbgThreadInfo(Thread, "resuming execution...");
 
@@ -257,6 +248,8 @@ namespace Ryujinx.Core.OsHle.Handles
 
                 WaitingToRun.Push(SchedThread);
 
+                SetReschedule(Thread.ProcessorId);
+
                 PrintDbgThreadInfo(Thread, "entering wait state...");
             }
 
@@ -287,71 +280,36 @@ namespace Ryujinx.Core.OsHle.Handles
             }
         }
 
-        private bool IsActive(KThread Thread)
-        {
-            if (!AllThreads.TryGetValue(Thread, out SchedulerThread SchedThread))
-            {
-                throw new InvalidOperationException();
-            }
-
-            return SchedThread.IsActive;
-        }
-
-        private bool AddActiveCore(KThread Thread)
+        private bool TryAddToCore(KThread Thread)
         {
-            int CoreMask;
+            //First, try running it on Ideal Core.
+            int IdealCore = Thread.IdealCore;
 
-            lock (SchedLock)
+            if (IdealCore != -1 && CoreThreads[IdealCore] == null)
             {
-                //First, try running it on Ideal Core.
-                int IdealCore = Thread.IdealCore;
+                Thread.ActualCore = IdealCore;
 
-                if (IdealCore != -1)
-                {
-                    CoreMask = 1 << IdealCore;
-
-                    if ((ActiveCores & CoreMask) == 0)
-                    {
-                        ActiveCores |= CoreMask;
+                CoreThreads[IdealCore] = Thread;
 
-                        Thread.ActualCore = IdealCore;
-
-                        return true;
-                    }
-                }
+                return true;
+            }
 
-                //If that fails, then try running on any core allowed by Core Mask.
-                CoreMask = Thread.CoreMask & ~ActiveCores;
+            //If that fails, then try running on any core allowed by Core Mask.
+            int CoreMask = Thread.CoreMask;
 
-                if (CoreMask != 0)
+            for (int Core = 0; Core < CoreThreads.Length; Core++, CoreMask >>= 1)
+            {
+                if ((CoreMask & 1) != 0 && CoreThreads[Core] == null)
                 {
-                    CoreMask &= -CoreMask;
+                    Thread.ActualCore = Core;
 
-                    ActiveCores |= CoreMask;
-
-                    for (int Bit = 0; Bit < 32; Bit++)
-                    {
-                        if (((CoreMask >> Bit) & 1) != 0)
-                        {
-                            Thread.ActualCore = Bit;
+                    CoreThreads[Core] = Thread;
 
-                            return true;
-                        }
-                    }
-
-                    throw new InvalidOperationException();
+                    return true;
                 }
-
-                return false;
             }
-        }
 
-        private void RemoveActiveCore(int Core)
-        {
-            lock (SchedLock)
-            {
-                ActiveCores &= ~(1 << Core);
-            }
+            return false;
         }
 
         private void PrintDbgThreadInfo(KThread Thread, string Message)

Ryujinx.Core/OsHle/Handles/KThread.cs

@@ -12,6 +12,8 @@ namespace Ryujinx.Core.OsHle.Handles
         public long MutexAddress   { get; set; }
         public long CondVarAddress { get; set; }
 
+        public bool CondVarSignaled { get; set; }
+
         private Process Process;
 
         public List<KThread> MutexWaiters { get; private set; }
@@ -21,8 +23,9 @@ namespace Ryujinx.Core.OsHle.Handles
         public int ActualPriority { get; private set; }
         public int WantedPriority { get; private set; }
 
-        public int ActualCore { get; set; }
-        public int IdealCore  { get; set; }
+        public int ActualCore  { get; set; }
+        public int ProcessorId { get; set; }
+        public int IdealCore   { get; set; }
 
         public int WaitHandle { get; set; }
 
@@ -31,16 +34,17 @@ namespace Ryujinx.Core.OsHle.Handles
         public KThread(
             AThread Thread,
             Process Process,
-            int     IdealCore,
+            int     ProcessorId,
             int     Priority)
         {
-            this.Thread    = Thread;
-            this.Process   = Process;
-            this.IdealCore = IdealCore;
+            this.Thread      = Thread;
+            this.Process     = Process;
+            this.ProcessorId = ProcessorId;
+            this.IdealCore   = ProcessorId;
 
             MutexWaiters = new List<KThread>();
 
-            CoreMask = 1 << IdealCore;
+            CoreMask = 1 << ProcessorId;
 
             ActualPriority = WantedPriority = Priority;
         }

Ryujinx.Core/OsHle/Handles/SchedulerThread.cs

@@ -11,6 +11,8 @@ namespace Ryujinx.Core.OsHle.Handles
 
         public bool IsActive { get; set; }
 
+        public bool NeedsReschedule { get; set; }
+
         public AutoResetEvent   WaitSync     { get; private set; }
         public ManualResetEvent WaitActivity { get; private set; }
         public AutoResetEvent   WaitSched    { get; private set; }

Ryujinx.Core/OsHle/Kernel/SvcHandler.cs

@@ -96,6 +96,8 @@ namespace Ryujinx.Core.OsHle.Kernel
 
                 Func(ThreadState);
 
+                Process.Scheduler.Reschedule(Process.GetThread(ThreadState.Tpidr));
+
                 Ns.Log.PrintDebug(LogClass.KernelSvc, $"{Func.Method.Name} ended.");
             }
             else

Ryujinx.Core/OsHle/Kernel/SvcThread.cs

@@ -55,13 +55,12 @@ namespace Ryujinx.Core.OsHle.Kernel
         {
             int Handle = (int)ThreadState.X0;
 
-            KThread CurrThread = Process.HandleTable.GetData<KThread>(Handle);
+            KThread NewThread = Process.HandleTable.GetData<KThread>(Handle);
 
-            if (CurrThread != null)
+            if (NewThread != null)
             {
-                Process.Scheduler.StartThread(CurrThread);
-
-                Process.Scheduler.Yield(Process.GetThread(ThreadState.Tpidr));
+                Process.Scheduler.StartThread(NewThread);
+                Process.Scheduler.SetReschedule(NewThread.ProcessorId);
 
                 ThreadState.X0 = 0;
             }
@@ -82,19 +81,19 @@ namespace Ryujinx.Core.OsHle.Kernel
 
         private void SvcSleepThread(AThreadState ThreadState)
         {
-            ulong Ns = ThreadState.X0;
+            ulong TimeoutNs = ThreadState.X0;
 
             KThread CurrThread = Process.GetThread(ThreadState.Tpidr);
 
-            if (Ns == 0)
+            if (TimeoutNs == 0)
             {
-                Process.Scheduler.Yield(CurrThread);
+                Process.Scheduler.SetReschedule(CurrThread.ActualCore);
             }
             else
             {
                 Process.Scheduler.Suspend(CurrThread);
 
-                Thread.Sleep(NsTimeConverter.GetTimeMs(Ns));
+                Thread.Sleep(NsTimeConverter.GetTimeMs(TimeoutNs));
 
                 Process.Scheduler.Resume(CurrThread);
             }
@@ -210,15 +209,7 @@ namespace Ryujinx.Core.OsHle.Kernel
 
             Thread.CoreMask = (int)CoreMask;
 
-            KThread CurrThread = Process.GetThread(ThreadState.Tpidr);
-
-            //Try yielding execution, for the case where the new
-            //core mask allows the thread to run on the current core.
-            Process.Scheduler.Yield(CurrThread);
-
-            //Try running the modified thread, for the case where one
-            //of the cores specified on the core mask is free.
-            Process.Scheduler.TryRunning(Thread);
+            Process.Scheduler.TryToRun(Thread);
 
             ThreadState.X0 = 0;
         }

Ryujinx.Core/OsHle/Kernel/SvcThreadSync.cs

@@ -96,10 +96,7 @@ namespace Ryujinx.Core.OsHle.Kernel
                 return;
             }
 
-            if (MutexUnlock(Process.GetThread(ThreadState.Tpidr), MutexAddress))
-            {
-                Process.Scheduler.Yield(Process.GetThread(ThreadState.Tpidr));
-            }
+            MutexUnlock(Process.GetThread(ThreadState.Tpidr), MutexAddress);
 
             ThreadState.X0 = 0;
         }
@@ -157,8 +154,6 @@ namespace Ryujinx.Core.OsHle.Kernel
                 return;
             }
 
-            Process.Scheduler.Yield(Process.GetThread(ThreadState.Tpidr));
-
             ThreadState.X0 = 0;
         }
 
@@ -167,7 +162,13 @@ namespace Ryujinx.Core.OsHle.Kernel
             long CondVarAddress = (long)ThreadState.X0;
             int  Count          =  (int)ThreadState.X1;
 
-            CondVarSignal(CondVarAddress, Count);
+            Ns.Log.PrintDebug(LogClass.KernelSvc,
+                "CondVarAddress = " + CondVarAddress.ToString("x16") + ", " +
+                "Count = "          + Count         .ToString("x8"));
+
+            KThread CurrThread = Process.GetThread(ThreadState.Tpidr);
+
+            CondVarSignal(CurrThread, CondVarAddress, Count);
 
             ThreadState.X0 = 0;
         }
@@ -179,12 +180,12 @@ namespace Ryujinx.Core.OsHle.Kernel
             int     WaitThreadHandle,
             long    MutexAddress)
         {
-            int MutexValue = Process.Memory.ReadInt32(MutexAddress);
-
-            Ns.Log.PrintDebug(LogClass.KernelSvc, "MutexValue = " + MutexValue.ToString("x8"));
-
             lock (Process.ThreadSyncLock)
             {
+                int MutexValue = Process.Memory.ReadInt32(MutexAddress);
+
+                Ns.Log.PrintDebug(LogClass.KernelSvc, "MutexValue = " + MutexValue.ToString("x8"));
+
                 if (MutexValue != (OwnerThreadHandle | MutexHasListenersMask))
                 {
                     return;
@@ -201,20 +202,13 @@ namespace Ryujinx.Core.OsHle.Kernel
             Process.Scheduler.EnterWait(CurrThread);
         }
 
-        private bool MutexUnlock(KThread CurrThread, long MutexAddress)
+        private void MutexUnlock(KThread CurrThread, long MutexAddress)
         {
-            if (CurrThread == null)
-            {
-                Ns.Log.PrintWarning(LogClass.KernelSvc, $"Invalid mutex 0x{MutexAddress:x16}!");
-
-                return false;
-            }
-
             lock (Process.ThreadSyncLock)
             {
                 //This is the new thread that will not own the mutex.
                 //If no threads are waiting for the lock, then it should be null.
-                KThread OwnerThread = GetHighestPriority(CurrThread.MutexWaiters, MutexAddress);
+                KThread OwnerThread = PopThread(CurrThread.MutexWaiters, x => x.MutexAddress == MutexAddress);
 
                 if (OwnerThread != null)
                 {
@@ -240,16 +234,12 @@ namespace Ryujinx.Core.OsHle.Kernel
                     Process.Scheduler.WakeUp(OwnerThread);
 
                     Ns.Log.PrintDebug(LogClass.KernelSvc, "Gave mutex to thread id " + OwnerThread.ThreadId + "!");
-
-                    return true;
                 }
                 else
                 {
                     Process.Memory.WriteInt32(MutexAddress, 0);
 
                     Ns.Log.PrintDebug(LogClass.KernelSvc, "No threads waiting mutex!");
-
-                    return false;
                 }
             }
         }
@@ -265,8 +255,10 @@ namespace Ryujinx.Core.OsHle.Kernel
             WaitThread.MutexAddress   = MutexAddress;
             WaitThread.CondVarAddress = CondVarAddress;
 
-            lock (Process.ThreadArbiterListLock)
+            lock (Process.ThreadArbiterList)
             {
+                WaitThread.CondVarSignaled = false;
+
                 Process.ThreadArbiterList.Add(WaitThread);
             }
 
@@ -274,58 +266,77 @@ namespace Ryujinx.Core.OsHle.Kernel
 
             if (Timeout != ulong.MaxValue)
             {
-                return Process.Scheduler.EnterWait(WaitThread, NsTimeConverter.GetTimeMs(Timeout));
+                Process.Scheduler.EnterWait(WaitThread, NsTimeConverter.GetTimeMs(Timeout));
+
+                lock (Process.ThreadArbiterList)
+                {
+                    if (!WaitThread.CondVarSignaled)
+                    {
+                        Process.ThreadArbiterList.Remove(WaitThread);
+
+                        return false;
+                    }
+                }
             }
             else
             {
                 Process.Scheduler.EnterWait(WaitThread);
-
-                return true;
             }
+
+            return true;
         }
 
-        private void CondVarSignal(long CondVarAddress, int Count)
+        private void CondVarSignal(KThread CurrThread, long CondVarAddress, int Count)
         {
-            lock (Process.ThreadArbiterListLock)
+            lock (Process.ThreadArbiterList)
             {
-                KThread CurrThread = GetHighestPriority(CondVarAddress);
-
-                while (CurrThread != null && (Count == -1 || Count-- > 0))
+                while (Count == -1 || Count-- > 0)
                 {
-                    AcquireMutexValue(CurrThread.MutexAddress);
+                    KThread WaitThread = PopThread(Process.ThreadArbiterList, x => x.CondVarAddress == CondVarAddress);
+
+                    if (WaitThread == null)
+                    {
+                        Ns.Log.PrintDebug(LogClass.KernelSvc, "No more threads to wake up!");
+
+                        break;
+                    }
 
-                    int MutexValue = Process.Memory.ReadInt32(CurrThread.MutexAddress);
+                    WaitThread.CondVarSignaled = true;
+
+                    AcquireMutexValue(WaitThread.MutexAddress);
+
+                    int MutexValue = Process.Memory.ReadInt32(WaitThread.MutexAddress);
+
+                    Ns.Log.PrintDebug(LogClass.KernelSvc, "MutexValue = " + MutexValue.ToString("x8"));
 
                     if (MutexValue == 0)
                     {
                         //Give the lock to this thread.
-                        Process.Memory.WriteInt32(CurrThread.MutexAddress, CurrThread.WaitHandle);
+                        Process.Memory.WriteInt32(WaitThread.MutexAddress, WaitThread.WaitHandle);
 
-                        CurrThread.WaitHandle     = 0;
-                        CurrThread.MutexAddress   = 0;
-                        CurrThread.CondVarAddress = 0;
+                        WaitThread.WaitHandle     = 0;
+                        WaitThread.MutexAddress   = 0;
+                        WaitThread.CondVarAddress = 0;
 
-                        CurrThread.MutexOwner?.UpdatePriority();
+                        WaitThread.MutexOwner?.UpdatePriority();
 
-                        CurrThread.MutexOwner = null;
+                        WaitThread.MutexOwner = null;
 
-                        Process.Scheduler.WakeUp(CurrThread);
+                        Process.Scheduler.WakeUp(WaitThread);
                     }
                     else
                     {
                         //Wait until the lock is released.
                         MutexValue &= ~MutexHasListenersMask;
 
-                        InsertWaitingMutexThread(MutexValue, CurrThread);
+                        InsertWaitingMutexThread(MutexValue, WaitThread);
 
                         MutexValue |= MutexHasListenersMask;
 
-                        Process.Memory.WriteInt32(CurrThread.MutexAddress, MutexValue);
+                        Process.Memory.WriteInt32(WaitThread.MutexAddress, MutexValue);
                     }
 
-                    ReleaseMutexValue(CurrThread.MutexAddress);
-
-                    CurrThread = GetHighestPriority(CondVarAddress);
+                    ReleaseMutexValue(WaitThread.MutexAddress);
                 }
             }
         }
@@ -381,17 +392,7 @@ namespace Ryujinx.Core.OsHle.Kernel
             }
         }
 
-        private KThread GetHighestPriority(List<KThread> Threads, long MutexAddress)
-        {
-            return GetHighestPriority(Threads, x => x.MutexAddress == MutexAddress);
-        }
-
-        private KThread GetHighestPriority(long CondVarAddress)
-        {
-            return GetHighestPriority(Process.ThreadArbiterList, x => x.CondVarAddress == CondVarAddress);
-        }
-
-        private KThread GetHighestPriority(List<KThread> Threads, Func<KThread, bool> Predicate)
+        private KThread PopThread(List<KThread> Threads, Func<KThread, bool> Predicate)
         {
             KThread Thread = Threads.OrderBy(x => x.ActualPriority).FirstOrDefault(Predicate);
 

Ryujinx.Core/OsHle/Process.cs

@@ -40,8 +40,6 @@ namespace Ryujinx.Core.OsHle
 
         public List<KThread> ThreadArbiterList { get; private set; }
 
-        public object ThreadArbiterListLock { get; private set; }
-
         public object ThreadSyncLock { get; private set; }
 
         public KProcessHandleTable HandleTable { get; private set; }
@@ -76,8 +74,6 @@ namespace Ryujinx.Core.OsHle
 
             ThreadArbiterList = new List<KThread>();
 
-            ThreadArbiterListLock = new object();
-
             ThreadSyncLock = new object();
 
             HandleTable = new KProcessHandleTable();