gistfile1.diff

commit 8f235b728ee99be69656ec7674fe160dab64feaf
Author: faramir118 <[email protected]>
Date:   Sat Feb 4 13:59:54 2012 -0600

    guard against concurrent read/write access to navMesh
    
    Need to add -DMMAP_CONCURRENCY=1 to cmake
    visual studio support coming later

diff --git a/src/game/CMakeLists.txt b/src/game/CMakeLists.txt
index 234c20a..7c70117 100644
--- a/src/game/CMakeLists.txt
+++ b/src/game/CMakeLists.txt
@@ -74,6 +74,10 @@ source_group("Movement"
     movement/util.cpp
 )
 
+if(MMAP_CONCURRENCY)
+  ADD_DEFINITIONS(-DMMAP_CONCURRENCY)
+endif()
+
 if(PCH)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
 endif()
diff --git a/src/game/MoveMap.cpp b/src/game/MoveMap.cpp
index 6323125..ee3f1e6 100644
--- a/src/game/MoveMap.cpp
+++ b/src/game/MoveMap.cpp
@@ -205,9 +205,13 @@ namespace MMAP
         dtMeshHeader* header = (dtMeshHeader*)data;
         dtTileRef tileRef = 0;
 
-        // memory allocated for data is now managed by detour, and will be deallocated when the tile is removed
-        if(DT_SUCCESS == mmap->navMesh->addTile(data, fileHeader.size, DT_TILE_FREE_DATA, 0, &tileRef))
+        LOCK_WRITE(mmap->navMeshLock);
+        dtStatus status = mmap->navMesh->addTile(data, fileHeader.size, DT_TILE_FREE_DATA, 0, &tileRef);
+        LOCK_RELEASE(mmap->navMeshLock);
+
+        if(DT_SUCCESS == status)
         {
+            // memory allocated for data is now managed by detour, and will be deallocated when the tile is removed
             mmap->mmapLoadedTiles.insert(std::pair<uint32, dtTileRef>(packedGridPos, tileRef));
             ++loadedTiles;
             sLog.outDetail("MMAP:loadMap: Loaded mmtile %03i[%02i,%02i] into %03i[%02i,%02i]", mapId, x, y, mapId, header->x, header->y);
@@ -247,7 +251,12 @@ namespace MMAP
         dtTileRef tileRef = mmap->mmapLoadedTiles[packedGridPos];
 
         // unload, and mark as non loaded
-        if(DT_SUCCESS != mmap->navMesh->removeTile(tileRef, NULL, NULL))
+
+        LOCK_WRITE(mmap->navMeshLock);
+        dtStatus status = mmap->navMesh->removeTile(tileRef, NULL, NULL);
+        LOCK_RELEASE(mmap->navMeshLock);
+
+        if(DT_SUCCESS != status)
         {
             // this is technically a memory leak
             // if the grid is later reloaded, dtNavMesh::addTile will return error but no extra memory is used
@@ -281,7 +290,12 @@ namespace MMAP
         {
             uint32 x = (i->first >> 16);
             uint32 y = (i->first & 0x0000FFFF);
-            if(DT_SUCCESS != mmap->navMesh->removeTile(i->second, NULL, NULL))
+
+            LOCK_WRITE(mmap->navMeshLock);
+            dtStatus status = mmap->navMesh->removeTile(i->second, NULL, NULL);
+            LOCK_RELEASE(mmap->navMeshLock);
+
+            if(DT_SUCCESS != status)
                 sLog.outError("MMAP:unloadMap: Could not unload %03u%02i%02i.mmtile from navmesh", mapId, x, y);
             else
             {
@@ -355,4 +369,14 @@ namespace MMAP
 
         return mmap->navMeshQueries[instanceId];
     }
+
+#ifdef MMAP_CONCURRENCY
+    ACE_RW_Mutex* MMapManager::GetNavMeshLock(uint32 mapId)
+    {
+	    if (loadedMMaps.find(mapId) == loadedMMaps.end())
+	    	return NULL;
+
+        return &loadedMMaps[mapId]->navMeshLock;
+    }
+#endif
 }
diff --git a/src/game/MoveMap.h b/src/game/MoveMap.h
index 58cac34..0425dd9 100644
--- a/src/game/MoveMap.h
+++ b/src/game/MoveMap.h
@@ -20,11 +20,14 @@
 #define _MOVE_MAP_H
 
 #include "Utilities/UnorderedMapSet.h"
+#include "MoveMapConcurrency.h"
 
 #include "../../dep/recastnavigation/Detour/Include/DetourAlloc.h"
 #include "../../dep/recastnavigation/Detour/Include/DetourNavMesh.h"
 #include "../../dep/recastnavigation/Detour/Include/DetourNavMeshQuery.h"
 
+class PathFinder;
+
 //  memory management
 inline void* dtCustomAlloc(int size, dtAllocHint /*hint*/)
 {
@@ -60,6 +63,10 @@ namespace MMAP
         // we have to use single dtNavMeshQuery for every instance, since those are not thread safe
         NavMeshQuerySet navMeshQueries;     // instanceId to query
         MMapTileSet mmapLoadedTiles;        // maps [map grid coords] to [dtTile]
+
+#ifdef MMAP_CONCURRENCY
+        ACE_RW_Mutex navMeshLock;
+#endif
     };
 
 
@@ -90,6 +97,12 @@ namespace MMAP
 
             MMapDataSet loadedMMaps;
             uint32 loadedTiles;
+
+#ifdef MMAP_CONCURRENCY
+        private:
+            friend class ::PathFinder;
+            ACE_RW_Mutex* GetNavMeshLock(uint32 mapId);
+#endif
     };
 
     // static class
diff --git a/src/game/MoveMapConcurrency.h b/src/game/MoveMapConcurrency.h
new file mode 100644
index 0000000..34b1b48
--- /dev/null
+++ b/src/game/MoveMapConcurrency.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2005-2011 MaNGOS <http://getmangos.com/>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef MOVEMAPCONCURRENCY_H
+#define MOVEMAPCONCURRENCY_H
+
+#ifdef MMAP_CONCURRENCY
+    #include <ace/RW_Mutex.h>
+
+    #define LOCK_WRITE(lock) lock.acquire_write()
+    #define LOCK_READ(lock) lock.acquire_read()
+    #define LOCK_RELEASE(lock) lock.release()
+
+#else
+
+    // mmaps are not threadsafe, but no locking overhead
+    #define LOCK_WRITE(lock)
+    #define LOCK_READ(lock)
+    #define LOCK_RELEASE(lock)
+
+#endif  // MMAP_CONCURRENCY
+
+#endif  // MOVEMAPCONCURRENCY_H
diff --git a/src/game/PathFinder.cpp b/src/game/PathFinder.cpp
index 0735674..448c9c2 100644
--- a/src/game/PathFinder.cpp
+++ b/src/game/PathFinder.cpp
@@ -38,6 +38,9 @@ PathFinder::PathFinder(const Unit* owner) :
         MMAP::MMapManager* mmap = MMAP::MMapFactory::createOrGetMMapManager();
         m_navMesh = mmap->GetNavMesh(mapId);
         m_navMeshQuery = mmap->GetNavMeshQuery(mapId, m_sourceUnit->GetInstanceId());
+#ifdef MMAP_CONCURRENCY
+        m_navMeshLock = mmap->GetNavMeshLock(mapId);
+#endif
     }
 
     createFilter();
@@ -90,7 +93,9 @@ bool PathFinder::calculate(float destX, float destY, float destZ, bool forceDest
     else
     {
         // target moved, so we need to update the poly path
+        LOCK_READ((*m_navMeshLock));
         BuildPolyPath(start, dest);
+        LOCK_RELEASE((*m_navMeshLock));
         return true;
     }
 }
diff --git a/src/game/PathFinder.h b/src/game/PathFinder.h
index e2c38a6..d4a67ee 100644
--- a/src/game/PathFinder.h
+++ b/src/game/PathFinder.h
@@ -30,6 +30,10 @@ using Movement::PointsArray;
 
 class Unit;
 
+#ifdef MMAP_CONCURRENCY
+class ACE_RW_Mutex;
+#endif
+
 // 74*4.0f=296y  number_of_points*interval = max_path_len
 // this is way more than actual evade range
 // I think we can safely cut those down even more
@@ -93,6 +97,9 @@ class PathFinder
         const Unit* const       m_sourceUnit;       // the unit that is moving
         const dtNavMesh*        m_navMesh;          // the nav mesh
         const dtNavMeshQuery*   m_navMeshQuery;     // the nav mesh query used to find the path
+#ifdef MMAP_CONCURRENCY
+        ACE_RW_Mutex*			m_navMeshLock;
+#endif
 
         dtQueryFilter m_filter;                     // use single filter for all movements, update it when needed