From a493ab2678078ce2066e8120ec93c0f6b4274df6 Mon Sep 17 00:00:00 2001
From: bunnei <bunneidev@gmail.com>
Date: Mon, 7 Jun 2021 21:10:51 -0700
Subject: [PATCH 1/3] hle: kernel: Remove service thread manager and use
 weak_ptr.

- We no longer need to queue up service threads to be destroyed.
- Fixes a race condition where a thread could be destroyed too early, which caused a crash in Pokemon Sword/Shield.
---
 src/core/hle/kernel/hle_ipc.h            |  6 +++---
 src/core/hle/kernel/k_server_session.cpp |  2 +-
 src/core/hle/kernel/kernel.cpp           | 18 ++++--------------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h
index 2aaf93fca..159565203 100644
--- a/src/core/hle/kernel/hle_ipc.h
+++ b/src/core/hle/kernel/hle_ipc.h
@@ -85,8 +85,8 @@ public:
      */
     void ClientDisconnected(KServerSession* session);
 
-    std::shared_ptr<ServiceThread> GetServiceThread() const {
-        return service_thread.lock();
+    std::weak_ptr<ServiceThread> GetServiceThread() const {
+        return service_thread;
     }
 
 protected:
@@ -152,7 +152,7 @@ public:
         session_handler = std::move(handler);
     }
 
-    std::shared_ptr<ServiceThread> GetServiceThread() const {
+    std::weak_ptr<ServiceThread> GetServiceThread() const {
         return session_handler->GetServiceThread();
     }
 
diff --git a/src/core/hle/kernel/k_server_session.cpp b/src/core/hle/kernel/k_server_session.cpp
index 528ca8614..61213c20e 100644
--- a/src/core/hle/kernel/k_server_session.cpp
+++ b/src/core/hle/kernel/k_server_session.cpp
@@ -119,7 +119,7 @@ ResultCode KServerSession::QueueSyncRequest(KThread* thread, Core::Memory::Memor
 
     context->PopulateFromIncomingCommandBuffer(kernel.CurrentProcess()->GetHandleTable(), cmd_buf);
 
-    if (auto strong_ptr = manager->GetServiceThread(); strong_ptr) {
+    if (auto strong_ptr = manager->GetServiceThread().lock()) {
         strong_ptr->QueueSyncRequest(*parent, std::move(context));
         return ResultSuccess;
     } else {
diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp
index 0ffb78d51..2ceeaeb5f 100644
--- a/src/core/hle/kernel/kernel.cpp
+++ b/src/core/hle/kernel/kernel.cpp
@@ -63,8 +63,6 @@ struct KernelCore::Impl {
         global_scheduler_context = std::make_unique<Kernel::GlobalSchedulerContext>(kernel);
         global_handle_table = std::make_unique<Kernel::KHandleTable>(kernel);
 
-        service_thread_manager =
-            std::make_unique<Common::ThreadWorker>(1, "yuzu:ServiceThreadManager");
         is_phantom_mode_for_singlecore = false;
 
         InitializePhysicalCores();
@@ -96,7 +94,6 @@ struct KernelCore::Impl {
         process_list.clear();
 
         // Ensures all service threads gracefully shutdown
-        service_thread_manager.reset();
         service_threads.clear();
 
         next_object_id = 0;
@@ -680,10 +677,6 @@ struct KernelCore::Impl {
     // Threads used for services
     std::unordered_set<std::shared_ptr<Kernel::ServiceThread>> service_threads;
 
-    // Service threads are managed by a worker thread, so that a calling service thread can queue up
-    // the release of itself
-    std::unique_ptr<Common::ThreadWorker> service_thread_manager;
-
     std::array<KThread*, Core::Hardware::NUM_CPU_CORES> suspend_threads;
     std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES> interrupts{};
     std::array<std::unique_ptr<Kernel::KScheduler>, Core::Hardware::NUM_CPU_CORES> schedulers{};
@@ -986,17 +979,14 @@ void KernelCore::ExitSVCProfile() {
 
 std::weak_ptr<Kernel::ServiceThread> KernelCore::CreateServiceThread(const std::string& name) {
     auto service_thread = std::make_shared<Kernel::ServiceThread>(*this, 1, name);
-    impl->service_thread_manager->QueueWork(
-        [this, service_thread] { impl->service_threads.emplace(service_thread); });
+    impl->service_threads.emplace(service_thread);
     return service_thread;
 }
 
 void KernelCore::ReleaseServiceThread(std::weak_ptr<Kernel::ServiceThread> service_thread) {
-    impl->service_thread_manager->QueueWork([this, service_thread] {
-        if (auto strong_ptr = service_thread.lock()) {
-            impl->service_threads.erase(strong_ptr);
-        }
-    });
+    if (auto strong_ptr = service_thread.lock()) {
+        impl->service_threads.erase(strong_ptr);
+    }
 }
 
 Init::KSlabResourceCounts& KernelCore::SlabResourceCounts() {

From 08d798b6fe8b09f28c0302b52c3b832b786d1b8a Mon Sep 17 00:00:00 2001
From: bunnei <bunneidev@gmail.com>
Date: Mon, 7 Jun 2021 21:55:37 -0700
Subject: [PATCH 2/3] hle: kernel: hle_ipc: Ensure SessionRequestHandler is
 valid.

---
 src/core/hle/kernel/hle_ipc.cpp          | 15 +++++++++++++++
 src/core/hle/kernel/hle_ipc.h            |  3 ++-
 src/core/hle/kernel/k_server_session.cpp | 13 +++++++++----
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp
index 260af87e5..45aced99f 100644
--- a/src/core/hle/kernel/hle_ipc.cpp
+++ b/src/core/hle/kernel/hle_ipc.cpp
@@ -41,6 +41,21 @@ SessionRequestManager::SessionRequestManager(KernelCore& kernel_) : kernel{kerne
 
 SessionRequestManager::~SessionRequestManager() = default;
 
+bool SessionRequestManager::HasSessionRequestHandler(const HLERequestContext& context) const {
+    if (IsDomain() && context.HasDomainMessageHeader()) {
+        const auto& message_header = context.GetDomainMessageHeader();
+        const auto object_id = message_header.object_id;
+
+        if (object_id > DomainHandlerCount()) {
+            LOG_CRITICAL(IPC, "object_id {} is too big!", object_id);
+            return false;
+        }
+        return DomainHandler(object_id - 1) != nullptr;
+    } else {
+        return session_handler != nullptr;
+    }
+}
+
 void SessionRequestHandler::ClientConnected(KServerSession* session) {
     session->SetSessionHandler(shared_from_this());
 }
diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h
index 159565203..a61870f8b 100644
--- a/src/core/hle/kernel/hle_ipc.h
+++ b/src/core/hle/kernel/hle_ipc.h
@@ -156,6 +156,8 @@ public:
         return session_handler->GetServiceThread();
     }
 
+    bool HasSessionRequestHandler(const HLERequestContext& context) const;
+
 private:
     bool is_domain{};
     SessionRequestHandlerPtr session_handler;
@@ -163,7 +165,6 @@ private:
 
 private:
     KernelCore& kernel;
-    std::weak_ptr<ServiceThread> service_thread;
 };
 
 /**
diff --git a/src/core/hle/kernel/k_server_session.cpp b/src/core/hle/kernel/k_server_session.cpp
index 61213c20e..dd62706a8 100644
--- a/src/core/hle/kernel/k_server_session.cpp
+++ b/src/core/hle/kernel/k_server_session.cpp
@@ -119,11 +119,16 @@ ResultCode KServerSession::QueueSyncRequest(KThread* thread, Core::Memory::Memor
 
     context->PopulateFromIncomingCommandBuffer(kernel.CurrentProcess()->GetHandleTable(), cmd_buf);
 
-    if (auto strong_ptr = manager->GetServiceThread().lock()) {
-        strong_ptr->QueueSyncRequest(*parent, std::move(context));
-        return ResultSuccess;
+    // Ensure we have a session request handler
+    if (manager->HasSessionRequestHandler(*context)) {
+        if (auto strong_ptr = manager->GetServiceThread().lock()) {
+            strong_ptr->QueueSyncRequest(*parent, std::move(context));
+            return ResultSuccess;
+        } else {
+            ASSERT_MSG(false, "strong_ptr is nullptr!");
+        }
     } else {
-        ASSERT_MSG(false, "strong_ptr was nullptr!");
+        ASSERT_MSG(false, "handler is invalid!");
     }
 
     return ResultSuccess;

From b8fb9b3f112cb43831aeac8ab1242ae653989067 Mon Sep 17 00:00:00 2001
From: bunnei <bunneidev@gmail.com>
Date: Tue, 8 Jun 2021 13:39:20 -0700
Subject: [PATCH 3/3] hle: kernel: KServerSession: Work-around scenario where
 session is closed too early.

---
 src/core/hle/kernel/k_server_session.cpp | 31 ++++++++++++++++++------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/core/hle/kernel/k_server_session.cpp b/src/core/hle/kernel/k_server_session.cpp
index dd62706a8..5c3c13ce6 100644
--- a/src/core/hle/kernel/k_server_session.cpp
+++ b/src/core/hle/kernel/k_server_session.cpp
@@ -8,6 +8,7 @@
 #include "common/assert.h"
 #include "common/common_types.h"
 #include "common/logging/log.h"
+#include "common/scope_exit.h"
 #include "core/core_timing.h"
 #include "core/hle/ipc_helpers.h"
 #include "core/hle/kernel/hle_ipc.h"
@@ -119,11 +120,20 @@ ResultCode KServerSession::QueueSyncRequest(KThread* thread, Core::Memory::Memor
 
     context->PopulateFromIncomingCommandBuffer(kernel.CurrentProcess()->GetHandleTable(), cmd_buf);
 
+    // In the event that something fails here, stub a result to prevent the game from crashing.
+    // This is a work-around in the event that somehow we process a service request after the
+    // session has been closed by the game. This has been observed to happen rarely in Pokemon
+    // Sword/Shield and is likely a result of us using host threads/scheduling for services.
+    // TODO(bunnei): Find a better solution here.
+    auto error_guard = SCOPE_GUARD({ CompleteSyncRequest(*context); });
+
     // Ensure we have a session request handler
     if (manager->HasSessionRequestHandler(*context)) {
         if (auto strong_ptr = manager->GetServiceThread().lock()) {
             strong_ptr->QueueSyncRequest(*parent, std::move(context));
-            return ResultSuccess;
+
+            // We succeeded.
+            error_guard.Cancel();
         } else {
             ASSERT_MSG(false, "strong_ptr is nullptr!");
         }
@@ -136,13 +146,20 @@ ResultCode KServerSession::QueueSyncRequest(KThread* thread, Core::Memory::Memor
 
 ResultCode KServerSession::CompleteSyncRequest(HLERequestContext& context) {
     ResultCode result = ResultSuccess;
+
     // If the session has been converted to a domain, handle the domain request
-    if (IsDomain() && context.HasDomainMessageHeader()) {
-        result = HandleDomainSyncRequest(context);
-        // If there is no domain header, the regular session handler is used
-    } else if (manager->HasSessionHandler()) {
-        // If this ServerSession has an associated HLE handler, forward the request to it.
-        result = manager->SessionHandler().HandleSyncRequest(*this, context);
+    if (manager->HasSessionRequestHandler(context)) {
+        if (IsDomain() && context.HasDomainMessageHeader()) {
+            result = HandleDomainSyncRequest(context);
+            // If there is no domain header, the regular session handler is used
+        } else if (manager->HasSessionHandler()) {
+            // If this ServerSession has an associated HLE handler, forward the request to it.
+            result = manager->SessionHandler().HandleSyncRequest(*this, context);
+        }
+    } else {
+        ASSERT_MSG(false, "Session handler is invalid, stubbing response!");
+        IPC::ResponseBuilder rb(context, 2);
+        rb.Push(ResultSuccess);
     }
 
     if (convert_to_domain) {