From aed293bba284fae2de9eea78d98bd10c1277f009 Mon Sep 17 00:00:00 2001 From: Mark Vejvoda Date: Wed, 28 Sep 2011 06:57:42 +0000 Subject: [PATCH] - fixed a bunch of memory leaks that will hopefully mean less overall memory requirements. --- source/glest_game/main/main.cpp | 11 ++++ source/glest_game/type_instances/faction.cpp | 47 +++++++++------- source/glest_game/type_instances/unit.cpp | 54 ++++++++++++++++++- source/glest_game/type_instances/unit.h | 24 ++++++++- source/glest_game/world/world.cpp | 12 +++++ .../include/platform/common/cache_manager.h | 5 +- source/shared_lib/include/util/util.h | 1 + source/shared_lib/include/xml/xml_parser.h | 2 + .../shared_lib/sources/graphics/particle.cpp | 3 ++ source/shared_lib/sources/util/util.cpp | 20 ++++--- source/shared_lib/sources/xml/xml_parser.cpp | 15 ++++-- 11 files changed, 161 insertions(+), 33 deletions(-) diff --git a/source/glest_game/main/main.cpp b/source/glest_game/main/main.cpp index d7fb9147..f45c1b80 100644 --- a/source/glest_game/main/main.cpp +++ b/source/glest_game/main/main.cpp @@ -150,6 +150,17 @@ static void cleanupProcessObjects() { if(SystemFlags::VERBOSE_MODE_ENABLED) printf("In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); SystemFlags::Close(); + XmlIo::getInstance().cleanup(); + + std::map &crcPlayerTextureCache = CacheManager::getCachedItem< std::map >(GameConstants::playerTextureCacheLookupKey); + //deleteMapValues(crcPlayerTextureCache.begin(),crcPlayerTextureCache.end()); + crcPlayerTextureCache.clear(); + + std::map &crcFactionPreviewTextureCache = CacheManager::getCachedItem< std::map >(GameConstants::factionPreviewTextureCacheLookupKey); + //deleteMapValues(crcFactionPreviewTextureCache.begin(),crcFactionPreviewTextureCache.end()); + crcFactionPreviewTextureCache.clear(); + + CacheManager::cleanupMutexes(); if(SystemFlags::VERBOSE_MODE_ENABLED) printf("In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); SystemFlags::SHUTDOWN_PROGRAM_MODE=true; diff --git a/source/glest_game/type_instances/faction.cpp b/source/glest_game/type_instances/faction.cpp index 473090b4..95b7ee35 100644 --- a/source/glest_game/type_instances/faction.cpp +++ b/source/glest_game/type_instances/faction.cpp @@ -172,6 +172,8 @@ void Faction::sortUnitsByCommandGroups() { //printf("====== Done sorting for faction # %d [%s] unitCount = %d\n",this->getIndex(),this->getType()->getName().c_str(),units.size()); + unsigned int originalUnitSize = units.size(); + std::vector unitIds; for(unsigned int i = 0; i < units.size(); ++i) { int unitId = units[i]->getId(); @@ -203,6 +205,7 @@ void Faction::sortUnitsByCommandGroups() { units.push_back(this->findUnit(unitId)); } + assert(originalUnitSize == units.size()); } // ===================================================== @@ -392,6 +395,12 @@ Faction::~Faction() { workerThread = NULL; } + MutexSafeWrapper safeMutex(unitsMutex,string(__FILE__) + "_" + intToStr(__LINE__)); + deleteValues(units.begin(), units.end()); + units.clear(); + + safeMutex.ReleaseLock(); + //delete texture; texture = NULL; if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); @@ -402,6 +411,26 @@ Faction::~Faction() { if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); } +void Faction::end() { + if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); + + if(workerThread != NULL) { + workerThread->signalQuit(); + if(workerThread->shutdownAndWait() == true) { + delete workerThread; + } + workerThread = NULL; + } + + MutexSafeWrapper safeMutex(unitsMutex,string(__FILE__) + "_" + intToStr(__LINE__)); + deleteValues(units.begin(), units.end()); + units.clear(); + + safeMutex.ReleaseLock(); + + if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); +} + Unit * Faction::getUnit(int i) const { Unit *result = units[i]; return result; @@ -474,24 +503,6 @@ void Faction::init( if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); } -void Faction::end() { - if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); - - if(workerThread != NULL) { - workerThread->signalQuit(); - if(workerThread->shutdownAndWait() == true) { - delete workerThread; - } - workerThread = NULL; - } - - MutexSafeWrapper safeMutex(unitsMutex,string(__FILE__) + "_" + intToStr(__LINE__)); - deleteValues(units.begin(), units.end()); - safeMutex.ReleaseLock(); - - if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); -} - // ================== get ================== const Resource *Faction::getResource(const ResourceType *rt) const{ diff --git a/source/glest_game/type_instances/unit.cpp b/source/glest_game/type_instances/unit.cpp index 0b8d591c..632f6558 100644 --- a/source/glest_game/type_instances/unit.cpp +++ b/source/glest_game/type_instances/unit.cpp @@ -40,7 +40,17 @@ namespace Glest{ namespace Game{ const int UnitPathBasic::maxBlockCount= GameConstants::updateFps / 2; +#ifdef LEAK_CHECK_UNITS +std::map UnitPathBasic::mapMemoryList; +std::map Unit::mapMemoryList; +std::map Unit::mapMemoryList2; +#endif + UnitPathBasic::UnitPathBasic() : UnitPathInterface() { +#ifdef LEAK_CHECK_UNITS + UnitPathBasic::mapMemoryList[this]=true; +#endif + this->blockCount = 0; this->pathQueue.clear(); this->lastPathCacheQueue.clear(); @@ -52,8 +62,26 @@ UnitPathBasic::~UnitPathBasic() { this->pathQueue.clear(); this->lastPathCacheQueue.clear(); this->map = NULL; + +#ifdef LEAK_CHECK_UNITS + UnitPathBasic::mapMemoryList.erase(this); +#endif } +#ifdef LEAK_CHECK_UNITS +void UnitPathBasic::dumpMemoryList() { + printf("===== START report of Unfreed UnitPathBasic pointers =====\n"); + for(std::map::iterator iterMap = UnitPathBasic::mapMemoryList.begin(); + iterMap != UnitPathBasic::mapMemoryList.end(); ++iterMap) { + printf("************** ==> Unfreed UnitPathBasic pointer [%p]\n",iterMap->first); + + if(Unit::mapMemoryList2.find(iterMap->first) != Unit::mapMemoryList2.end()) { + printf("Found owner unit id [%d]\n",Unit::mapMemoryList2[iterMap->first]); + } + } +} +#endif + bool UnitPathBasic::isEmpty() const { return pathQueue.empty(); } @@ -249,6 +277,10 @@ const int Unit::invalidId= -1; Game *Unit::game = NULL; Unit::Unit(int id, UnitPathInterface *unitpath, const Vec2i &pos, const UnitType *type, Faction *faction, Map *map, CardinalDir placeFacing):id(id) { +#ifdef LEAK_CHECK_UNITS + Unit::mapMemoryList[this]=true; +#endif + lastSynchDataString=""; modelFacing = CardinalDir::NORTH; lastStuckFrame = 0; @@ -263,6 +295,8 @@ Unit::Unit(int id, UnitPathInterface *unitpath, const Vec2i &pos, const UnitType rotationZ=.0f; rotationX=.0f; + this->unitPath = unitpath; + this->unitPath->setMap(map); RandomGen random; @@ -270,8 +304,6 @@ Unit::Unit(int id, UnitPathInterface *unitpath, const Vec2i &pos, const UnitType throw runtime_error("#2 Invalid path position = " + pos.getString()); } - this->unitPath = unitpath; - this->unitPath->setMap(map); this->pos=pos; this->type=type; this->faction=faction; @@ -402,6 +434,10 @@ Unit::~Unit() { delete currentAttackBoostOriginatorEffect.currentAppliedEffect; currentAttackBoostOriginatorEffect.currentAppliedEffect = NULL; +#ifdef LEAK_CHECK_UNITS + Unit::mapMemoryList2[this->unitPath] = this->getId(); +#endif + delete this->unitPath; this->unitPath = NULL; @@ -410,8 +446,22 @@ Unit::~Unit() { //MutexSafeWrapper safeMutex1(&mutexDeletedUnits,string(__FILE__) + "_" + intToStr(__LINE__)); //deletedUnits[this]=true; + +#ifdef LEAK_CHECK_UNITS + Unit::mapMemoryList.erase(this); +#endif } +#ifdef LEAK_CHECK_UNITS +void Unit::dumpMemoryList() { + printf("===== START report of Unfreed Unit pointers =====\n"); + for(std::map::iterator iterMap = Unit::mapMemoryList.begin(); + iterMap != Unit::mapMemoryList.end(); ++iterMap) { + printf("************** ==> Unfreed Unit pointer [%p]\n",iterMap->first); + } +} +#endif + //bool Unit::isUnitDeleted(void *unit) { // bool result = false; // MutexSafeWrapper safeMutex(&mutexDeletedUnits,string(__FILE__) + "_" + intToStr(__LINE__)); diff --git a/source/glest_game/type_instances/unit.h b/source/glest_game/type_instances/unit.h index 8b93fd89..157c4164 100644 --- a/source/glest_game/type_instances/unit.h +++ b/source/glest_game/type_instances/unit.h @@ -20,6 +20,8 @@ #include #include "leak_dumper.h" +//#define LEAK_CHECK_UNITS + namespace Glest { namespace Game { using Shared::Graphics::ParticleSystem; @@ -102,6 +104,8 @@ public: class UnitPathInterface { public: + UnitPathInterface() {} + virtual ~UnitPathInterface() {} virtual bool isBlocked() const = 0; virtual bool isEmpty() const = 0; @@ -128,6 +132,10 @@ private: static const int maxBlockCount; Map *map; +#ifdef LEAK_CHECK_UNITS + static std::map mapMemoryList; +#endif + private: int blockCount; vector pathQueue; @@ -136,6 +144,11 @@ private: public: UnitPathBasic(); virtual ~UnitPathBasic(); + +#ifdef LEAK_CHECK_UNITS + static void dumpMemoryList(); +#endif + virtual bool isBlocked() const; virtual bool isEmpty() const; virtual bool isStuck() const; @@ -175,7 +188,7 @@ private: Map *map; public: - UnitPath() : blockCount(0), map(NULL) {} /**< Construct path object */ + UnitPath() : UnitPathInterface(), blockCount(0), map(NULL) {} /**< Construct path object */ virtual bool isBlocked() const {return blockCount >= maxBlockCount;} /**< is this path blocked */ virtual bool isEmpty() const {return list::empty();} /**< is path empty */ virtual bool isStuck() const {return false; } @@ -262,12 +275,21 @@ private: typedef list Observers; typedef vector UnitParticleSystems; +#ifdef LEAK_CHECK_UNITS + static std::map mapMemoryList; +#endif + public: static const float speedDivider; static const int maxDeadCount; static const float highlightTime; static const int invalidId; +#ifdef LEAK_CHECK_UNITS + static std::map mapMemoryList2; + static void dumpMemoryList(); +#endif + private: const int id; int hp; diff --git a/source/glest_game/world/world.cpp b/source/glest_game/world/world.cpp index f2185e4a..24c330d9 100644 --- a/source/glest_game/world/world.cpp +++ b/source/glest_game/world/world.cpp @@ -102,6 +102,12 @@ World::~World() { } factions.clear(); +#ifdef LEAK_CHECK_UNITS + printf("%s::%s\n",__FILE__,__FUNCTION__); + Unit::dumpMemoryList(); + UnitPathBasic::dumpMemoryList(); +#endif + if(SystemFlags::getSystemSettingType(SystemFlags::debugSystem).enabled) SystemFlags::OutputDebug(SystemFlags::debugSystem,"In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); delete techTree; @@ -147,6 +153,12 @@ void World::end(){ } factions.clear(); +#ifdef LEAK_CHECK_UNITS + printf("%s::%s\n",__FILE__,__FUNCTION__); + Unit::dumpMemoryList(); + UnitPathBasic::dumpMemoryList(); +#endif + fogOfWarOverride = false; map.end(); diff --git a/source/shared_lib/include/platform/common/cache_manager.h b/source/shared_lib/include/platform/common/cache_manager.h index 94ed5e01..f8052de9 100644 --- a/source/shared_lib/include/platform/common/cache_manager.h +++ b/source/shared_lib/include/platform/common/cache_manager.h @@ -93,7 +93,7 @@ protected: public: CacheManager() { } - ~CacheManager() { + static void cleanupMutexes() { for(std::map::iterator iterMap = itemCacheMutexList.begin(); iterMap != itemCacheMutexList.end(); iterMap++) { delete iterMap->second; @@ -101,6 +101,9 @@ public: } itemCacheMutexList.clear(); } + ~CacheManager() { + CacheManager::cleanupMutexes(); + } template static void setCachedItem(string cacheKey, const T value) { diff --git a/source/shared_lib/include/util/util.h b/source/shared_lib/include/util/util.h index b9af1079..a18f386f 100644 --- a/source/shared_lib/include/util/util.h +++ b/source/shared_lib/include/util/util.h @@ -133,6 +133,7 @@ public: static CURL *initHTTP(); static void cleanupHTTP(CURL **handle,bool globalCleanup=false); + static void globalCleanupHTTP(); static bool getThreadedLoggerRunning(); static std::size_t getLogEntryBufferCount(); diff --git a/source/shared_lib/include/xml/xml_parser.h b/source/shared_lib/include/xml/xml_parser.h index 5e327786..98f724e6 100644 --- a/source/shared_lib/include/xml/xml_parser.h +++ b/source/shared_lib/include/xml/xml_parser.h @@ -53,6 +53,8 @@ private: public: static XmlIo &getInstance(); ~XmlIo(); + void cleanup(); + XmlNode *load(const string &path, std::map mapTagReplacementValues); void save(const string &path, const XmlNode *node); }; diff --git a/source/shared_lib/sources/graphics/particle.cpp b/source/shared_lib/sources/graphics/particle.cpp index fc27112a..6b59aeff 100644 --- a/source/shared_lib/sources/graphics/particle.cpp +++ b/source/shared_lib/sources/graphics/particle.cpp @@ -83,6 +83,9 @@ ParticleSystem::~ParticleSystem(){ //delete [] particles; particles.clear(); + + delete particleObserver; + particleObserver = NULL; } // =============== VIRTUAL ====================== diff --git a/source/shared_lib/sources/util/util.cpp b/source/shared_lib/sources/util/util.cpp index 7cbbb83c..369d10e5 100644 --- a/source/shared_lib/sources/util/util.cpp +++ b/source/shared_lib/sources/util/util.cpp @@ -271,17 +271,21 @@ SystemFlags::SystemFlags() { } +void SystemFlags::globalCleanupHTTP() { + if(SystemFlags::curl_global_init_called == true) { + SystemFlags::curl_global_init_called = false; + curl_global_cleanup(); + //printf("In [%s::%s Line %d] curl_global_cleanup called\n",__FILE__,__FUNCTION__,__LINE__); + } +} + void SystemFlags::cleanupHTTP(CURL **handle, bool globalCleanup) { if(handle != NULL && *handle != NULL) { curl_easy_cleanup(*handle); *handle = NULL; if(globalCleanup == true) { - if(SystemFlags::curl_global_init_called == true) { - SystemFlags::curl_global_init_called = false; - curl_global_cleanup(); - //printf("In [%s::%s Line %d] curl_global_cleanup called\n",__FILE__,__FUNCTION__,__LINE__); - } + SystemFlags::globalCleanupHTTP(); } } } @@ -297,9 +301,7 @@ SystemFlags::~SystemFlags() { curl_handle = NULL; } if(SystemFlags::curl_global_init_called == true) { - SystemFlags::curl_global_init_called = false; - curl_global_cleanup(); - //printf("In [%s::%s Line %d] curl_global_cleanup called\n",__FILE__,__FUNCTION__,__LINE__); + SystemFlags::globalCleanupHTTP(); } if(SystemFlags::VERBOSE_MODE_ENABLED) printf("In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); @@ -373,6 +375,8 @@ void SystemFlags::Close() { } } + SystemFlags::globalCleanupHTTP(); + if(SystemFlags::VERBOSE_MODE_ENABLED) printf("In [%s::%s Line: %d]\n",__FILE__,__FUNCTION__,__LINE__); } diff --git a/source/shared_lib/sources/xml/xml_parser.cpp b/source/shared_lib/sources/xml/xml_parser.cpp index 028d9f09..e3e2246f 100644 --- a/source/shared_lib/sources/xml/xml_parser.cpp +++ b/source/shared_lib/sources/xml/xml_parser.cpp @@ -65,6 +65,8 @@ bool XmlIo::initialized= false; XmlIo::XmlIo() { try{ XMLPlatformUtils::Initialize(); + + XmlIo::initialized= true; } catch(const XMLException &e){ SystemFlags::OutputDebug(SystemFlags::debugError,"In [%s::%s Line: %d] Error initializing XML system, msg %s\n",__FILE__,__FUNCTION__,__LINE__,e.getMessage()); @@ -83,13 +85,20 @@ XmlIo::XmlIo() { } } -XmlIo &XmlIo::getInstance(){ +XmlIo &XmlIo::getInstance() { static XmlIo XmlIo; return XmlIo; } -XmlIo::~XmlIo(){ - XMLPlatformUtils::Terminate(); +void XmlIo::cleanup() { + if(XmlIo::initialized == true) { + XmlIo::initialized= false; + XMLPlatformUtils::Terminate(); + } +} + +XmlIo::~XmlIo() { + cleanup(); } XmlNode *XmlIo::load(const string &path, std::map mapTagReplacementValues) {