From 85527cc7c7dfce81278d0373ffce97c70e35d5d7 Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 2 Nov 2022 20:08:19 -0400 Subject: [PATCH 1/2] kernel: avoid racy behavior in global suspension --- src/core/hle/kernel/kernel.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 09c36ee09..6df77b423 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -1109,16 +1109,28 @@ void KernelCore::Suspend(bool suspended) { const bool should_suspend{exception_exited || suspended}; const auto activity = should_suspend ? ProcessActivity::Paused : ProcessActivity::Runnable; - for (auto* process : GetProcessList()) { - process->SetActivity(activity); + std::vector> process_threads; + { + KScopedSchedulerLock sl{*this}; + + if (auto* process = CurrentProcess(); process != nullptr) { + process->SetActivity(activity); + + if (!should_suspend) { + // Runnable now; no need to wait. + return; + } - if (should_suspend) { - // Wait for execution to stop for (auto* thread : process->GetThreadList()) { - thread->WaitUntilSuspended(); + process_threads.emplace_back(thread); } } } + + // Wait for execution to stop. + for (auto& thread : process_threads) { + thread->WaitUntilSuspended(); + } } void KernelCore::ShutdownCores() { From e6fe40428c3260d551ac5c795651e2efcfdfc0ea Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 2 Nov 2022 20:21:32 -0400 Subject: [PATCH 2/2] service_thread: register service threads to the logical owner process --- src/core/hle/kernel/k_thread.cpp | 4 ++-- src/core/hle/kernel/k_thread.h | 2 +- src/core/hle/kernel/kernel.cpp | 27 ++++++++++++++++---------- src/core/hle/kernel/kernel.h | 2 +- src/core/hle/kernel/service_thread.cpp | 24 +++++++++++++++++------ 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/core/hle/kernel/k_thread.cpp b/src/core/hle/kernel/k_thread.cpp index cc88d08f0..78076a346 100644 --- a/src/core/hle/kernel/k_thread.cpp +++ b/src/core/hle/kernel/k_thread.cpp @@ -263,9 +263,9 @@ Result KThread::InitializeThread(KThread* thread, KThreadFunction func, uintptr_ R_SUCCEED(); } -Result KThread::InitializeDummyThread(KThread* thread) { +Result KThread::InitializeDummyThread(KThread* thread, KProcess* owner) { // Initialize the thread. - R_TRY(thread->Initialize({}, {}, {}, DummyThreadPriority, 3, {}, ThreadType::Dummy)); + R_TRY(thread->Initialize({}, {}, {}, DummyThreadPriority, 3, owner, ThreadType::Dummy)); // Initialize emulation parameters. thread->stack_parameters.disable_count = 0; diff --git a/src/core/hle/kernel/k_thread.h b/src/core/hle/kernel/k_thread.h index 30aa10c9a..f38c92bff 100644 --- a/src/core/hle/kernel/k_thread.h +++ b/src/core/hle/kernel/k_thread.h @@ -415,7 +415,7 @@ public: static void PostDestroy(uintptr_t arg); - [[nodiscard]] static Result InitializeDummyThread(KThread* thread); + [[nodiscard]] static Result InitializeDummyThread(KThread* thread, KProcess* owner); [[nodiscard]] static Result InitializeMainThread(Core::System& system, KThread* thread, s32 virt_core); diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 6df77b423..d1892e078 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -84,7 +84,7 @@ struct KernelCore::Impl { InitializeResourceManagers(pt_heap_region.GetAddress(), pt_heap_region.GetSize()); } - RegisterHostThread(); + RegisterHostThread(nullptr); default_service_thread = CreateServiceThread(kernel, "DefaultServiceThread"); } @@ -300,15 +300,18 @@ struct KernelCore::Impl { } // Gets the dummy KThread for the caller, allocating a new one if this is the first time - KThread* GetHostDummyThread() { + KThread* GetHostDummyThread(KThread* existing_thread) { auto initialize = [this](KThread* thread) { - ASSERT(KThread::InitializeDummyThread(thread).IsSuccess()); + ASSERT(KThread::InitializeDummyThread(thread, nullptr).IsSuccess()); thread->SetName(fmt::format("DummyThread:{}", GetHostThreadId())); return thread; }; - thread_local auto raw_thread = KThread(system.Kernel()); - thread_local auto thread = initialize(&raw_thread); + thread_local KThread raw_thread{system.Kernel()}; + thread_local KThread* thread = nullptr; + if (thread == nullptr) { + thread = (existing_thread == nullptr) ? initialize(&raw_thread) : existing_thread; + } return thread; } @@ -323,9 +326,9 @@ struct KernelCore::Impl { } /// Registers a new host thread by allocating a host thread ID for it - void RegisterHostThread() { + void RegisterHostThread(KThread* existing_thread) { [[maybe_unused]] const auto this_id = GetHostThreadId(); - [[maybe_unused]] const auto dummy_thread = GetHostDummyThread(); + [[maybe_unused]] const auto dummy_thread = GetHostDummyThread(existing_thread); } [[nodiscard]] u32 GetCurrentHostThreadID() { @@ -356,7 +359,7 @@ struct KernelCore::Impl { KThread* GetCurrentEmuThread() { const auto thread_id = GetCurrentHostThreadID(); if (thread_id >= Core::Hardware::NUM_CPU_CORES) { - return GetHostDummyThread(); + return GetHostDummyThread(nullptr); } return current_thread; @@ -1033,8 +1036,12 @@ void KernelCore::RegisterCoreThread(std::size_t core_id) { impl->RegisterCoreThread(core_id); } -void KernelCore::RegisterHostThread() { - impl->RegisterHostThread(); +void KernelCore::RegisterHostThread(KThread* existing_thread) { + impl->RegisterHostThread(existing_thread); + + if (existing_thread != nullptr) { + ASSERT(GetCurrentEmuThread() == existing_thread); + } } u32 KernelCore::GetCurrentHostThreadID() const { diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index 4ae6b3923..8a21568f7 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -236,7 +236,7 @@ public: void RegisterCoreThread(std::size_t core_id); /// Register the current thread as a non CPU core thread. - void RegisterHostThread(); + void RegisterHostThread(KThread* existing_thread = nullptr); /// Gets the virtual memory manager for the kernel. KMemoryManager& MemoryManager(); diff --git a/src/core/hle/kernel/service_thread.cpp b/src/core/hle/kernel/service_thread.cpp index c8fe42537..7a85be77f 100644 --- a/src/core/hle/kernel/service_thread.cpp +++ b/src/core/hle/kernel/service_thread.cpp @@ -36,11 +36,12 @@ public: private: KernelCore& kernel; - std::jthread m_thread; + std::jthread m_host_thread; std::mutex m_session_mutex; std::map> m_sessions; KEvent* m_wakeup_event; KProcess* m_process; + KThread* m_thread; std::atomic m_shutdown_requested; const std::string m_service_name; }; @@ -132,7 +133,7 @@ void ServiceThread::Impl::SessionClosed(KServerSession* server_session, void ServiceThread::Impl::LoopProcess() { Common::SetCurrentThreadName(m_service_name.c_str()); - kernel.RegisterHostThread(); + kernel.RegisterHostThread(m_thread); while (!m_shutdown_requested.load()) { WaitAndProcessImpl(); @@ -160,7 +161,7 @@ ServiceThread::Impl::~Impl() { // Shut down the processing thread. m_shutdown_requested.store(true); m_wakeup_event->Signal(); - m_thread.join(); + m_host_thread.join(); // Lock mutex. m_session_mutex.lock(); @@ -177,6 +178,9 @@ ServiceThread::Impl::~Impl() { m_wakeup_event->GetReadableEvent().Close(); m_wakeup_event->Close(); + // Close thread. + m_thread->Close(); + // Close process. m_process->Close(); } @@ -199,11 +203,19 @@ ServiceThread::Impl::Impl(KernelCore& kernel_, const std::string& service_name) // Commit the event reservation. event_reservation.Commit(); - // Register the event. - KEvent::Register(kernel, m_wakeup_event); + // Reserve a new thread from the process resource limit + KScopedResourceReservation thread_reservation(m_process, LimitableResource::Threads); + ASSERT(thread_reservation.Succeeded()); + + // Initialize thread. + m_thread = KThread::Create(kernel); + ASSERT(KThread::InitializeDummyThread(m_thread, m_process).IsSuccess()); + + // Commit the thread reservation. + thread_reservation.Commit(); // Start thread. - m_thread = std::jthread([this] { LoopProcess(); }); + m_host_thread = std::jthread([this] { LoopProcess(); }); } ServiceThread::ServiceThread(KernelCore& kernel, const std::string& name)