Core/Instances: Fix mutex released after being deleted

Modify how InstanceSave is deleted so the local mutex can be released before deleting the class itself.

Valgrind log:
 Invalid read of size 4
  at 0x662662B: __pthread_mutex_unlock_usercnt (pthread_mutex_unlock.c:52)
  by 0x55D3C55: ACE_OS::mutex_unlock(pthread_mutex_t*) (OS_NS_Thread.cpp:2335)
  by 0xB20057: Player::CleanupsBeforeDelete(bool) (OS_NS_Thread.inl:3519)
  by 0xD0E2FA: WorldSession::LogoutPlayer(bool) (WorldSession.cpp:527)
  by 0xC66D34: WorldSession::HandleLogoutRequestOpcode(WorldPacket&) (MiscHandler.cpp:403)
  by 0xD0EA82: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
  by 0xD9AD66: World::UpdateSessions(unsigned int) (World.cpp:2615)
  by 0xD9BEC4: World::Update(unsigned int) (World.cpp:1978)
  by 0xA035E5: WorldRunnable::run() (WorldRunnable.cpp:60)
  by 0xEC8D39: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:183)
  by 0x55D7555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
  by 0x6622B4F: start_thread (pthread_create.c:304)
Address 0x1884bb08 is 56 bytes inside a block of size 104 free'd
  at 0x4C279DC: operator delete(void*) (vg_replace_malloc.c:457)
  by 0xC9D533: InstanceSaveManager::RemoveInstanceSave(unsigned int) (InstanceSaveMgr.cpp:159)
  by 0xC9E826: InstanceSave::UnloadIfEmpty() (InstanceSaveMgr.cpp:238)
  by 0xB2003E: Player::CleanupsBeforeDelete(bool) (InstanceSaveMgr.h:84)
  by 0xD0E2FA: WorldSession::LogoutPlayer(bool) (WorldSession.cpp:527)
  by 0xC66D34: WorldSession::HandleLogoutRequestOpcode(WorldPacket&) (MiscHandler.cpp:403)
  by 0xD0EA82: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
  by 0xD9AD66: World::UpdateSessions(unsigned int) (World.cpp:2615)
  by 0xD9BEC4: World::Update(unsigned int) (World.cpp:1978)
  by 0xA035E5: WorldRunnable::run() (WorldRunnable.cpp:60)
  by 0xEC8D39: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:183)
  by 0x55D7555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)

2 files changed, 30 insertions, 4 deletions

diff --git a/src/server/game/Instances/InstanceSaveMgr.cpp b/src/server/game/Instances/InstanceSaveMgr.cpp
index a422fd8531c..6370eda6568 100644
--- a/src/server/game/Instances/InstanceSaveMgr.cpp
+++ b/src/server/game/Instances/InstanceSaveMgr.cpp
@@ -156,14 +156,14 @@ void InstanceSaveManager::RemoveInstanceSave(uint32 InstanceId)
             CharacterDatabase.Execute(stmt);
         }
 
-        delete itr->second;
+        itr->second->SetToDelete(true);
         m_instanceSaveById.erase(itr);
     }
 }
 
 InstanceSave::InstanceSave(uint16 MapId, uint32 InstanceId, Difficulty difficulty, time_t resetTime, bool canReset)
 : m_resetTime(resetTime), m_instanceid(InstanceId), m_mapid(MapId),
-  m_difficulty(difficulty), m_canReset(canReset)
+  m_difficulty(difficulty), m_canReset(canReset), m_toDelete(false)
 {
 }
 
diff --git a/src/server/game/Instances/InstanceSaveMgr.h b/src/server/game/Instances/InstanceSaveMgr.h
index c5dcaf32463..ef55005d0d6 100644
--- a/src/server/game/Instances/InstanceSaveMgr.h
+++ b/src/server/game/Instances/InstanceSaveMgr.h
@@ -81,10 +81,29 @@ class InstanceSave
         /* online players bound to the instance (perm/solo)
            does not include the members of the group unless they have permanent saves */
         void AddPlayer(Player* player) { TRINITY_GUARD(ACE_Thread_Mutex, _lock); m_playerList.push_back(player); }
-        bool RemovePlayer(Player* player) { TRINITY_GUARD(ACE_Thread_Mutex, _lock); m_playerList.remove(player); return UnloadIfEmpty(); }
+        bool RemovePlayer(Player* player)
+        { 
+            _lock.acquire();
+            m_playerList.remove(player);
+            bool isStillValid = UnloadIfEmpty();
+            _lock.release();
+
+            //delete here if needed, after releasing the lock
+            if(m_toDelete)
+                delete this;
+
+            return isStillValid;
+        }
         /* all groups bound to the instance */
         void AddGroup(Group* group) { m_groupList.push_back(group); }
-        bool RemoveGroup(Group* group) { m_groupList.remove(group); return UnloadIfEmpty(); }
+        bool RemoveGroup(Group* group)
+        { 
+            m_groupList.remove(group);
+            bool isStillValid = UnloadIfEmpty();
+            if(m_toDelete)
+                delete this;
+            return isStillValid;
+        }
 
         /* instances cannot be reset (except at the global reset time)
            if there are players permanently bound to it
@@ -96,6 +115,12 @@ class InstanceSave
            but that would depend on a lot of things that can easily change in future */
         Difficulty GetDifficulty() const { return m_difficulty; }
 
+        /* used to flag the InstanceSave as to be deleted, so the caller can delete it */
+        void SetToDelete(bool toDelete)
+        {
+            m_toDelete = toDelete;
+        }
+
         typedef std::list<Player*> PlayerListType;
         typedef std::list<Group*> GroupListType;
     private:
@@ -110,6 +135,7 @@ class InstanceSave
         uint32 m_mapid;
         Difficulty m_difficulty;
         bool m_canReset;
+        bool m_toDelete;
 
         ACE_Thread_Mutex _lock;
 };