Kernel: Address Feedback.

src/core/hle/kernel/kernel.cpp

@@ -3,6 +3,7 @@
 // Refer to the license.txt file included.
 
 #include <atomic>
+#include <bitset>
 #include <functional>
 #include <memory>
 #include <mutex>
@@ -17,6 +18,7 @@
 #include "core/core.h"
 #include "core/core_timing.h"
 #include "core/core_timing_util.h"
+#include "core/hardware_properties.h"
 #include "core/hle/kernel/client_port.h"
 #include "core/hle/kernel/errors.h"
 #include "core/hle/kernel/handle_table.h"
@@ -188,6 +190,7 @@ struct KernelCore::Impl {
     }
 
     void RegisterCoreThread(std::size_t core_id) {
+        std::unique_lock lock{register_thread_mutex};
         const std::thread::id this_id = std::this_thread::get_id();
         const auto it = host_thread_ids.find(this_id);
         ASSERT(core_id < Core::Hardware::NUM_CPU_CORES);
@@ -198,13 +201,14 @@ struct KernelCore::Impl {
     }
 
     void RegisterHostThread() {
+        std::unique_lock lock{register_thread_mutex};
         const std::thread::id this_id = std::this_thread::get_id();
         const auto it = host_thread_ids.find(this_id);
         ASSERT(it == host_thread_ids.end());
         host_thread_ids[this_id] = registered_thread_ids++;
     }
 
-    u32 GetCurrentHostThreadId() const {
+    u32 GetCurrentHostThreadID() const {
         const std::thread::id this_id = std::this_thread::get_id();
         const auto it = host_thread_ids.find(this_id);
         if (it == host_thread_ids.end()) {
@@ -213,9 +217,9 @@ struct KernelCore::Impl {
         return it->second;
     }
 
-    Core::EmuThreadHandle GetCurrentEmuThreadId() const {
+    Core::EmuThreadHandle GetCurrentEmuThreadID() const {
         Core::EmuThreadHandle result = Core::EmuThreadHandle::InvalidHandle();
-        result.host_handle = GetCurrentHostThreadId();
+        result.host_handle = GetCurrentHostThreadID();
         if (result.host_handle >= Core::Hardware::NUM_CPU_CORES) {
             return result;
         }
@@ -246,8 +250,8 @@ struct KernelCore::Impl {
     std::shared_ptr<Core::Timing::EventType> thread_wakeup_event_type;
     std::shared_ptr<Core::Timing::EventType> preemption_event;
 
-    // TODO(yuriks): This can be removed if Thread objects are explicitly pooled in the future,
+    // This is the kernel's handle table or supervisor handle table which
-    // allowing us to simply use a pool index or similar.
+    // stores all the objects in place.
     Kernel::HandleTable global_handle_table;
 
     /// Map of named ports managed by the kernel, which can be retrieved using
@@ -257,10 +261,11 @@ struct KernelCore::Impl {
     std::unique_ptr<Core::ExclusiveMonitor> exclusive_monitor;
     std::vector<Kernel::PhysicalCore> cores;
 
-    // 0-3 Ids represent core threads, >3 represent others
+    // 0-3 IDs represent core threads, >3 represent others
     std::unordered_map<std::thread::id, u32> host_thread_ids;
     u32 registered_thread_ids{Core::Hardware::NUM_CPU_CORES};
-    std::bitset<Core::Hardware::NUM_CPU_CORES> registered_core_threads{};
+    std::bitset<Core::Hardware::NUM_CPU_CORES> registered_core_threads;
+    std::mutex register_thread_mutex;
 
     // System context
     Core::System& system;
@@ -420,12 +425,12 @@ void KernelCore::RegisterHostThread() {
     impl->RegisterHostThread();
 }
 
-u32 KernelCore::GetCurrentHostThreadId() const {
+u32 KernelCore::GetCurrentHostThreadID() const {
-    return impl->GetCurrentHostThreadId();
+    return impl->GetCurrentHostThreadID();
 }
 
-Core::EmuThreadHandle KernelCore::GetCurrentEmuThreadId() const {
+Core::EmuThreadHandle KernelCore::GetCurrentEmuThreadID() const {
-    return impl->GetCurrentEmuThreadId();
+    return impl->GetCurrentEmuThreadID();
 }
 
 } // namespace Kernel

src/core/hle/kernel/kernel.h

@@ -8,10 +8,10 @@
 #include <string>
 #include <unordered_map>
 #include <vector>
-#include "core/hardware_properties.h"
 #include "core/hle/kernel/object.h"
 
 namespace Core {
+struct EmuThreadHandle;
 class ExclusiveMonitor;
 class System;
 } // namespace Core
@@ -136,10 +136,10 @@ public:
     bool IsValidNamedPort(NamedPortTable::const_iterator port) const;
 
     /// Gets the current host_thread/guest_thread handle.
-    Core::EmuThreadHandle GetCurrentEmuThreadId() const;
+    Core::EmuThreadHandle GetCurrentEmuThreadID() const;
 
     /// Gets the current host_thread handle.
-    u32 GetCurrentHostThreadId() const;
+    u32 GetCurrentHostThreadID() const;
 
     /// Register the current thread as a CPU Core Thread.
     void RegisterCoreThread(std::size_t core_id);

src/core/hle/kernel/scheduler.cpp

@@ -358,18 +358,22 @@ void GlobalScheduler::Shutdown() {
 }
 
 void GlobalScheduler::Lock() {
-    Core::EmuThreadHandle current_thread = kernel.GetCurrentEmuThreadId();
+    Core::EmuThreadHandle current_thread = kernel.GetCurrentEmuThreadID();
     if (current_thread == current_owner) {
         ++scope_lock;
     } else {
         inner_lock.lock();
         current_owner = current_thread;
+        ASSERT(current_owner != Core::EmuThreadHandle::InvalidHandle());
         scope_lock = 1;
     }
 }
 
 void GlobalScheduler::Unlock() {
-    if (--scope_lock == 0) {
+    if (--scope_lock != 0) {
+        ASSERT(scope_lock > 0);
+        return;
+    }
     for (std::size_t i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) {
         SelectThread(i);
     }
@@ -377,7 +381,6 @@ void GlobalScheduler::Unlock() {
     scope_lock = 1;
     inner_lock.unlock();
     // TODO(Blinkhawk): Setup the interrupts and change context on current core.
-    }
 }
 
 Scheduler::Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, std::size_t core_id)

src/core/hle/kernel/scheduler.h

@@ -171,7 +171,7 @@ private:
 
     /// Scheduler lock mechanisms.
     std::mutex inner_lock{}; // TODO(Blinkhawk): Replace for a SpinLock
-    std::atomic<std::size_t> scope_lock{};
+    std::atomic<s64> scope_lock{};
     Core::EmuThreadHandle current_owner{Core::EmuThreadHandle::InvalidHandle()};
 
     /// Lists all thread ids that aren't deleted/etc.
@@ -245,7 +245,7 @@ private:
 
 class SchedulerLock {
 public:
-    SchedulerLock(KernelCore& kernel);
+    explicit SchedulerLock(KernelCore& kernel);
     ~SchedulerLock();
 
 protected:
@@ -254,7 +254,7 @@ protected:
 
 class SchedulerLockAndSleep : public SchedulerLock {
 public:
-    SchedulerLockAndSleep(KernelCore& kernel, Handle& event_handle, Thread* time_task,
+    explicit SchedulerLockAndSleep(KernelCore& kernel, Handle& event_handle, Thread* time_task,
                                    s64 nanoseconds);
     ~SchedulerLockAndSleep();
 

src/core/hle/kernel/time_manager.cpp

@@ -2,6 +2,7 @@
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
+#include "common/assert.h"
 #include "core/core.h"
 #include "core/core_timing.h"
 #include "core/core_timing_util.h"
@@ -34,9 +35,10 @@ void TimeManager::ScheduleTimeEvent(Handle& event_handle, Thread* timetask, s64
 }
 
 void TimeManager::UnscheduleTimeEvent(Handle event_handle) {
-    if (event_handle != InvalidHandle) {
+    if (event_handle == InvalidHandle) {
-        system.CoreTiming().UnscheduleEvent(time_manager_event_type, event_handle);
+        return;
     }
+    system.CoreTiming().UnscheduleEvent(time_manager_event_type, event_handle);
 }
 
 } // namespace Kernel

src/core/hle/kernel/time_manager.h

@@ -20,12 +20,19 @@ namespace Kernel {
 
 class Thread;
 
+/**
+ * The `TimeManager` takes care of scheduling time events on threads and executes their TimeUp
+ * method when the event is triggered.
+ */
 class TimeManager {
 public:
-    TimeManager(Core::System& system);
+    explicit TimeManager(Core::System& system);
 
+    /// Schedule a time event on `timetask` thread that will expire in 'nanoseconds'
+    /// returns a non-invalid handle in `event_handle` if correctly scheduled
     void ScheduleTimeEvent(Handle& event_handle, Thread* timetask, s64 nanoseconds);
 
+    /// Unschedule an existing time event
     void UnscheduleTimeEvent(Handle event_handle);
 
 private: