From 6480d24a8c6a40e11d7791d679a0a7c2122ee9b2 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Mon, 29 Dec 2014 10:55:30 -0200 Subject: [PATCH] Kernel: Start using boost::intrusive_ptr for lifetime management --- externals/boost | 2 +- src/core/hle/kernel/address_arbiter.cpp | 2 +- src/core/hle/kernel/event.cpp | 10 ++++---- src/core/hle/kernel/kernel.cpp | 21 +++++++-------- src/core/hle/kernel/kernel.h | 21 +++++++++------ src/core/hle/kernel/mutex.cpp | 6 ++--- src/core/hle/kernel/semaphore.cpp | 4 +-- src/core/hle/kernel/shared_memory.cpp | 4 +-- src/core/hle/kernel/thread.cpp | 34 ++++++++++++------------- src/core/hle/kernel/thread.h | 6 ++--- src/core/hle/service/service.cpp | 3 ++- src/core/hle/svc.cpp | 20 +++++++++------ 12 files changed, 70 insertions(+), 63 deletions(-) diff --git a/externals/boost b/externals/boost index 97052c28a..a1afc91d3 160000 --- a/externals/boost +++ b/externals/boost @@ -1 +1 @@ -Subproject commit 97052c28acb141dbf3c5e14114af99045344b695 +Subproject commit a1afc91d3aaa3da06bdbc13c78613e1466653405 diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index 2f5ccac09..bb0f4b14b 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/src/core/hle/kernel/address_arbiter.cpp @@ -30,7 +30,7 @@ public: /// Arbitrate an address ResultCode ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 value) { - Object* object = Kernel::g_handle_table.GetGeneric(handle); + Object* object = Kernel::g_handle_table.GetGeneric(handle).get(); if (object == nullptr) return InvalidHandle(ErrorModule::Kernel); diff --git a/src/core/hle/kernel/event.cpp b/src/core/hle/kernel/event.cpp index 697e08681..271190dbe 100644 --- a/src/core/hle/kernel/event.cpp +++ b/src/core/hle/kernel/event.cpp @@ -53,7 +53,7 @@ public: * @return Result of operation, 0 on success, otherwise error code */ ResultCode SetPermanentLock(Handle handle, const bool permanent_locked) { - Event* evt = g_handle_table.Get(handle); + Event* evt = g_handle_table.Get(handle).get(); if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel); evt->permanent_locked = permanent_locked; @@ -67,7 +67,7 @@ ResultCode SetPermanentLock(Handle handle, const bool permanent_locked) { * @return Result of operation, 0 on success, otherwise error code */ ResultCode SetEventLocked(const Handle handle, const bool locked) { - Event* evt = g_handle_table.Get(handle); + Event* evt = g_handle_table.Get(handle).get(); if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel); if (!evt->permanent_locked) { @@ -82,13 +82,13 @@ ResultCode SetEventLocked(const Handle handle, const bool locked) { * @return Result of operation, 0 on success, otherwise error code */ ResultCode SignalEvent(const Handle handle) { - Event* evt = g_handle_table.Get(handle); + Event* evt = g_handle_table.Get(handle).get(); if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel); // Resume threads waiting for event to signal bool event_caught = false; for (size_t i = 0; i < evt->waiting_threads.size(); ++i) { - Thread* thread = Kernel::g_handle_table.Get(evt->waiting_threads[i]); + Thread* thread = Kernel::g_handle_table.Get(evt->waiting_threads[i]).get(); if (thread != nullptr) thread->ResumeFromWait(); @@ -112,7 +112,7 @@ ResultCode SignalEvent(const Handle handle) { * @return Result of operation, 0 on success, otherwise error code */ ResultCode ClearEvent(Handle handle) { - Event* evt = g_handle_table.Get(handle); + Event* evt = g_handle_table.Get(handle).get(); if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel); if (!evt->permanent_locked) { diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 9ac4a5adc..f32aed403 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -13,7 +13,7 @@ namespace Kernel { -Thread* g_main_thread = nullptr; +smart_ptr g_main_thread = nullptr; HandleTable g_handle_table; u64 g_program_id = 0; @@ -22,7 +22,7 @@ HandleTable::HandleTable() { Clear(); } -ResultVal HandleTable::Create(Object* obj) { +ResultVal HandleTable::Create(smart_ptr obj) { _dbg_assert_(Kernel, obj != nullptr); u16 slot = next_free_slot; @@ -38,22 +38,22 @@ ResultVal HandleTable::Create(Object* obj) { // CTR-OS doesn't use generation 0, so skip straight to 1. if (next_generation >= (1 << 15)) next_generation = 1; - generations[slot] = generation; - intrusive_ptr_add_ref(obj); - objects[slot] = obj; - Handle handle = generation | (slot << 15); obj->handle = handle; + + generations[slot] = generation; + objects[slot] = std::move(obj); + return MakeResult(handle); } ResultVal HandleTable::Duplicate(Handle handle) { - Object* object = GetGeneric(handle); + smart_ptr object = GetGeneric(handle); if (object == nullptr) { LOG_ERROR(Kernel, "Tried to duplicate invalid handle: %08X", handle); return ERR_INVALID_HANDLE; } - return Create(object); + return Create(std::move(object)); } ResultCode HandleTable::Close(Handle handle) { @@ -63,7 +63,6 @@ ResultCode HandleTable::Close(Handle handle) { size_t slot = GetSlot(handle); u16 generation = GetGeneration(handle); - intrusive_ptr_release(objects[slot]); objects[slot] = nullptr; generations[generation] = next_free_slot; @@ -78,7 +77,7 @@ bool HandleTable::IsValid(Handle handle) const { return slot < MAX_COUNT && objects[slot] != nullptr && generations[slot] == generation; } -Object* HandleTable::GetGeneric(Handle handle) const { +smart_ptr HandleTable::GetGeneric(Handle handle) const { if (handle == CurrentThread) { return GetCurrentThread(); } else if (handle == CurrentProcess) { @@ -95,8 +94,6 @@ Object* HandleTable::GetGeneric(Handle handle) const { void HandleTable::Clear() { for (size_t i = 0; i < MAX_COUNT; ++i) { generations[i] = i + 1; - if (objects[i] != nullptr) - intrusive_ptr_release(objects[i]); objects[i] = nullptr; } next_free_slot = 0; diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index 244527154..4be91e806 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -4,6 +4,8 @@ #pragma once +#include + #include #include #include "common/common.h" @@ -75,7 +77,7 @@ private: unsigned int ref_count = 0; }; -// Special functions that will later be used by boost::instrusive_ptr to do automatic ref-counting +// Special functions used by boost::instrusive_ptr to do automatic ref-counting inline void intrusive_ptr_add_ref(Object* object) { ++object->ref_count; } @@ -86,6 +88,9 @@ inline void intrusive_ptr_release(Object* object) { } } +template +using smart_ptr = boost::intrusive_ptr; + /** * This class allows the creation of Handles, which are references to objects that can be tested * for validity and looked up. Here they are used to pass references to kernel objects to/from the @@ -118,7 +123,7 @@ public: * @return The created Handle or one of the following errors: * - `ERR_OUT_OF_HANDLES`: the maximum number of handles has been exceeded. */ - ResultVal Create(Object* obj); + ResultVal Create(smart_ptr obj); /** * Returns a new handle that points to the same object as the passed in handle. @@ -142,7 +147,7 @@ public: * Looks up a handle. * @returns Pointer to the looked-up object, or `nullptr` if the handle is not valid. */ - Object* GetGeneric(Handle handle) const; + smart_ptr GetGeneric(Handle handle) const; /** * Looks up a handle while verifying its type. @@ -150,10 +155,10 @@ public: * type differs from the handle type `T::HANDLE_TYPE`. */ template - T* Get(Handle handle) const { - Object* object = GetGeneric(handle); + smart_ptr Get(Handle handle) const { + smart_ptr object = GetGeneric(handle); if (object != nullptr && object->GetHandleType() == T::HANDLE_TYPE) { - return static_cast(object); + return boost::static_pointer_cast(object); } return nullptr; } @@ -172,7 +177,7 @@ private: static u16 GetGeneration(Handle handle) { return handle & 0x7FFF; } /// Stores the Object referenced by the handle or null if the slot is empty. - std::array objects; + std::array, MAX_COUNT> objects; /** * The value of `next_generation` when the handle was created, used to check for validity. For @@ -191,7 +196,7 @@ private: }; extern HandleTable g_handle_table; -extern Thread* g_main_thread; +extern smart_ptr g_main_thread; /// The ID code of the currently running game /// TODO(Subv): This variable should not be here, diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index cc3128b9b..7b7a4d4d5 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -48,7 +48,7 @@ void MutexAcquireLock(Mutex* mutex, Handle thread = GetCurrentThread()->GetHandl bool ReleaseMutexForThread(Mutex* mutex, Handle thread_handle) { MutexAcquireLock(mutex, thread_handle); - Thread* thread = Kernel::g_handle_table.Get(thread_handle); + Thread* thread = Kernel::g_handle_table.Get(thread_handle).get(); if (thread != nullptr) thread->ResumeFromWait(); return true; @@ -90,7 +90,7 @@ void ReleaseThreadMutexes(Handle thread) { // Release every mutex that the thread holds, and resume execution on the waiting threads for (MutexMap::iterator iter = locked.first; iter != locked.second; ++iter) { - Mutex* mutex = g_handle_table.Get(iter->second); + Mutex* mutex = g_handle_table.Get(iter->second).get(); ResumeWaitingThread(mutex); } @@ -118,7 +118,7 @@ bool ReleaseMutex(Mutex* mutex) { * @param handle Handle to mutex to release */ ResultCode ReleaseMutex(Handle handle) { - Mutex* mutex = Kernel::g_handle_table.Get(handle); + Mutex* mutex = Kernel::g_handle_table.Get(handle).get(); if (mutex == nullptr) return InvalidHandle(ErrorModule::Kernel); if (!ReleaseMutex(mutex)) { diff --git a/src/core/hle/kernel/semaphore.cpp b/src/core/hle/kernel/semaphore.cpp index d7eeaa3da..88ec9a104 100644 --- a/src/core/hle/kernel/semaphore.cpp +++ b/src/core/hle/kernel/semaphore.cpp @@ -70,7 +70,7 @@ ResultCode CreateSemaphore(Handle* handle, s32 initial_count, } ResultCode ReleaseSemaphore(s32* count, Handle handle, s32 release_count) { - Semaphore* semaphore = g_handle_table.Get(handle); + Semaphore* semaphore = g_handle_table.Get(handle).get(); if (semaphore == nullptr) return InvalidHandle(ErrorModule::Kernel); @@ -84,7 +84,7 @@ ResultCode ReleaseSemaphore(s32* count, Handle handle, s32 release_count) { // Notify some of the threads that the semaphore has been released // stop once the semaphore is full again or there are no more waiting threads while (!semaphore->waiting_threads.empty() && semaphore->IsAvailable()) { - Thread* thread = Kernel::g_handle_table.Get(semaphore->waiting_threads.front()); + Thread* thread = Kernel::g_handle_table.Get(semaphore->waiting_threads.front()).get(); if (thread != nullptr) thread->ResumeFromWait(); semaphore->waiting_threads.pop(); diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index cea1f6fa1..5368e4728 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -61,7 +61,7 @@ ResultCode MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions return ResultCode(ErrorDescription::InvalidAddress, ErrorModule::Kernel, ErrorSummary::InvalidArgument, ErrorLevel::Permanent); } - SharedMemory* shared_memory = Kernel::g_handle_table.Get(handle); + SharedMemory* shared_memory = Kernel::g_handle_table.Get(handle).get(); if (shared_memory == nullptr) return InvalidHandle(ErrorModule::Kernel); shared_memory->base_address = address; @@ -72,7 +72,7 @@ ResultCode MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions } ResultVal GetSharedMemoryPointer(Handle handle, u32 offset) { - SharedMemory* shared_memory = Kernel::g_handle_table.Get(handle); + SharedMemory* shared_memory = Kernel::g_handle_table.Get(handle).get(); if (shared_memory == nullptr) return InvalidHandle(ErrorModule::Kernel); if (0 != shared_memory->base_address) diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 351dc24f2..452037284 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -35,7 +35,7 @@ ResultVal Thread::WaitSynchronization() { } // Lists all thread ids that aren't deleted/etc. -static std::vector thread_queue; // TODO(yuriks): Owned +static std::vector> thread_queue; // Lists only ready thread ids. static Common::ThreadQueueList thread_ready_queue; @@ -109,8 +109,8 @@ void Thread::Stop(const char* reason) { ChangeReadyState(this, false); status = THREADSTATUS_DORMANT; - for (Thread* waiting_thread : waiting_threads) { - if (CheckWaitType(waiting_thread, WAITTYPE_THREADEND, this)) + for (auto& waiting_thread : waiting_threads) { + if (CheckWaitType(waiting_thread.get(), WAITTYPE_THREADEND, this)) waiting_thread->ResumeFromWait(); } waiting_threads.clear(); @@ -142,15 +142,15 @@ Thread* ArbitrateHighestPriorityThread(Object* arbiter, u32 address) { s32 priority = THREADPRIO_LOWEST; // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (Thread* thread : thread_queue) { - if (!CheckWaitType(thread, WAITTYPE_ARB, arbiter, address)) + for (auto& thread : thread_queue) { + if (!CheckWaitType(thread.get(), WAITTYPE_ARB, arbiter, address)) continue; if (thread == nullptr) continue; // TODO(yuriks): Thread handle will hang around forever. Should clean up. if(thread->current_priority <= priority) { - highest_priority_thread = thread; + highest_priority_thread = thread.get(); priority = thread->current_priority; } } @@ -167,8 +167,8 @@ Thread* ArbitrateHighestPriorityThread(Object* arbiter, u32 address) { void ArbitrateAllThreads(Object* arbiter, u32 address) { // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (Thread* thread : thread_queue) { - if (CheckWaitType(thread, WAITTYPE_ARB, arbiter, address)) + for (auto& thread : thread_queue) { + if (CheckWaitType(thread.get(), WAITTYPE_ARB, arbiter, address)) thread->ResumeFromWait(); } } @@ -251,15 +251,15 @@ static void DebugThreadQueue() { return; } LOG_DEBUG(Kernel, "0x%02X 0x%08X (current)", thread->current_priority, GetCurrentThread()->GetHandle()); - for (Thread* t : thread_queue) { - s32 priority = thread_ready_queue.contains(t); + for (auto& t : thread_queue) { + s32 priority = thread_ready_queue.contains(t.get()); if (priority != -1) { LOG_DEBUG(Kernel, "0x%02X 0x%08X", priority, t->GetHandle()); } } } -ResultVal Thread::Create(const char* name, u32 entry_point, s32 priority, u32 arg, +ResultVal> Thread::Create(const char* name, u32 entry_point, s32 priority, u32 arg, s32 processor_id, u32 stack_top, int stack_size) { _dbg_assert_(Kernel, name != nullptr); @@ -313,7 +313,7 @@ ResultVal Thread::Create(const char* name, u32 entry_point, s32 priorit ResetThread(thread, arg, 0); CallThread(thread); - return MakeResult(thread); + return MakeResult>(thread); } /// Set the priority of the thread specified by handle @@ -343,13 +343,13 @@ void Thread::SetPriority(s32 priority) { } /// Sets up the primary application thread -Thread* SetupMainThread(s32 priority, int stack_size) { +smart_ptr SetupMainThread(s32 priority, int stack_size) { // Initialize new "main" thread - ResultVal thread_res = Thread::Create("main", Core::g_app_core->GetPC(), priority, 0, + ResultVal> thread_res = Thread::Create("main", Core::g_app_core->GetPC(), priority, 0, THREADPROCESSORID_0, Memory::SCRATCHPAD_VADDR_END, stack_size); // TODO(yuriks): Propagate error _dbg_assert_(Kernel, thread_res.Succeeded()); - Thread* thread = *thread_res; + smart_ptr thread = std::move(*thread_res); // If running another thread already, set it to "ready" state Thread* cur = GetCurrentThread(); @@ -358,7 +358,7 @@ Thread* SetupMainThread(s32 priority, int stack_size) { } // Run new "main" thread - current_thread = thread; + current_thread = thread.get(); thread->status = THREADSTATUS_RUNNING; Core::g_app_core->LoadContext(thread->context); @@ -378,7 +378,7 @@ void Reschedule() { } else { LOG_TRACE(Kernel, "cannot context switch from 0x%08X, no higher priority thread!", prev->GetHandle()); - for (Thread* thread : thread_queue) { + for (auto& thread : thread_queue) { LOG_TRACE(Kernel, "\thandle=0x%08X prio=0x%02X, status=0x%08X wait_type=0x%08X wait_handle=0x%08X", thread->GetHandle(), thread->current_priority, thread->status, thread->wait_type, thread->wait_object->GetHandle()); } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 5f404093f..6b2f2e8ec 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -53,7 +53,7 @@ namespace Kernel { class Thread : public Kernel::Object { public: - static ResultVal Create(const char* name, u32 entry_point, s32 priority, u32 arg, + static ResultVal> Create(const char* name, u32 entry_point, s32 priority, u32 arg, s32 processor_id, u32 stack_top, int stack_size = Kernel::DEFAULT_STACK_SIZE); std::string GetName() const override { return name; } @@ -97,7 +97,7 @@ public: Object* wait_object; VAddr wait_address; - std::vector waiting_threads; // TODO(yuriks): Owned + std::vector> waiting_threads; std::string name; @@ -106,7 +106,7 @@ private: }; /// Sets up the primary application thread -Thread* SetupMainThread(s32 priority, int stack_size = Kernel::DEFAULT_STACK_SIZE); +smart_ptr SetupMainThread(s32 priority, int stack_size = Kernel::DEFAULT_STACK_SIZE); /// Reschedules to the next available thread (call after current thread is suspended) void Reschedule(); diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index 0f3cc2aa8..8c559467a 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -71,7 +71,8 @@ void Manager::DeleteService(const std::string& port_name) { /// Get a Service Interface from its Handle Interface* Manager::FetchFromHandle(Handle handle) { - return Kernel::g_handle_table.Get(handle); + // TODO(yuriks): This function is very suspicious and should probably be exterminated. + return Kernel::g_handle_table.Get(handle).get(); } /// Get a Service Interface from its port diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index e607b5b8b..1d59db594 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -26,6 +26,8 @@ namespace SVC { +using Kernel::smart_ptr; + enum ControlMemoryOperation { MEMORY_OPERATION_HEAP = 0x00000003, MEMORY_OPERATION_GSP_HEAP = 0x00010003, @@ -93,7 +95,7 @@ static Result ConnectToPort(Handle* out, const char* port_name) { /// Synchronize to an OS service static Result SendSyncRequest(Handle handle) { - Kernel::Session* session = Kernel::g_handle_table.Get(handle); + smart_ptr session = Kernel::g_handle_table.Get(handle); if (session == nullptr) { return InvalidHandle(ErrorModule::Kernel).raw; } @@ -120,7 +122,7 @@ static Result WaitSynchronization1(Handle handle, s64 nano_seconds) { // TODO(bunnei): Do something with nano_seconds, currently ignoring this bool wait_infinite = (nano_seconds == -1); // Used to wait until a thread has terminated - Kernel::Object* object = Kernel::g_handle_table.GetGeneric(handle); + smart_ptr object = Kernel::g_handle_table.GetGeneric(handle); if (object == nullptr) return InvalidHandle(ErrorModule::Kernel).raw; @@ -149,7 +151,7 @@ static Result WaitSynchronizationN(s32* out, Handle* handles, s32 handle_count, // Iterate through each handle, synchronize kernel object for (s32 i = 0; i < handle_count; i++) { - Kernel::Object* object = Kernel::g_handle_table.GetGeneric(handles[i]); + smart_ptr object = Kernel::g_handle_table.GetGeneric(handles[i]); if (object == nullptr) return InvalidHandle(ErrorModule::Kernel).raw; @@ -221,6 +223,8 @@ static Result GetResourceLimitCurrentValues(s64* values, Handle resource_limit, /// Creates a new thread static Result CreateThread(u32 priority, u32 entry_point, u32 arg, u32 stack_top, u32 processor_id) { + using Kernel::Thread; + std::string name; if (Symbols::HasSymbol(entry_point)) { TSymbol symbol = Symbols::GetSymbol(entry_point); @@ -229,11 +233,11 @@ static Result CreateThread(u32 priority, u32 entry_point, u32 arg, u32 stack_top name = Common::StringFromFormat("unknown-%08x", entry_point); } - ResultVal thread_res = Kernel::Thread::Create(name.c_str(), entry_point, priority, arg, + ResultVal> thread_res = Kernel::Thread::Create(name.c_str(), entry_point, priority, arg, processor_id, stack_top); if (thread_res.Failed()) return thread_res.Code().raw; - Kernel::Thread* thread = *thread_res; + smart_ptr thread = std::move(*thread_res); Core::g_app_core->SetReg(1, thread->GetHandle()); @@ -254,7 +258,7 @@ static void ExitThread() { /// Gets the priority for the specified thread static Result GetThreadPriority(s32* priority, Handle handle) { - const Kernel::Thread* thread = Kernel::g_handle_table.Get(handle); + const smart_ptr thread = Kernel::g_handle_table.Get(handle); if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel).raw; @@ -264,7 +268,7 @@ static Result GetThreadPriority(s32* priority, Handle handle) { /// Sets the priority for the specified thread static Result SetThreadPriority(Handle handle, s32 priority) { - Kernel::Thread* thread = Kernel::g_handle_table.Get(handle); + smart_ptr thread = Kernel::g_handle_table.Get(handle); if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel).raw; @@ -291,7 +295,7 @@ static Result ReleaseMutex(Handle handle) { static Result GetThreadId(u32* thread_id, Handle handle) { LOG_TRACE(Kernel_SVC, "called thread=0x%08X", handle); - const Kernel::Thread* thread = Kernel::g_handle_table.Get(handle); + const smart_ptr thread = Kernel::g_handle_table.Get(handle); if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel).raw;