From dbc21ed90396326c5b52d5847f37697e4f6b160b Mon Sep 17 00:00:00 2001 From: H0zen Date: Fri, 4 Mar 2016 14:30:48 +0200 Subject: [PATCH] More robust checks on mutex acquire. - When using ACE_xxx_Guard, the caller must ensure the internal lock is really acquired before entering the critical section (see http://www.dre.vanderbilt.edu/Doxygen/6.0.1/html/libace-doc/a00186.html#_details - Warnings paragraph) --- src/game/Object/ObjectAccessor.cpp | 110 ++++++++++++++++------------- src/game/Object/ObjectAccessor.h | 22 +++--- 2 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/game/Object/ObjectAccessor.cpp b/src/game/Object/ObjectAccessor.cpp index 92dc1837..6ddbb07e 100644 --- a/src/game/Object/ObjectAccessor.cpp +++ b/src/game/Object/ObjectAccessor.cpp @@ -93,12 +93,14 @@ Player* ObjectAccessor::FindPlayer(ObjectGuid guid, bool inWorld /*= true*/) Player* ObjectAccessor::FindPlayerByName(const char* name) { - HashMapHolder::ReadGuard g(HashMapHolder::GetLock()); - HashMapHolder::MapType& m = sObjectAccessor.GetPlayers(); - for (HashMapHolder::MapType::iterator iter = m.begin(); iter != m.end(); ++iter) - if (iter->second->IsInWorld() && (::strcmp(name, iter->second->GetName()) == 0)) - { return iter->second; } - + HashMapHolder::ReadGuard g(HashMapHolder::GetLock(), true); + if (g.locked()) + { + HashMapHolder::MapType& m = sObjectAccessor.GetPlayers(); + for (HashMapHolder::MapType::iterator iter = m.begin(); iter != m.end(); ++iter) + if (iter->second->IsInWorld() && (::strcmp(name, iter->second->GetName()) == 0)) + { return iter->second; } + } return NULL; } @@ -129,15 +131,16 @@ void ObjectAccessor::KickPlayer(ObjectGuid guid) Corpse* ObjectAccessor::GetCorpseForPlayerGUID(ObjectGuid guid) { - Guard guard(i_corpseGuard); - - Player2CorpsesMapType::iterator iter = i_player2corpse.find(guid); - if (iter == i_player2corpse.end()) - { return NULL; } - - MANGOS_ASSERT(iter->second->GetType() != CORPSE_BONES); - - return iter->second; + ACE_Guard guard(i_corpseGuard, true); + if (guard.locked()) + { + Player2CorpsesMapType::iterator iter = i_player2corpse.find(guid); + if (iter == i_player2corpse.end()) + { return NULL; } + MANGOS_ASSERT(iter->second->GetType() != CORPSE_BONES); + return iter->second; + } + return NULL; } void @@ -145,19 +148,22 @@ ObjectAccessor::RemoveCorpse(Corpse* corpse) { MANGOS_ASSERT(corpse && corpse->GetType() != CORPSE_BONES); - Guard guard(i_corpseGuard); - Player2CorpsesMapType::iterator iter = i_player2corpse.find(corpse->GetOwnerGuid()); - if (iter == i_player2corpse.end()) - { return; } + ACE_Guard guard(i_corpseGuard, true); + if (guard.locked()) + { + Player2CorpsesMapType::iterator iter = i_player2corpse.find(corpse->GetOwnerGuid()); + if (iter == i_player2corpse.end()) + { return; } - // build mapid*cellid -> guid_set map - CellPair cell_pair = MaNGOS::ComputeCellPair(corpse->GetPositionX(), corpse->GetPositionY()); - uint32 cell_id = (cell_pair.y_coord * TOTAL_NUMBER_OF_CELLS_PER_MAP) + cell_pair.x_coord; + // build mapid*cellid -> guid_set map + CellPair cell_pair = MaNGOS::ComputeCellPair(corpse->GetPositionX(), corpse->GetPositionY()); + uint32 cell_id = (cell_pair.y_coord * TOTAL_NUMBER_OF_CELLS_PER_MAP) + cell_pair.x_coord; - sObjectMgr.DeleteCorpseCellData(corpse->GetMapId(), cell_id, corpse->GetOwnerGuid().GetCounter()); - corpse->RemoveFromWorld(); + sObjectMgr.DeleteCorpseCellData(corpse->GetMapId(), cell_id, corpse->GetOwnerGuid().GetCounter()); + corpse->RemoveFromWorld(); - i_player2corpse.erase(iter); + i_player2corpse.erase(iter); + } } void @@ -165,37 +171,43 @@ ObjectAccessor::AddCorpse(Corpse* corpse) { MANGOS_ASSERT(corpse && corpse->GetType() != CORPSE_BONES); - Guard guard(i_corpseGuard); - MANGOS_ASSERT(i_player2corpse.find(corpse->GetOwnerGuid()) == i_player2corpse.end()); - i_player2corpse[corpse->GetOwnerGuid()] = corpse; + ACE_Guard guard(i_corpseGuard, true); + if (guard.locked()) + { + MANGOS_ASSERT(i_player2corpse.find(corpse->GetOwnerGuid()) == i_player2corpse.end()); + i_player2corpse[corpse->GetOwnerGuid()] = corpse; - // build mapid*cellid -> guid_set map - CellPair cell_pair = MaNGOS::ComputeCellPair(corpse->GetPositionX(), corpse->GetPositionY()); - uint32 cell_id = (cell_pair.y_coord * TOTAL_NUMBER_OF_CELLS_PER_MAP) + cell_pair.x_coord; + // build mapid*cellid -> guid_set map + CellPair cell_pair = MaNGOS::ComputeCellPair(corpse->GetPositionX(), corpse->GetPositionY()); + uint32 cell_id = (cell_pair.y_coord * TOTAL_NUMBER_OF_CELLS_PER_MAP) + cell_pair.x_coord; - sObjectMgr.AddCorpseCellData(corpse->GetMapId(), cell_id, corpse->GetOwnerGuid().GetCounter(), corpse->GetInstanceId()); + sObjectMgr.AddCorpseCellData(corpse->GetMapId(), cell_id, corpse->GetOwnerGuid().GetCounter(), corpse->GetInstanceId()); + } } void ObjectAccessor::AddCorpsesToGrid(GridPair const& gridpair, GridType& grid, Map* map) { - Guard guard(i_corpseGuard); - for (Player2CorpsesMapType::iterator iter = i_player2corpse.begin(); iter != i_player2corpse.end(); ++iter) - if (iter->second->GetGrid() == gridpair) - { - // verify, if the corpse in our instance (add only corpses which are) - if (map->Instanceable()) - { - if (iter->second->GetInstanceId() == map->GetInstanceId()) - { - grid.AddWorldObject(iter->second); - } - } - else - { - grid.AddWorldObject(iter->second); - } - } + ACE_Guard guard(i_corpseGuard, true); + if (guard.locked()) + { + for (Player2CorpsesMapType::iterator iter = i_player2corpse.begin(); iter != i_player2corpse.end(); ++iter) + if (iter->second->GetGrid() == gridpair) + { + // verify, if the corpse in our instance (add only corpses which are) + if (map->Instanceable()) + { + if (iter->second->GetInstanceId() == map->GetInstanceId()) + { + grid.AddWorldObject(iter->second); + } + } + else + { + grid.AddWorldObject(iter->second); + } + } + } } Corpse* diff --git a/src/game/Object/ObjectAccessor.h b/src/game/Object/ObjectAccessor.h index a1d53bd8..56a529af 100644 --- a/src/game/Object/ObjectAccessor.h +++ b/src/game/Object/ObjectAccessor.h @@ -59,21 +59,28 @@ class HashMapHolder static void Insert(T* o) { - WriteGuard guard(i_lock); - m_objectMap[o->GetObjectGuid()] = o; + WriteGuard guard(i_lock, true); + if (guard.locked()) + m_objectMap[o->GetObjectGuid()] = o; } static void Remove(T* o) { - WriteGuard guard(i_lock); - m_objectMap.erase(o->GetObjectGuid()); + WriteGuard guard(i_lock, true); + if (guard.locked()) + m_objectMap.erase(o->GetObjectGuid()); } static T* Find(ObjectGuid guid) { - ReadGuard guard(i_lock); - typename MapType::iterator itr = m_objectMap.find(guid); - return (itr != m_objectMap.end()) ? itr->second : NULL; + ReadGuard guard(i_lock, true); + if (guard.locked()) + { + typename MapType::iterator itr = m_objectMap.find(guid); + return (itr != m_objectMap.end()) ? itr->second : NULL; + } + else + return NULL; } static MapType& GetContainer() { return m_objectMap; } @@ -137,7 +144,6 @@ class ObjectAccessor : public MaNGOS::Singleton Guard; LockType i_playerGuard; LockType i_corpseGuard;