fix(eventbus): Prevent dangling pointer crash during bot cleanup

During bot destruction, the Player object may be destroyed before
the BotAI destructor runs, causing _bot to become a dangling pointer.
When UnsubscribeFromEventBuses() tries to access _bot->GetGUID(),
it crashes with ACCESS_VIOLATION at GenericEventBus.h line 277.

Fix:
- Add _cachedBotGuid member to BotAI, initialized at construction
- Add UnsubscribeByGuid() method to GenericEventBus template
- Update UnsubscribeFromEventBuses() to use cached GUID directly
  via EventBus<T>::instance()->UnsubscribeByGuid(_cachedBotGuid)

This ensures safe cleanup even when Player is already destroyed.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
This commit is contained in:
agatho
2026-02-04 20:25:02 -03:00
committed by luis
co-authored by Claude Opus 4.5
parent 7562fa663b
commit 38c85f0a05
4 changed files with 51 additions and 19 deletions
+4 -1
View File
@@ -124,7 +124,10 @@ std::vector<uint32> BotAI::GetCompletableQuestIds() const
// CONSTRUCTOR / DESTRUCTOR
// ============================================================================
BotAI::BotAI(Player* bot, bool instanceOnlyMode) : _bot(bot), _instanceOnlyMode(instanceOnlyMode)
BotAI::BotAI(Player* bot, bool instanceOnlyMode)
: _bot(bot)
, _cachedBotGuid(bot ? bot->GetGUID() : ObjectGuid::Empty) // Cache for safe destructor cleanup
, _instanceOnlyMode(instanceOnlyMode)
{
// Initialize performance tracking
_performanceMetrics.lastUpdate = std::chrono::steady_clock::now();
+2
View File
@@ -321,6 +321,7 @@ public:
Player* GetBot() const { return _bot; }
ObjectGuid GetBotGuid() const { return _bot ? _bot->GetGUID() : ObjectGuid::Empty; }
ObjectGuid GetCachedBotGuid() const { return _cachedBotGuid; } // Safe during destructor
// ========================================================================
// LIFECYCLE MANAGEMENT - Two-Phase AddToWorld Pattern
@@ -957,6 +958,7 @@ protected:
protected:
// Core components
Player* _bot;
ObjectGuid _cachedBotGuid; // Cached at construction for safe destructor cleanup
BotAIState _aiState = BotAIState::SOLO;
ObjectGuid _currentTarget;
@@ -23,6 +23,7 @@
#include "BotAI.h"
#include "GameTime.h"
#include "Core/Events/GenericEventBus.h"
#include "Group/GroupEventBus.h"
#include "Combat/CombatEventBus.h"
#include "Cooldown/CooldownEventBus.h"
@@ -92,28 +93,29 @@ void BotAI::SubscribeToEventBuses()
void BotAI::UnsubscribeFromEventBuses()
{
if (!_bot)
// CRITICAL FIX: Use cached GUID instead of _bot pointer during destructor
// The Player object may already be destroyed when BotAI destructor runs,
// making _bot a dangling pointer. Using the cached GUID is safe.
if (_cachedBotGuid.IsEmpty())
return;
// CRITICAL: Unsubscribe from all event buses to prevent dangling pointers
GroupEventBus::instance()->Unsubscribe(this);
CombatEventBus::instance()->Unsubscribe(this);
CooldownEventBus::instance()->Unsubscribe(this);
AuraEventBus::instance()->Unsubscribe(this);
LootEventBus::instance()->Unsubscribe(this);
QuestEventBus::instance()->Unsubscribe(this);
ResourceEventBus::instance()->Unsubscribe(this);
SocialEventBus::instance()->Unsubscribe(this);
AuctionEventBus::instance()->Unsubscribe(this);
NPCEventBus::instance()->Unsubscribe(this);
InstanceEventBus::instance()->Unsubscribe(this);
ProfessionEventBus::instance()->Unsubscribe(this);
// CRITICAL: Unsubscribe from all event buses using GUID (safe during destructor)
// Call the underlying template directly with the new UnsubscribeByGuid method
EventBus<GroupEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<CombatEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<CooldownEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<AuraEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<LootEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<QuestEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<ResourceEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<SocialEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<AuctionEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<NPCEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<InstanceEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
EventBus<ProfessionEvent>::instance()->UnsubscribeByGuid(_cachedBotGuid);
// CRITICAL: Use GetGUID().ToString() instead of GetName() during destructor
// GetName() may access invalid memory during bot destruction
// This prevents ACCESS_VIOLATION crash at BotAI_EventHandlers.cpp line 110
TC_LOG_DEBUG("playerbot.events", "Bot unsubscribed from all event buses (GUID: {})",
_bot->GetGUID().ToString());
_cachedBotGuid.ToString());
}
// ============================================================================
@@ -285,6 +285,31 @@ public:
subscriberGuid.ToString());
}
/**
* @brief Unsubscribe a bot by GUID (safe for use during destructor)
*
* This overload is specifically designed for use during BotAI destruction,
* when the Player object may already be destroyed but we have a cached GUID.
*
* @param botGuid The cached GUID of the bot to unsubscribe
*
* Thread Safety: Yes (mutex-protected)
* Performance: O(1) average case (hash map erase)
*/
void UnsubscribeByGuid(ObjectGuid const& botGuid)
{
if (botGuid.IsEmpty())
return;
std::lock_guard lock(_subscriptionMutex);
_subscriptions.erase(botGuid);
_subscriberPointers.erase(botGuid);
TC_LOG_DEBUG("playerbot.events", "EventBus: Bot {} unsubscribed by GUID from all events",
botGuid.ToString());
}
/**
* @brief Unsubscribe a bot from specific event types
*