Here is my idea about why in multithreading environment crashes at InstanceSaveMgr.h are experienced:
[FONT=Verdana]sInstanceSaveMgr[/FONT] as protected by [FONT=Verdana]ACE_Thread_Mutex[/FONT] is (or meant to be) thread-safe. It maintains
InstanceSaveHashMap m_instanceSaveById;internally. But the thread-safety is broken by its public method
InstanceSave *GetInstanceSave(uint32 InstanceId);Here the thread safety issue is postponed to [FONT=Verdana]InstanceSave[/FONT] class which in turn is not safe. Not in theory only, but in practice we see several call of its methods from Map.cpp. May an InstanceSave with the same id be requested from several maps? I don’t know, but the structure of the code does not explicitly prevents this. Then crashes in subs from [FONT=Verdana]InstanceSaveMgr.h[/FONT] handling [FONT=Verdana]m_playerList[/FONT] and [FONT=Verdana]m_groupList[/FONT] are understandable.
As far as my practical experience with multithreading and ACE is zero, I ask someone more experienced to consider the arguments above and, eventually, to fix the issue.
Some additional thoughts:
[SPOILER]I saw crashes in AddGroup() and in RemovePlayer(), the latter experienced by our local team. It seems to me that the thread safety should be implemented in the [FONT=Verdana]InstanceSave[/FONT] class directly, and a blocking mutex scheme should be used. Another way could be hiding [FONT=Verdana]GetInstanceSave()[/FONT] method and implement the needed functionality in the thread-safe [FONT=Verdana]InstanceSaveManager[/FONT] class. However, I still cannot explain why the thread safety can be broken by the destructor [FONT=Verdana]~WorldSession()[/FONT]. As far as I know, sessions are updated by a single (“main”) thread, in contrast to the maps.
By the way, in InstanceSaveMgr.h:119
friend class ACE_Singleton<InstanceSaveManager, ACE_Null_Mutex>;ACE_Null_Mutex is surely a typing error, and must be read as ACE_Thread_Mutex, but fortunately it changes nothing.
[/SPOILER]