diff --git a/src/modules/Playerbot/Lifecycle/Instance/QueueStatePoller.cpp b/src/modules/Playerbot/Lifecycle/Instance/QueueStatePoller.cpp index 9a784e92c..245d17ca6 100644 --- a/src/modules/Playerbot/Lifecycle/Instance/QueueStatePoller.cpp +++ b/src/modules/Playerbot/Lifecycle/Instance/QueueStatePoller.cpp @@ -462,13 +462,14 @@ void QueueStatePoller::DoPollBGQueue(BattlegroundTypeId bgTypeId, BattlegroundBr snapshot.hordeCount = hordeCount; snapshot.minPlayersPerTeam = minPlayers; snapshot.maxPlayersPerTeam = maxPlayers; - snapshot.allianceShortage = static_cast(minPlayers) - static_cast(allianceCount); - snapshot.hordeShortage = static_cast(minPlayers) - static_cast(hordeCount); + // Use maxPlayers for shortage calculation - we want to fill the BG, not just meet minimum + snapshot.allianceShortage = static_cast(maxPlayers) - static_cast(allianceCount); + snapshot.hordeShortage = static_cast(maxPlayers) - static_cast(hordeCount); snapshot.timestamp = time(nullptr); TC_LOG_DEBUG("playerbot.jit", "QueueStatePoller: BG Poll - Type={} Bracket={} Alliance={}/{} Horde={}/{} Shortage=A:{}/H:{}", static_cast(bgTypeId), static_cast(bracket), - allianceCount, minPlayers, hordeCount, minPlayers, + allianceCount, maxPlayers, hordeCount, maxPlayers, snapshot.allianceShortage, snapshot.hordeShortage); // Process shortage if detected diff --git a/src/modules/Playerbot/Session/BotWorldSessionMgr.cpp b/src/modules/Playerbot/Session/BotWorldSessionMgr.cpp index fee1a81b9..555578f26 100644 --- a/src/modules/Playerbot/Session/BotWorldSessionMgr.cpp +++ b/src/modules/Playerbot/Session/BotWorldSessionMgr.cpp @@ -399,6 +399,19 @@ void BotWorldSessionMgr::RemovePlayerBot(ObjectGuid playerGuid) TC_LOG_INFO("module.playerbot.session", "Queuing bot for removal (name unavailable)"); } + // CRITICAL FIX (Map.cpp:1968 use-after-free crash): + // Mark bot as destroyed BEFORE clearing update mask to prevent re-adding to _updateObjects. + // + // Problem: After ClearUpdateMask(true) removes bot from _updateObjects, bot AI continues + // running and may modify properties. Property setters call AddToObjectUpdateIfNeeded() + // which re-adds the bot to _updateObjects. When bot is finally destroyed during logout, + // MapUpdater worker threads find a dangling pointer -> ACCESS_VIOLATION. + // + // Solution: Set m_isDestroyedObject=true FIRST. This blocks AddToObjectUpdateIfNeeded() + // from ever re-adding the bot to _updateObjects (check added in BaseEntity.cpp). + player->SetDestroyedObject(true); + TC_LOG_DEBUG("module.playerbot.session", "Marked bot {} as destroyed to prevent _updateObjects re-add", playerGuid.ToString()); + // CRITICAL FIX (Cell::Visit crash - CellImpl.h:65): // Clear visibility notification flags BEFORE queuing for removal. // diff --git a/src/server/game/Entities/Object/BaseEntity.cpp b/src/server/game/Entities/Object/BaseEntity.cpp index db10953c7..54416a2dc 100644 --- a/src/server/game/Entities/Object/BaseEntity.cpp +++ b/src/server/game/Entities/Object/BaseEntity.cpp @@ -636,7 +636,13 @@ UF::UpdateFieldFlag BaseEntity::GetUpdateFieldFlagsFor(Player const* /*target*/) void BaseEntity::AddToObjectUpdateIfNeeded() { - if (m_inWorld && !m_objectUpdated) + // PLAYERBOT FIX: Also check m_isDestroyedObject to prevent re-adding to _updateObjects + // after an object is marked for destruction but before actual deletion. + // Race condition: RemovePlayerBot() marks bot for removal, ClearUpdateMask(true) removes + // from _updateObjects, but bot AI continues running and modifies properties. Without this + // check, property setters would re-add the bot to _updateObjects, creating a dangling + // pointer when the bot is finally destroyed. + if (m_inWorld && !m_objectUpdated && !m_isDestroyedObject) m_objectUpdated = AddToObjectUpdate(); } diff --git a/src/server/game/Maps/Map.cpp b/src/server/game/Maps/Map.cpp index fa3ea1c2d..935e3bab0 100644 --- a/src/server/game/Maps/Map.cpp +++ b/src/server/game/Maps/Map.cpp @@ -1970,18 +1970,25 @@ void Map::SendObjectUpdates() BaseEntity* obj = *_updateObjects.begin(); _updateObjects.erase(_updateObjects.begin()); - // PLAYERBOT FIX: Graceful skip instead of ASSERT for race condition prevention - // Race condition in BaseEntity::RemoveFromWorld(): - // 1. m_inWorld = false (FIRST) - // 2. ClearUpdateMask(true) -> RemoveFromObjectUpdate() (SECOND) - // There's a window between these two operations where: - // - Object is still in _updateObjects (not yet removed) - // - But IsInWorld() returns false (already set to false) - // If Map::SendObjectUpdates runs during this window, the ASSERT would crash. - // Solution: Skip objects that are not in world - they don't need updates anyway. + // PLAYERBOT FIX: Multiple safety checks for race condition prevention + // + // Race condition scenarios: + // 1. BaseEntity::RemoveFromWorld() sets m_inWorld=false before ClearUpdateMask removes from set + // 2. Bot marked for removal (SetDestroyedObject) but still in _updateObjects due to re-add + // 3. Object freed but memory not yet overwritten - partial corruption + // + // Check 1: Skip objects not in world if (!obj->IsInWorld()) { - TC_LOG_DEBUG("maps", "Map::SendObjectUpdates: Skipping object not in world (race condition prevention)"); + TC_LOG_DEBUG("maps", "Map::SendObjectUpdates: Skipping object not in world"); + continue; + } + + // Check 2: Skip objects marked for destruction (prevents use-after-free) + // This catches objects that passed IsInWorld() but are being destroyed + if (obj->IsDestroyedObject()) + { + TC_LOG_DEBUG("maps", "Map::SendObjectUpdates: Skipping destroyed object"); continue; }