From c9b4cfa184f021abf38436a6c8739a458339891e Mon Sep 17 00:00:00 2001 From: bash <31279994+hermensbas@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:13:45 +0200 Subject: [PATCH] [Revert] Threading leftover which belonged to other related PRs's (once green needs be merged) (#1583) * Revert "Correct side effects of merge f5ef5bd1c2039290be1b1108872d046e1e65ebe1 (#1512)" This reverts commit 966bf1d6afbf946ac92931b7d572fd76d79c56b0. * Revert "Fix ACCESS_VIOLATION in mod-playerbots: purge stale AIs, add thread-safety, and harden HasRealPlayerMaster (#1507)" This reverts commit f5ef5bd1c2039290be1b1108872d046e1e65ebe1. --- src/PlayerbotAI.cpp | 30 +++---------------- src/PlayerbotMgr.cpp | 71 +++++++------------------------------------- src/PlayerbotMgr.h | 26 ---------------- src/Playerbots.cpp | 4 --- 4 files changed, 14 insertions(+), 117 deletions(-) diff --git a/src/PlayerbotAI.cpp b/src/PlayerbotAI.cpp index 2b2bc89b..5dbe5cb9 100644 --- a/src/PlayerbotAI.cpp +++ b/src/PlayerbotAI.cpp @@ -4124,37 +4124,15 @@ bool IsAlliance(uint8 race) bool PlayerbotAI::HasRealPlayerMaster() { -// if (master) -// { -// PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(master); -// return !masterBotAI || masterBotAI->IsRealPlayer(); -// } -// -// return false; - - // Removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) - /* 1) The "master" pointer can be null if the bot was created - without a master player or if the master was just removed. */ - if (!master) - return false; - - /* 2) Is the master player still present in the world? - If FindPlayer fails, we invalidate "master" and stop here. */ - if (!ObjectAccessor::FindPlayer(master->GetGUID())) + if (master) { - master = nullptr; // avoids repeating the check on the next tick - return false; + PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(master); + return !masterBotAI || masterBotAI->IsRealPlayer(); } - /* 3) If the master is a bot, we check that it is itself controlled - by a real player. Otherwise, it's already a real player → true. */ - if (PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(master)) - return masterBotAI->IsRealPlayer(); // bot controlled by a player? - - return true; // master = real player + return false; } - bool PlayerbotAI::HasActivePlayerMaster() { return master && !GET_PLAYERBOT_AI(master); } bool PlayerbotAI::IsAlt() { return HasRealPlayerMaster() && !sRandomPlayerbotMgr->IsRandomBot(bot); } diff --git a/src/PlayerbotMgr.cpp b/src/PlayerbotMgr.cpp index 0b6d3967..027e2bab 100644 --- a/src/PlayerbotMgr.cpp +++ b/src/PlayerbotMgr.cpp @@ -38,8 +38,6 @@ #include "WorldSessionMgr.h" #include "DatabaseEnv.h" // Added for gender choice #include // Added for gender choice -#include "Log.h" // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) -#include // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) class BotInitGuard { @@ -1728,70 +1726,21 @@ void PlayerbotsMgr::RemovePlayerBotData(ObjectGuid const& guid, bool is_AI) PlayerbotAI* PlayerbotsMgr::GetPlayerbotAI(Player* player) { - // if (!(sPlayerbotAIConfig->enabled) || !player) - // { + if (!(sPlayerbotAIConfig->enabled) || !player) + { + return nullptr; + } + // if (player->GetSession()->isLogingOut() || player->IsDuringRemoveFromWorld()) { // return nullptr; // } - // // if (player->GetSession()->isLogingOut() || player->IsDuringRemoveFromWorld()) { - // // return nullptr; - // // } - // auto itr = _playerbotsAIMap.find(player->GetGUID()); - // if (itr != _playerbotsAIMap.end()) - // { - // if (itr->second->IsBotAI()) - // return reinterpret_cast(itr->second); - // } - // - // return nullptr; - - // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) - if (!player || !sPlayerbotAIConfig->enabled) - return nullptr; - - // 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); - } - - // Transient state: NEVER break the master ⇄ bots relationship here. - if (!ObjectAccessor::FindPlayer(guid)) + auto itr = _playerbotsAIMap.find(player->GetGUID()); + if (itr != _playerbotsAIMap.end()) { - RemovePlayerbotAI(guid, /*removeMgrEntry=*/false); - } - return nullptr; -} - -// removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) -PlayerbotAI* PlayerbotsMgr::GetPlayerbotAIByGuid(ObjectGuid guid) -{ - if (!sPlayerbotAIConfig->enabled) - return nullptr; - - 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 it->second; - _playerbotsAIMap.erase(it); - LOG_DEBUG("playerbots", "Removed stale AI for GUID {}", - static_cast(guid.GetRawValue())); + if (itr->second->IsBotAI()) + return reinterpret_cast(itr->second); } - if (removeMgrEntry) - _playerbotsMgrMap.erase(guid); // we NO longer touch the relation in a "soft" purge + return nullptr; } PlayerbotMgr* PlayerbotsMgr::GetPlayerbotMgr(Player* player) diff --git a/src/PlayerbotMgr.h b/src/PlayerbotMgr.h index 17a49612..5ef31776 100644 --- a/src/PlayerbotMgr.h +++ b/src/PlayerbotMgr.h @@ -12,7 +12,6 @@ #include "PlayerbotAIBase.h" #include "QueryHolder.h" #include "QueryResult.h" -#include // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) class ChatHandler; class PlayerbotAI; @@ -115,38 +114,13 @@ public: void RemovePlayerBotData(ObjectGuid const& guid, bool is_AI); PlayerbotAI* GetPlayerbotAI(Player* player); - 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: std::unordered_map _playerbotsAIMap; std::unordered_map _playerbotsMgrMap; - mutable std::shared_mutex _aiMutex; // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) }; #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 diff --git a/src/Playerbots.cpp b/src/Playerbots.cpp index 76b32efa..ed44a70f 100644 --- a/src/Playerbots.cpp +++ b/src/Playerbots.cpp @@ -377,10 +377,6 @@ public: void OnPlayerbotLogout(Player* player) override { - // immediate purge of the bot's AI upon disconnection - if (player && player->GetSession()->IsBot()) - sPlayerbotsMgr->RemovePlayerbotAI(player->GetGUID()); // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION) - if (PlayerbotMgr* playerbotMgr = GET_PLAYERBOT_MGR(player)) { PlayerbotAI* botAI = GET_PLAYERBOT_AI(player);