From e01f5e58c8dd8197b118c4fe931d19768425ea12 Mon Sep 17 00:00:00 2001 From: 0blu Date: Sat, 31 Aug 2024 21:04:07 +0200 Subject: [PATCH] Fix shutdown crash when with `ThreadSpecificPtr` --- src/shared/ThreadSpecificPtr.cpp | 12 +++++++- src/shared/ThreadSpecificPtr.h | 47 +++++++++++++++++--------------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/shared/ThreadSpecificPtr.cpp b/src/shared/ThreadSpecificPtr.cpp index bc83309c668..dd74e937709 100644 --- a/src/shared/ThreadSpecificPtr.cpp +++ b/src/shared/ThreadSpecificPtr.cpp @@ -1,3 +1,13 @@ #include "./ThreadSpecificPtr.h" +#include "Errors.h" -thread_local std::map MaNGOS::thread_specific_ptr_data; +thread_local MaNGOS::ThreadSpecificHolder MaNGOS::gtl_ThreadSpecificPtrHolder; + +MaNGOS::ThreadSpecificHolder::~ThreadSpecificHolder() noexcept +{ + for (auto it : thread_specific_ptr_data) + { + // Every reference must be deleted before the holder is deallocated, otherwise memory will leak + MANGOS_ASSERT(it.second == nullptr); + } +} diff --git a/src/shared/ThreadSpecificPtr.h b/src/shared/ThreadSpecificPtr.h index db5573f4213..c5a9d6cb3c0 100644 --- a/src/shared/ThreadSpecificPtr.h +++ b/src/shared/ThreadSpecificPtr.h @@ -16,40 +16,39 @@ #ifndef MANGOS_THREAD_SPECIFIC_PTR_H_ #define MANGOS_THREAD_SPECIFIC_PTR_H_ +#include "Policies/NoCopyNoMove.h" + #include #include namespace MaNGOS { - extern thread_local std::map thread_specific_ptr_data; + struct ThreadSpecificHolder + { + ~ThreadSpecificHolder() noexcept; + std::map thread_specific_ptr_data; + }; + + extern thread_local ThreadSpecificHolder gtl_ThreadSpecificPtrHolder; /// This class is the same as `boost::thread_specific_ptr` or `ACE_TSS` /// Since you cant use `thread_local` on a non static class member, /// this class allows you to have thread specific storage on instances bases. /// Meaning instanceA and instanceB on the same thread, have a different object attached to it. template - class ThreadSpecificPtr + class ThreadSpecificPtr : public MaNGOS::Policies::NoCopyNoMove { public: - // not copy, not movable - ThreadSpecificPtr(const ThreadSpecificPtr&) = delete; - ThreadSpecificPtr& operator=(const ThreadSpecificPtr&) = delete; - ThreadSpecificPtr(ThreadSpecificPtr&&) = delete; - ThreadSpecificPtr& operator=(ThreadSpecificPtr&&) = delete; - ThreadSpecificPtr() = default; - ~ThreadSpecificPtr() - { - reset(); - // Finally, remove the class instance from the map - thread_specific_ptr_data.erase(this); - } + // Important: We cannot `this->reset()` our pointer, because the holder might already be deallocated when ThreadSpecificPtr was used in a global context + // We have to trust that the user cleaned up everything (otherwise an ASSERT() in `ThreadSpecificHolder` will fail) + ~ThreadSpecificPtr() = default; T* get() const { - auto it = thread_specific_ptr_data.find(const_cast(static_cast(this))); - if (it != thread_specific_ptr_data.end()) + auto it = gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.find(const_cast(static_cast(this))); + if (it != gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.end()) return static_cast(it->second); return nullptr; } @@ -68,8 +67,8 @@ namespace MaNGOS /// Releases the pointer. You have ownership and have to delete it! T* release() { - auto it = thread_specific_ptr_data.find(this); - if (it == thread_specific_ptr_data.end()) + auto it = gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.find(this); + if (it == gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.end()) return nullptr; T* ptr = static_cast(it->second); @@ -80,10 +79,14 @@ namespace MaNGOS /// This function will delete the exising pointer void reset(T* new_value = nullptr) { - auto it = thread_specific_ptr_data.find(this); - if (it != thread_specific_ptr_data.end() && it->second != nullptr) - delete static_cast(it->second); - thread_specific_ptr_data[this] = new_value; + auto it = gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.find(this); + if (it != gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.end()) + { + if (it->second != nullptr) + delete static_cast(it->second); + } + + gtl_ThreadSpecificPtrHolder.thread_specific_ptr_data.insert(it, std::make_pair((void*)this, (void*)new_value)); } }; }