From 5c0408596f6ccf5d2b171321bac386713b169d5b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 10 Oct 2018 00:42:10 -0400 Subject: [PATCH] kernel/thread: Use a regular pointer for the owner/current process There's no real need to use a shared pointer in these cases, and only makes object management more fragile in terms of how easy it would be to introduce cycles. Instead, just do the simple thing of using a regular pointer. Much of this is just a hold-over from citra anyways. It also doesn't make sense from a behavioral point of view for a process' thread to prolong the lifetime of the process itself (the process is supposed to own the thread, not the other way around). --- src/core/arm/dynarmic/arm_dynarmic.cpp | 2 +- src/core/core.cpp | 7 ++++--- src/core/core.h | 10 +++++----- src/core/hle/kernel/kernel.cpp | 12 ++++++------ src/core/hle/kernel/kernel.h | 10 +++++----- src/core/hle/kernel/scheduler.cpp | 8 ++++---- src/core/hle/kernel/svc.cpp | 12 ++++++------ src/core/hle/kernel/thread.cpp | 8 ++++---- src/core/hle/kernel/thread.h | 8 ++++---- src/tests/core/arm/arm_test_common.cpp | 3 ++- 10 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index 7e978cf7a..0762321a9 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -129,7 +129,7 @@ public: }; std::unique_ptr ARM_Dynarmic::MakeJit() const { - auto& current_process = Core::CurrentProcess(); + auto* current_process = Core::CurrentProcess(); auto** const page_table = current_process->VMManager().page_table.pointers.data(); Dynarmic::A64::UserConfig config; diff --git a/src/core/core.cpp b/src/core/core.cpp index b6acfb3e4..e2fb9e038 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -136,7 +136,8 @@ struct System::Impl { if (virtual_filesystem == nullptr) virtual_filesystem = std::make_shared(); - kernel.MakeCurrentProcess(Kernel::Process::Create(kernel, "main")); + auto main_process = Kernel::Process::Create(kernel, "main"); + kernel.MakeCurrentProcess(main_process.get()); cpu_barrier = std::make_shared(); cpu_exclusive_monitor = Cpu::MakeExclusiveMonitor(cpu_cores.size()); @@ -361,11 +362,11 @@ const std::shared_ptr& System::Scheduler(std::size_t core_ind return impl->cpu_cores[core_index]->Scheduler(); } -Kernel::SharedPtr& System::CurrentProcess() { +Kernel::Process* System::CurrentProcess() { return impl->kernel.CurrentProcess(); } -const Kernel::SharedPtr& System::CurrentProcess() const { +const Kernel::Process* System::CurrentProcess() const { return impl->kernel.CurrentProcess(); } diff --git a/src/core/core.h b/src/core/core.h index f9a3e97e3..ea4d53914 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -174,11 +174,11 @@ public: /// Gets the scheduler for the CPU core with the specified index const std::shared_ptr& Scheduler(std::size_t core_index); - /// Provides a reference to the current process - Kernel::SharedPtr& CurrentProcess(); + /// Provides a pointer to the current process + Kernel::Process* CurrentProcess(); - /// Provides a constant reference to the current process. - const Kernel::SharedPtr& CurrentProcess() const; + /// Provides a constant pointer to the current process. + const Kernel::Process* CurrentProcess() const; /// Provides a reference to the kernel instance. Kernel::KernelCore& Kernel(); @@ -246,7 +246,7 @@ inline TelemetrySession& Telemetry() { return System::GetInstance().TelemetrySession(); } -inline Kernel::SharedPtr& CurrentProcess() { +inline Kernel::Process* CurrentProcess() { return System::GetInstance().CurrentProcess(); } diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 98eb74298..bd680adfe 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -116,7 +116,7 @@ struct KernelCore::Impl { next_thread_id = 1; process_list.clear(); - current_process.reset(); + current_process = nullptr; handle_table.Clear(); resource_limits.fill(nullptr); @@ -207,7 +207,7 @@ struct KernelCore::Impl { // Lists all processes that exist in the current session. std::vector> process_list; - SharedPtr current_process; + Process* current_process = nullptr; Kernel::HandleTable handle_table; std::array, 4> resource_limits; @@ -266,15 +266,15 @@ void KernelCore::AppendNewProcess(SharedPtr process) { impl->process_list.push_back(std::move(process)); } -void KernelCore::MakeCurrentProcess(SharedPtr process) { - impl->current_process = std::move(process); +void KernelCore::MakeCurrentProcess(Process* process) { + impl->current_process = process; } -SharedPtr& KernelCore::CurrentProcess() { +Process* KernelCore::CurrentProcess() { return impl->current_process; } -const SharedPtr& KernelCore::CurrentProcess() const { +const Process* KernelCore::CurrentProcess() const { return impl->current_process; } diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index c0771ecf0..41554821f 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -66,13 +66,13 @@ public: void AppendNewProcess(SharedPtr process); /// Makes the given process the new current process. - void MakeCurrentProcess(SharedPtr process); + void MakeCurrentProcess(Process* process); - /// Retrieves a reference to the current process. - SharedPtr& CurrentProcess(); + /// Retrieves a pointer to the current process. + Process* CurrentProcess(); - /// Retrieves a const reference to the current process. - const SharedPtr& CurrentProcess() const; + /// Retrieves a const pointer to the current process. + const Process* CurrentProcess() const; /// Adds a port to the named port table void AddNamedPort(std::string name, SharedPtr port); diff --git a/src/core/hle/kernel/scheduler.cpp b/src/core/hle/kernel/scheduler.cpp index cfd6e1bad..1342c597e 100644 --- a/src/core/hle/kernel/scheduler.cpp +++ b/src/core/hle/kernel/scheduler.cpp @@ -9,7 +9,7 @@ #include "common/logging/log.h" #include "core/arm/arm_interface.h" #include "core/core.h" -#include "core/core_timing.h" +#include "core/hle/kernel/kernel.h" #include "core/hle/kernel/process.h" #include "core/hle/kernel/scheduler.h" @@ -78,16 +78,16 @@ void Scheduler::SwitchContext(Thread* new_thread) { // Cancel any outstanding wakeup events for this thread new_thread->CancelWakeupTimer(); - auto previous_process = Core::CurrentProcess(); + auto* const previous_process = Core::CurrentProcess(); current_thread = new_thread; ready_queue.remove(new_thread->GetPriority(), new_thread); new_thread->SetStatus(ThreadStatus::Running); - const auto thread_owner_process = current_thread->GetOwnerProcess(); + auto* const thread_owner_process = current_thread->GetOwnerProcess(); if (previous_process != thread_owner_process) { - Core::CurrentProcess() = thread_owner_process; + Core::System::GetInstance().Kernel().MakeCurrentProcess(thread_owner_process); SetCurrentPageTable(&Core::CurrentProcess()->VMManager().page_table); } diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index b488b508d..3afcce3fe 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -341,7 +341,7 @@ static ResultCode GetInfo(u64* result, u64 info_id, u64 handle, u64 info_sub_id) LOG_TRACE(Kernel_SVC, "called info_id=0x{:X}, info_sub_id=0x{:X}, handle=0x{:08X}", info_id, info_sub_id, handle); - const auto& current_process = Core::CurrentProcess(); + const auto* current_process = Core::CurrentProcess(); const auto& vm_manager = current_process->VMManager(); switch (static_cast(info_id)) { @@ -439,7 +439,7 @@ static ResultCode GetThreadContext(VAddr thread_context, Handle handle) { return ERR_INVALID_HANDLE; } - const auto current_process = Core::CurrentProcess(); + const auto* current_process = Core::CurrentProcess(); if (thread->GetOwnerProcess() != current_process) { return ERR_INVALID_HANDLE; } @@ -531,7 +531,7 @@ static ResultCode MapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 s return ERR_INVALID_HANDLE; } - return shared_memory->Map(Core::CurrentProcess().get(), addr, permissions_type, + return shared_memory->Map(Core::CurrentProcess(), addr, permissions_type, MemoryPermission::DontCare); } @@ -550,7 +550,7 @@ static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 auto& kernel = Core::System::GetInstance().Kernel(); auto shared_memory = kernel.HandleTable().Get(shared_memory_handle); - return shared_memory->Unmap(Core::CurrentProcess().get(), addr); + return shared_memory->Unmap(Core::CurrentProcess(), addr); } /// Query process memory @@ -588,7 +588,7 @@ static ResultCode QueryMemory(MemoryInfo* memory_info, PageInfo* page_info, VAdd /// Exits the current process static void ExitProcess() { - auto& current_process = Core::CurrentProcess(); + auto* current_process = Core::CurrentProcess(); LOG_INFO(Kernel_SVC, "Process {} exiting", current_process->GetProcessID()); ASSERT_MSG(current_process->GetStatus() == ProcessStatus::Running, @@ -636,7 +636,7 @@ static ResultCode CreateThread(Handle* out_handle, VAddr entry_point, u64 arg, V auto& kernel = Core::System::GetInstance().Kernel(); CASCADE_RESULT(SharedPtr thread, Thread::Create(kernel, name, entry_point, priority, arg, processor_id, stack_top, - Core::CurrentProcess())); + *Core::CurrentProcess())); const auto new_guest_handle = kernel.HandleTable().Create(thread); if (new_guest_handle.Failed()) { return new_guest_handle.Code(); diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 8e514cf9a..33aed8c23 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -194,7 +194,7 @@ static void ResetThreadContext(Core::ARM_Interface::ThreadContext& context, VAdd ResultVal> Thread::Create(KernelCore& kernel, std::string name, VAddr entry_point, u32 priority, u64 arg, s32 processor_id, - VAddr stack_top, SharedPtr owner_process) { + VAddr stack_top, Process& owner_process) { // Check if priority is in ranged. Lowest priority -> highest priority id. if (priority > THREADPRIO_LOWEST) { LOG_ERROR(Kernel_SVC, "Invalid thread priority: {}", priority); @@ -208,7 +208,7 @@ ResultVal> Thread::Create(KernelCore& kernel, std::string name // TODO(yuriks): Other checks, returning 0xD9001BEA - if (!Memory::IsValidVirtualAddress(*owner_process, entry_point)) { + if (!Memory::IsValidVirtualAddress(owner_process, entry_point)) { LOG_ERROR(Kernel_SVC, "(name={}): invalid entry {:016X}", name, entry_point); // TODO (bunnei): Find the correct error code to use here return ResultCode(-1); @@ -232,7 +232,7 @@ ResultVal> Thread::Create(KernelCore& kernel, std::string name thread->wait_handle = 0; thread->name = std::move(name); thread->callback_handle = kernel.ThreadWakeupCallbackHandleTable().Create(thread).Unwrap(); - thread->owner_process = owner_process; + thread->owner_process = &owner_process; thread->scheduler = Core::System::GetInstance().Scheduler(processor_id).get(); thread->scheduler->AddThread(thread, priority); thread->tls_address = thread->owner_process->MarkNextAvailableTLSSlotAsUsed(*thread); @@ -264,7 +264,7 @@ SharedPtr SetupMainThread(KernelCore& kernel, VAddr entry_point, u32 pri // Initialize new "main" thread const VAddr stack_top = owner_process.VMManager().GetTLSIORegionEndAddress(); auto thread_res = Thread::Create(kernel, "main", entry_point, priority, 0, THREADPROCESSORID_0, - stack_top, &owner_process); + stack_top, owner_process); SharedPtr thread = std::move(thread_res).Unwrap(); diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index c6ffbd28c..f4d7bd235 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -89,7 +89,7 @@ public: static ResultVal> Create(KernelCore& kernel, std::string name, VAddr entry_point, u32 priority, u64 arg, s32 processor_id, VAddr stack_top, - SharedPtr owner_process); + Process& owner_process); std::string GetName() const override { return name; @@ -262,11 +262,11 @@ public: return processor_id; } - SharedPtr& GetOwnerProcess() { + Process* GetOwnerProcess() { return owner_process; } - const SharedPtr& GetOwnerProcess() const { + const Process* GetOwnerProcess() const { return owner_process; } @@ -386,7 +386,7 @@ private: u64 tpidr_el0 = 0; ///< TPIDR_EL0 read/write system register. /// Process that owns this thread - SharedPtr owner_process; + Process* owner_process; /// Objects that the thread is waiting on, in the same order as they were /// passed to WaitSynchronization1/N. diff --git a/src/tests/core/arm/arm_test_common.cpp b/src/tests/core/arm/arm_test_common.cpp index c0a57e71f..37e15bad0 100644 --- a/src/tests/core/arm/arm_test_common.cpp +++ b/src/tests/core/arm/arm_test_common.cpp @@ -15,7 +15,8 @@ namespace ArmTests { TestEnvironment::TestEnvironment(bool mutable_memory_) : mutable_memory(mutable_memory_), test_memory(std::make_shared(this)) { - Core::CurrentProcess() = Kernel::Process::Create(kernel, ""); + auto process = Kernel::Process::Create(kernel, ""); + kernel.MakeCurrentProcess(process.get()); page_table = &Core::CurrentProcess()->VMManager().page_table; std::fill(page_table->pointers.begin(), page_table->pointers.end(), nullptr);