Fix ACCESS_VIOLATION in mod-playerbots: purge stale AIs, add thread-safety, and harden HasRealPlayerMaster (#1507)

src/PlayerbotAI.cpp

@@ -3970,15 +3970,37 @@ bool IsAlliance(uint8 race)
 
 bool PlayerbotAI::HasRealPlayerMaster()
 {
-    if (master)
+//     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()))
     {
-        PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(master);
-        return !masterBotAI || masterBotAI->IsRealPlayer();
+        master = nullptr;           // avoids repeating the check on the next tick
+        return false;
     }
 
-    return false;
+   /* 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
 }
 
+
 bool PlayerbotAI::HasActivePlayerMaster() { return master && !GET_PLAYERBOT_AI(master); }
 
 bool PlayerbotAI::IsAlt() { return HasRealPlayerMaster() && !sRandomPlayerbotMgr->IsRandomBot(bot); }

src/PlayerbotMgr.cpp

@@ -38,6 +38,8 @@
 #include "WorldSessionMgr.h"
 #include "DatabaseEnv.h"        // Added for gender choice
 #include <algorithm>            // Added for gender choice
+#include "Log.h" // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
+#include <shared_mutex> // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
 
 class BotInitGuard
 {
@@ -1726,23 +1728,55 @@ void PlayerbotsMgr::RemovePlayerBotData(ObjectGuid const& guid, bool is_AI)
 
 PlayerbotAI* PlayerbotsMgr::GetPlayerbotAI(Player* player)
 {
-    if (!(sPlayerbotAIConfig->enabled) || !player)
-    {
-        return nullptr;
-    }
-    // if (player->GetSession()->isLogingOut() || player->IsDuringRemoveFromWorld()) {
+    // if (!(sPlayerbotAIConfig->enabled) || !player)
+    // {
     //     return nullptr;
     // }
-    auto itr = _playerbotsAIMap.find(player->GetGUID());
-    if (itr != _playerbotsAIMap.end())
-    {
-        if (itr->second->IsBotAI())
+    // // 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<PlayerbotAI*>(itr->second);
+    // }
+	// 
+    // return nullptr;
+	
+	// removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
+    if (!sPlayerbotAIConfig->enabled || !player)
+        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<PlayerbotAI*>(itr->second);
     }
 
+    // does the player still exist?
+    if (!ObjectAccessor::FindPlayer(player->GetGUID()))
+        RemovePlayerbotAI(player->GetGUID());          // orphaned AI -> cleanup
+
     return nullptr;
 }
 
+// removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
+void PlayerbotsMgr::RemovePlayerbotAI(ObjectGuid const& guid)
+{
+    std::unique_lock lock(_aiMutex);
+
+    if (auto itr = _playerbotsAIMap.find(guid); itr != _playerbotsAIMap.end())
+    {
+        delete itr->second;
+        _playerbotsAIMap.erase(itr);
+        LOG_DEBUG("playerbots", "Removed stale AI entry for GUID {}", static_cast<uint64>(guid.GetRawValue()));
+    }
+
+    _playerbotsMgrMap.erase(guid);
+}
+
 PlayerbotMgr* PlayerbotsMgr::GetPlayerbotMgr(Player* player)
 {
     if (!(sPlayerbotAIConfig->enabled) || !player)

src/PlayerbotMgr.h

@@ -12,6 +12,7 @@
 #include "PlayerbotAIBase.h"
 #include "QueryHolder.h"
 #include "QueryResult.h"
+#include <shared_mutex>                 // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
 
 class ChatHandler;
 class PlayerbotAI;
@@ -114,11 +115,13 @@ 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)
     PlayerbotMgr* GetPlayerbotMgr(Player* player);
 
 private:
     std::unordered_map<ObjectGuid, PlayerbotAIBase*> _playerbotsAIMap;
     std::unordered_map<ObjectGuid, PlayerbotAIBase*> _playerbotsMgrMap;
+    mutable std::shared_mutex _aiMutex;   // removes a long-standing crash (0xC0000005 ACCESS_VIOLATION)
 };
 
 #define sPlayerbotsMgr PlayerbotsMgr::instance()

src/Playerbots.cpp

@@ -363,6 +363,10 @@ 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);