From 966bf1d6afbf946ac92931b7d572fd76d79c56b0 Mon Sep 17 00:00:00 2001 From: Alex Dcnh <140754794+Wishmaster117@users.noreply.github.com> Date: Fri, 8 Aug 2025 20:36:03 +0200 Subject: [PATCH] Correct side effects of merge f5ef5bd1c2039290be1b1108872d046e1e65ebe1 (#1512) * Update PlayerbotMgr.h * Update PlayerbotMgr.cpp * Update PlayerbotMgr.cpp * Update PlayerbotMgr.cpp * Update PlayerbotMgr.cpp * Update PlayerbotMgr.cpp * Update PlayerbotMgr.cpp * Update PlayerbotMgr.h * Update PlayerbotMgr.cpp * Update PlayerbotMgr.h * Update PlayerbotMgr.cpp * Update PlayerbotMgr.h * Update PlayerbotMgr.h --- src/PlayerbotMgr.cpp | 53 +++++++++++++++++++++++++++++--------------- src/PlayerbotMgr.h | 25 ++++++++++++++++++++- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/PlayerbotMgr.cpp b/src/PlayerbotMgr.cpp index 591512e3..0b6d3967 100644 --- a/src/PlayerbotMgr.cpp +++ b/src/PlayerbotMgr.cpp @@ -1745,36 +1745,53 @@ PlayerbotAI* PlayerbotsMgr::GetPlayerbotAI(Player* player) // return nullptr; // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) - if (!sPlayerbotAIConfig->enabled || !player) + if (!player || !sPlayerbotAIConfig->enabled) return nullptr; - - { // protected read - std::shared_lock lock(_aiMutex); - auto itr = _playerbotsAIMap.find(player->GetGUID()); - if (itr != _playerbotsAIMap.end() && itr->second->IsBotAI()) - return reinterpret_cast(itr->second); + + // First read the GUID into a local variable, but ONLY after the check! + ObjectGuid guid = player->GetGUID(); // <-- OK here, we know that player != nullptr + { + std::shared_lock rlock(_aiMutex); + auto it = _playerbotsAIMap.find(guid); + if (it != _playerbotsAIMap.end() && it->second->IsBotAI()) + return static_cast(it->second); } - // does the player still exist? - if (!ObjectAccessor::FindPlayer(player->GetGUID())) - RemovePlayerbotAI(player->GetGUID()); // orphaned AI -> cleanup - + // Transient state: NEVER break the master ⇄ bots relationship here. + if (!ObjectAccessor::FindPlayer(guid)) + { + RemovePlayerbotAI(guid, /*removeMgrEntry=*/false); + } return nullptr; } // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) -void PlayerbotsMgr::RemovePlayerbotAI(ObjectGuid const& guid) +PlayerbotAI* PlayerbotsMgr::GetPlayerbotAIByGuid(ObjectGuid guid) { - std::unique_lock lock(_aiMutex); + if (!sPlayerbotAIConfig->enabled) + return nullptr; - if (auto itr = _playerbotsAIMap.find(guid); itr != _playerbotsAIMap.end()) + std::shared_lock rlock(_aiMutex); + auto it = _playerbotsAIMap.find(guid); + if (it != _playerbotsAIMap.end() && it->second->IsBotAI()) + return static_cast(it->second); + return nullptr; +} + +void PlayerbotsMgr::RemovePlayerbotAI(ObjectGuid const& guid, bool removeMgrEntry /*= true*/) +{ + std::unique_lock wlock(_aiMutex); + + if (auto it = _playerbotsAIMap.find(guid); it != _playerbotsAIMap.end()) { - delete itr->second; - _playerbotsAIMap.erase(itr); - LOG_DEBUG("playerbots", "Removed stale AI entry for GUID {}", static_cast(guid.GetRawValue())); + delete it->second; + _playerbotsAIMap.erase(it); + LOG_DEBUG("playerbots", "Removed stale AI for GUID {}", + static_cast(guid.GetRawValue())); } - _playerbotsMgrMap.erase(guid); + if (removeMgrEntry) + _playerbotsMgrMap.erase(guid); // we NO longer touch the relation in a "soft" purge } PlayerbotMgr* PlayerbotsMgr::GetPlayerbotMgr(Player* player) diff --git a/src/PlayerbotMgr.h b/src/PlayerbotMgr.h index 44343163..17a49612 100644 --- a/src/PlayerbotMgr.h +++ b/src/PlayerbotMgr.h @@ -115,7 +115,11 @@ public: void RemovePlayerBotData(ObjectGuid const& guid, bool is_AI); PlayerbotAI* GetPlayerbotAI(Player* player); - void RemovePlayerbotAI(ObjectGuid const& guid); // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) + PlayerbotAI* GetPlayerbotAIByGuid(ObjectGuid guid); // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) + // void RemovePlayerbotAI(ObjectGuid const& guid); // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) + // removeMgrEntry = true => "hard" purge (AI + manager relation), for real logouts + // removeMgrEntry = false => "soft" purge (AI only), for detected "stale" cases + void RemovePlayerbotAI(ObjectGuid const& guid, bool removeMgrEntry = true); PlayerbotMgr* GetPlayerbotMgr(Player* player); private: @@ -126,4 +130,23 @@ private: #define sPlayerbotsMgr PlayerbotsMgr::instance() +// Temporary addition If it keeps crashing, we will use them. +// Like +// BEFORE : PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot); +// AFTER (safe) : PlayerbotAI* botAI = GET_PLAYERBOT_AI_SAFE(bot); +// BEFORE : if (PlayerbotAI* botAI = GET_PLAYERBOT_AI(player)) { ... } +// AFTER (safe) : if (PlayerbotAI* botAI = GET_PLAYERBOT_AI_SAFE(player)) { ... } +// --- SAFE helpers (append to PlayerbotMgr.h) --- +inline PlayerbotAI* GET_PLAYERBOT_AI_SAFE(Player* p) +{ + // Avoid any dereference during transient states (nullptr, teleport, flight, etc.) + return p ? sPlayerbotsMgr->GetPlayerbotAI(p) : nullptr; +} + +inline PlayerbotMgr* GET_PLAYERBOT_MGR_SAFE(Player* p) +{ + return p ? sPlayerbotsMgr->GetPlayerbotMgr(p) : nullptr; +} +// --- end SAFE helpers --- + #endif