Merge pull request #7712 from bunnei/fix-thread-exit

Accurately implement thread exit

src/common/fiber.cpp

@@ -124,7 +124,10 @@ void Fiber::YieldTo(std::weak_ptr<Fiber> weak_from, Fiber& to) {
 
     // "from" might no longer be valid if the thread was killed
     if (auto from = weak_from.lock()) {
-        ASSERT(from->impl->previous_fiber != nullptr);
+        if (from->impl->previous_fiber == nullptr) {
+            ASSERT_MSG(false, "previous_fiber is nullptr!");
+            return;
+        }
         from->impl->previous_fiber->impl->context = transfer.fctx;
         from->impl->previous_fiber->impl->guard.unlock();
         from->impl->previous_fiber.reset();

src/core/CMakeLists.txt

@@ -247,6 +247,9 @@ add_library(core STATIC
     hle/kernel/k_trace.h
     hle/kernel/k_transfer_memory.cpp
     hle/kernel/k_transfer_memory.h
+    hle/kernel/k_worker_task.h
+    hle/kernel/k_worker_task_manager.cpp
+    hle/kernel/k_worker_task_manager.h
     hle/kernel/k_writable_event.cpp
     hle/kernel/k_writable_event.h
     hle/kernel/kernel.cpp

src/core/hle/kernel/k_process.cpp

@@ -149,6 +149,10 @@ ResultCode KProcess::Initialize(KProcess* process, Core::System& system, std::st
     return ResultSuccess;
 }
 
+void KProcess::DoWorkerTaskImpl() {
+    UNIMPLEMENTED();
+}
+
 KResourceLimit* KProcess::GetResourceLimit() const {
     return resource_limit;
 }
@@ -477,7 +481,7 @@ void KProcess::Finalize() {
     }
 
     // Perform inherited finalization.
-    KAutoObjectWithSlabHeapAndContainer<KProcess, KSynchronizationObject>::Finalize();
+    KAutoObjectWithSlabHeapAndContainer<KProcess, KWorkerTask>::Finalize();
 }
 
 /**

src/core/hle/kernel/k_process.h

@@ -15,6 +15,7 @@
 #include "core/hle/kernel/k_condition_variable.h"
 #include "core/hle/kernel/k_handle_table.h"
 #include "core/hle/kernel/k_synchronization_object.h"
+#include "core/hle/kernel/k_worker_task.h"
 #include "core/hle/kernel/process_capability.h"
 #include "core/hle/kernel/slab_helpers.h"
 #include "core/hle/result.h"
@@ -62,8 +63,7 @@ enum class ProcessStatus {
     DebugBreak,
 };
 
-class KProcess final
+class KProcess final : public KAutoObjectWithSlabHeapAndContainer<KProcess, KWorkerTask> {
-    : public KAutoObjectWithSlabHeapAndContainer<KProcess, KSynchronizationObject> {
     KERNEL_AUTOOBJECT_TRAITS(KProcess, KSynchronizationObject);
 
 public:
@@ -345,6 +345,8 @@ public:
 
     bool IsSignaled() const override;
 
+    void DoWorkerTaskImpl();
+
     void PinCurrentThread(s32 core_id);
     void UnpinCurrentThread(s32 core_id);
     void UnpinThread(KThread* thread);

src/core/hle/kernel/k_thread.cpp

@@ -30,6 +30,7 @@
 #include "core/hle/kernel/k_system_control.h"
 #include "core/hle/kernel/k_thread.h"
 #include "core/hle/kernel/k_thread_queue.h"
+#include "core/hle/kernel/k_worker_task_manager.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/svc_results.h"
 #include "core/hle/kernel/time_manager.h"
@@ -332,7 +333,7 @@ void KThread::Finalize() {
     }
 
     // Perform inherited finalization.
-    KAutoObjectWithSlabHeapAndContainer<KThread, KSynchronizationObject>::Finalize();
+    KSynchronizationObject::Finalize();
 }
 
 bool KThread::IsSignaled() const {
@@ -376,11 +377,28 @@ void KThread::StartTermination() {
 
     // Register terminated dpc flag.
     RegisterDpc(DpcFlag::Terminated);
+}
+
+void KThread::FinishTermination() {
+    // Ensure that the thread is not executing on any core.
+    if (parent != nullptr) {
+        for (std::size_t i = 0; i < static_cast<std::size_t>(Core::Hardware::NUM_CPU_CORES); ++i) {
+            KThread* core_thread{};
+            do {
+                core_thread = kernel.Scheduler(i).GetCurrentThread();
+            } while (core_thread == this);
+        }
+    }
 
     // Close the thread.
     this->Close();
 }
 
+void KThread::DoWorkerTaskImpl() {
+    // Finish the termination that was begun by Exit().
+    this->FinishTermination();
+}
+
 void KThread::Pin(s32 current_core) {
     ASSERT(kernel.GlobalSchedulerContext().IsLocked());
 
@@ -417,12 +435,7 @@ void KThread::Pin(s32 current_core) {
                                          static_cast<u32>(ThreadState::SuspendShift)));
 
         // Update our state.
-        const ThreadState old_state = thread_state;
+        UpdateState();
-        thread_state = static_cast<ThreadState>(GetSuspendFlags() |
-                                                static_cast<u32>(old_state & ThreadState::Mask));
-        if (thread_state != old_state) {
-            KScheduler::OnThreadStateChanged(kernel, this, old_state);
-        }
     }
 
     // TODO(bunnei): Update our SVC access permissions.
@@ -463,20 +476,13 @@ void KThread::Unpin() {
     }
 
     // Allow performing thread suspension (if termination hasn't been requested).
-    {
+    if (!IsTerminationRequested()) {
         // Update our allow flags.
-        if (!IsTerminationRequested()) {
+        suspend_allowed_flags |= (1 << (static_cast<u32>(SuspendType::Thread) +
-            suspend_allowed_flags |= (1 << (static_cast<u32>(SuspendType::Thread) +
+                                        static_cast<u32>(ThreadState::SuspendShift)));
-                                            static_cast<u32>(ThreadState::SuspendShift)));
-        }
 
         // Update our state.
-        const ThreadState old_state = thread_state;
+        UpdateState();
-        thread_state = static_cast<ThreadState>(GetSuspendFlags() |
-                                                static_cast<u32>(old_state & ThreadState::Mask));
-        if (thread_state != old_state) {
-            KScheduler::OnThreadStateChanged(kernel, this, old_state);
-        }
     }
 
     // TODO(bunnei): Update our SVC access permissions.
@@ -689,12 +695,7 @@ void KThread::Resume(SuspendType type) {
         ~(1u << (static_cast<u32>(ThreadState::SuspendShift) + static_cast<u32>(type)));
 
     // Update our state.
-    const ThreadState old_state = thread_state;
+    this->UpdateState();
-    thread_state = static_cast<ThreadState>(GetSuspendFlags() |
-                                            static_cast<u32>(old_state & ThreadState::Mask));
-    if (thread_state != old_state) {
-        KScheduler::OnThreadStateChanged(kernel, this, old_state);
-    }
 }
 
 void KThread::WaitCancel() {
@@ -721,19 +722,22 @@ void KThread::TrySuspend() {
     ASSERT(GetNumKernelWaiters() == 0);
 
     // Perform the suspend.
-    Suspend();
+    this->UpdateState();
 }
 
-void KThread::Suspend() {
+void KThread::UpdateState() {
     ASSERT(kernel.GlobalSchedulerContext().IsLocked());
-    ASSERT(IsSuspendRequested());
 
     // Set our suspend flags in state.
     const auto old_state = thread_state;
-    thread_state = static_cast<ThreadState>(GetSuspendFlags()) | (old_state & ThreadState::Mask);
+    const auto new_state =
+        static_cast<ThreadState>(this->GetSuspendFlags()) | (old_state & ThreadState::Mask);
+    thread_state = new_state;
 
     // Note the state change in scheduler.
-    KScheduler::OnThreadStateChanged(kernel, this, old_state);
+    if (new_state != old_state) {
+        KScheduler::OnThreadStateChanged(kernel, this, old_state);
+    }
 }
 
 void KThread::Continue() {
@@ -998,13 +1002,16 @@ ResultCode KThread::Run() {
 
         // If the current thread has been asked to suspend, suspend it and retry.
         if (GetCurrentThread(kernel).IsSuspended()) {
-            GetCurrentThread(kernel).Suspend();
+            GetCurrentThread(kernel).UpdateState();
             continue;
         }
 
         // If we're not a kernel thread and we've been asked to suspend, suspend ourselves.
-        if (IsUserThread() && IsSuspended()) {
+        if (KProcess* owner = this->GetOwnerProcess(); owner != nullptr) {
-            Suspend();
+            if (IsUserThread() && IsSuspended()) {
+                this->UpdateState();
+            }
+            owner->IncrementThreadCount();
         }
 
         // Set our state and finish.
@@ -1029,11 +1036,18 @@ void KThread::Exit() {
     {
         KScopedSchedulerLock sl{kernel};
 
+        // Disallow all suspension.
+        suspend_allowed_flags = 0;
+        this->UpdateState();
+
         // Disallow all suspension.
         suspend_allowed_flags = 0;
 
         // Start termination.
         StartTermination();
+
+        // Register the thread as a work task.
+        KWorkerTaskManager::AddTask(kernel, KWorkerTaskManager::WorkerType::Exit, this);
     }
 }
 

src/core/hle/kernel/k_thread.h

@@ -19,6 +19,7 @@
 #include "core/hle/kernel/k_light_lock.h"
 #include "core/hle/kernel/k_spin_lock.h"
 #include "core/hle/kernel/k_synchronization_object.h"
+#include "core/hle/kernel/k_worker_task.h"
 #include "core/hle/kernel/slab_helpers.h"
 #include "core/hle/kernel/svc_common.h"
 #include "core/hle/kernel/svc_types.h"
@@ -100,7 +101,7 @@ enum class ThreadWaitReasonForDebugging : u32 {
 [[nodiscard]] KThread& GetCurrentThread(KernelCore& kernel);
 [[nodiscard]] s32 GetCurrentCoreId(KernelCore& kernel);
 
-class KThread final : public KAutoObjectWithSlabHeapAndContainer<KThread, KSynchronizationObject>,
+class KThread final : public KAutoObjectWithSlabHeapAndContainer<KThread, KWorkerTask>,
                       public boost::intrusive::list_base_hook<> {
     KERNEL_AUTOOBJECT_TRAITS(KThread, KSynchronizationObject);
 
@@ -192,9 +193,9 @@ public:
 
     void TrySuspend();
 
-    void Continue();
+    void UpdateState();
 
-    void Suspend();
+    void Continue();
 
     constexpr void SetSyncedIndex(s32 index) {
         synced_index = index;
@@ -385,6 +386,8 @@ public:
 
     void OnTimer();
 
+    void DoWorkerTaskImpl();
+
     static void PostDestroy(uintptr_t arg);
 
     [[nodiscard]] static ResultCode InitializeDummyThread(KThread* thread);
@@ -679,6 +682,8 @@ private:
 
     void StartTermination();
 
+    void FinishTermination();
+
     [[nodiscard]] ResultCode Initialize(KThreadFunction func, uintptr_t arg, VAddr user_stack_top,
                                         s32 prio, s32 virt_core, KProcess* owner, ThreadType type);
 

src/core/hle/kernel/k_worker_task.h

@@ -0,0 +1,18 @@
+// Copyright 2022 yuzu Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include "core/hle/kernel/k_synchronization_object.h"
+
+namespace Kernel {
+
+class KWorkerTask : public KSynchronizationObject {
+public:
+    explicit KWorkerTask(KernelCore& kernel_);
+
+    void DoWorkerTask();
+};
+
+} // namespace Kernel

src/core/hle/kernel/k_worker_task_manager.cpp

@@ -0,0 +1,42 @@
+// Copyright 2022 yuzu Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#include "common/assert.h"
+#include "core/hle/kernel/k_process.h"
+#include "core/hle/kernel/k_thread.h"
+#include "core/hle/kernel/k_worker_task.h"
+#include "core/hle/kernel/k_worker_task_manager.h"
+#include "core/hle/kernel/kernel.h"
+
+namespace Kernel {
+
+KWorkerTask::KWorkerTask(KernelCore& kernel_) : KSynchronizationObject{kernel_} {}
+
+void KWorkerTask::DoWorkerTask() {
+    if (auto* const thread = this->DynamicCast<KThread*>(); thread != nullptr) {
+        return thread->DoWorkerTaskImpl();
+    } else {
+        auto* const process = this->DynamicCast<KProcess*>();
+        ASSERT(process != nullptr);
+
+        return process->DoWorkerTaskImpl();
+    }
+}
+
+KWorkerTaskManager::KWorkerTaskManager() : m_waiting_thread(1, "yuzu:KWorkerTaskManager") {}
+
+void KWorkerTaskManager::AddTask(KernelCore& kernel, WorkerType type, KWorkerTask* task) {
+    ASSERT(type <= WorkerType::Count);
+    kernel.WorkerTaskManager().AddTask(kernel, task);
+}
+
+void KWorkerTaskManager::AddTask(KernelCore& kernel, KWorkerTask* task) {
+    KScopedSchedulerLock sl(kernel);
+    m_waiting_thread.QueueWork([task]() {
+        // Do the task.
+        task->DoWorkerTask();
+    });
+}
+
+} // namespace Kernel

src/core/hle/kernel/k_worker_task_manager.h

@@ -0,0 +1,33 @@
+// Copyright 2022 yuzu Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include "common/common_types.h"
+#include "common/thread_worker.h"
+
+namespace Kernel {
+
+class KernelCore;
+class KWorkerTask;
+
+class KWorkerTaskManager final {
+public:
+    enum class WorkerType : u32 {
+        Exit,
+        Count,
+    };
+
+    KWorkerTaskManager();
+
+    static void AddTask(KernelCore& kernel_, WorkerType type, KWorkerTask* task);
+
+private:
+    void AddTask(KernelCore& kernel, KWorkerTask* task);
+
+private:
+    Common::ThreadWorker m_waiting_thread;
+};
+
+} // namespace Kernel

src/core/hle/kernel/kernel.cpp

@@ -37,6 +37,7 @@
 #include "core/hle/kernel/k_shared_memory.h"
 #include "core/hle/kernel/k_slab_heap.h"
 #include "core/hle/kernel/k_thread.h"
+#include "core/hle/kernel/k_worker_task_manager.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/physical_core.h"
 #include "core/hle/kernel/service_thread.h"
@@ -797,6 +798,8 @@ struct KernelCore::Impl {
 
     std::array<u64, Core::Hardware::NUM_CPU_CORES> svc_ticks{};
 
+    KWorkerTaskManager worker_task_manager;
+
     // System context
     Core::System& system;
 };
@@ -1137,6 +1140,14 @@ const Init::KSlabResourceCounts& KernelCore::SlabResourceCounts() const {
     return impl->slab_resource_counts;
 }
 
+KWorkerTaskManager& KernelCore::WorkerTaskManager() {
+    return impl->worker_task_manager;
+}
+
+const KWorkerTaskManager& KernelCore::WorkerTaskManager() const {
+    return impl->worker_task_manager;
+}
+
 bool KernelCore::IsPhantomModeForSingleCore() const {
     return impl->IsPhantomModeForSingleCore();
 }

src/core/hle/kernel/kernel.h

@@ -52,6 +52,7 @@ class KSharedMemory;
 class KSharedMemoryInfo;
 class KThread;
 class KTransferMemory;
+class KWorkerTaskManager;
 class KWritableEvent;
 class KCodeMemory;
 class PhysicalCore;
@@ -343,6 +344,12 @@ public:
     /// Gets the current slab resource counts.
     const Init::KSlabResourceCounts& SlabResourceCounts() const;
 
+    /// Gets the current worker task manager, used for dispatching KThread/KProcess tasks.
+    KWorkerTaskManager& WorkerTaskManager();
+
+    /// Gets the current worker task manager, used for dispatching KThread/KProcess tasks.
+    const KWorkerTaskManager& WorkerTaskManager() const;
+
 private:
     friend class KProcess;
     friend class KThread;