Browse Source

Fix most of memleaks discovered by valgrind

Ivan Savenko 2 years ago
parent
commit
20ef3a69e7

+ 1 - 1
AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -90,7 +90,7 @@ void AINodeStorage::initialize(const PathfinderOptions & options, const CGameSta
 
 	//TODO: fix this code duplication with NodeStorage::initialize, problem is to keep `resetTile` inline
 	const PlayerColor fowPlayer = ai->playerID;
-	const auto fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(fowPlayer)->fogOfWarMap;
+	const auto & fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(fowPlayer)->fogOfWarMap;
 	const int3 sizes = gs->getMapSize();
 
 	//Each thread gets different x, but an array of y located next to each other in memory

+ 5 - 3
client/CMT.cpp

@@ -346,7 +346,6 @@ int main(int argc, char * argv[])
 	session["autoSkip"].Bool()  = vm.count("autoSkip");
 	session["oneGoodAI"].Bool() = vm.count("oneGoodAI");
 	session["aiSolo"].Bool() = false;
-	std::shared_ptr<CMainMenu> mmenu;
 	
 	if(vm.count("testmap"))
 	{
@@ -362,7 +361,7 @@ int main(int argc, char * argv[])
 	}
 	else
 	{
-		mmenu = CMainMenu::create();
+		auto mmenu = CMainMenu::create();
 		GH.curInt = mmenu.get();
 	}
 	
@@ -399,7 +398,7 @@ int main(int argc, char * argv[])
 		//start lobby immediately
 		names.push_back(session["username"].String());
 		ESelectionScreen sscreen = session["gamemode"].Integer() == 0 ? ESelectionScreen::newGame : ESelectionScreen::loadGame;
-		mmenu->openLobby(sscreen, session["host"].Bool(), &names, ELoadMode::MULTI);
+		CMM->openLobby(sscreen, session["host"].Bool(), &names, ELoadMode::MULTI);
 	}
 	
 	// Restore remote session - start game immediately
@@ -472,6 +471,9 @@ static void quitApplication()
 			CCS->musich->release();
 			CCS->soundh->release();
 
+			delete CCS->consoleh;
+			delete CCS->curh;
+
 			vstd::clear_pointer(CCS);
 		}
 		CMessage::dispose();

+ 2 - 0
client/eventsSDL/InputHandler.cpp

@@ -316,6 +316,8 @@ void InputHandler::handleUserEvent(const SDL_UserEvent & current)
 	auto heapFunctor = static_cast<std::function<void()>*>(current.data1);
 
 	(*heapFunctor)();
+
+	delete heapFunctor;
 }
 
 const Point & InputHandler::getCursorPosition() const

+ 5 - 5
lib/CGameInfoCallback.cpp

@@ -726,11 +726,11 @@ CGameInfoCallback::CGameInfoCallback(CGameState * GS):
 {
 }
 
-std::shared_ptr<const boost::multi_array<ui8, 3>> CPlayerSpecificInfoCallback::getVisibilityMap() const
-{
-	//boost::shared_lock<boost::shared_mutex> lock(*gs->mx);
-	return gs->getPlayerTeam(*getPlayerID())->fogOfWarMap;
-}
+//std::shared_ptr<const boost::multi_array<ui8, 3>> CPlayerSpecificInfoCallback::getVisibilityMap() const
+//{
+//	//boost::shared_lock<boost::shared_mutex> lock(*gs->mx);
+//	return gs->getPlayerTeam(*getPlayerID())->fogOfWarMap;
+//}
 
 int CPlayerSpecificInfoCallback::howManyTowns() const
 {

+ 1 - 1
lib/CGameInfoCallback.h

@@ -247,7 +247,7 @@ public:
 
 	virtual int getResourceAmount(GameResID type) const;
 	virtual TResources getResourceAmount() const;
-	virtual std::shared_ptr<const boost::multi_array<ui8, 3>> getVisibilityMap() const; //returns visibility map
+	//virtual std::shared_ptr<const boost::multi_array<ui8, 3>> getVisibilityMap() const; //returns visibility map
 	//virtual const PlayerSettings * getPlayerSettings(PlayerColor color) const;
 };
 

+ 1 - 1
lib/CPlayerState.h

@@ -98,7 +98,7 @@ public:
 	TeamID id; //position in gameState::teams
 	std::set<PlayerColor> players; // members of this team
 	//TODO: boost::array, bool if possible
-	std::shared_ptr<boost::multi_array<ui8, 3>> fogOfWarMap; //[z][x][y] true - visible, false - hidden
+	std::unique_ptr<boost::multi_array<ui8, 3>> fogOfWarMap; //[z][x][y] true - visible, false - hidden
 
 	TeamState();
 	TeamState(TeamState && other) noexcept;

+ 0 - 2
lib/ObstacleHandler.cpp

@@ -106,8 +106,6 @@ ObstacleInfo * ObstacleHandler::loadFromJson(const std::string & scope, const Js
 	info->isAbsoluteObstacle = json["absolute"].Bool();
 	info->isForegroundObstacle = json["foreground"].Bool();
 
-	objects.emplace_back(info);
-
 	return info;
 }
 

+ 10 - 0
lib/VCMI_Lib.cpp

@@ -259,6 +259,11 @@ void LibClasses::clear()
 	delete battlefieldsHandler;
 	delete generaltexth;
 	delete identifiersHandler;
+	delete obstacleHandler;
+	delete terrainTypeHandler;
+	delete riverTypeHandler;
+	delete roadTypeHandler;
+	delete settingsHandler;
 	makeNull();
 }
 
@@ -282,6 +287,11 @@ void LibClasses::makeNull()
 #endif
 	battlefieldsHandler = nullptr;
 	identifiersHandler = nullptr;
+	obstacleHandler = nullptr;
+	terrainTypeHandler = nullptr;
+	riverTypeHandler = nullptr;
+	roadTypeHandler = nullptr;
+	settingsHandler = nullptr;
 }
 
 LibClasses::LibClasses()

+ 2 - 2
lib/gameState/CGameState.cpp

@@ -700,7 +700,7 @@ void CGameState::initFogOfWar()
 	int layers = map->levels();
 	for(auto & elem : teams)
 	{
-		auto fow = elem.second.fogOfWarMap;
+		auto & fow = elem.second.fogOfWarMap;
 		fow->resize(boost::extents[layers][map->width][map->height]);
 		std::fill(fow->data(), fow->data() + fow->num_elements(), 0);
 
@@ -1952,7 +1952,7 @@ bool RumorState::update(int id, int extra)
 TeamState::TeamState()
 {
 	setNodeType(TEAM);
-	fogOfWarMap = std::make_shared<boost::multi_array<ui8, 3>>();
+	fogOfWarMap = std::make_unique<boost::multi_array<ui8, 3>>();
 }
 
 TeamState::TeamState(TeamState && other) noexcept:

+ 15 - 3
lib/mapObjects/CGHeroInstance.cpp

@@ -327,13 +327,21 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 	{
 		// hero starts with default spellbook presence status
 		if(!getArt(ArtifactPosition::SPELLBOOK) && type->haveSpellBook)
-			putArtifact(ArtifactPosition::SPELLBOOK, ArtifactUtils::createNewArtifactInstance(ArtifactID::SPELLBOOK));
+		{
+			auto artifact = ArtifactUtils::createNewArtifactInstance(ArtifactID::SPELLBOOK);
+			cb->gameState()->map->addNewArtifactInstance(artifact);
+			putArtifact(ArtifactPosition::SPELLBOOK, artifact);
+		}
 	}
 	else
 		spells -= SpellID::SPELLBOOK_PRESET;
 
 	if(!getArt(ArtifactPosition::MACH4))
-		putArtifact(ArtifactPosition::MACH4, ArtifactUtils::createNewArtifactInstance(ArtifactID::CATAPULT)); //everyone has a catapult
+	{
+		auto artifact = ArtifactUtils::createNewArtifactInstance(ArtifactID::CATAPULT);
+		cb->gameState()->map->addNewArtifactInstance(artifact);
+		putArtifact(ArtifactPosition::MACH4, artifact); //everyone has a catapult
+	}
 
 	if(!hasBonus(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)))
 	{
@@ -448,7 +456,11 @@ void CGHeroInstance::initArmy(CRandomGenerator & rand, IArmyDescriptor * dst)
 				ArtifactPosition slot = art->getPossibleSlots().at(ArtBearer::HERO).front();
 
 				if(!getArt(slot))
-					putArtifact(slot, ArtifactUtils::createNewArtifactInstance(aid));
+				{
+					auto artifact = ArtifactUtils::createNewArtifactInstance(aid);
+					cb->gameState()->map->addNewArtifactInstance(artifact);
+					putArtifact(slot, artifact);
+				}
 				else
 					logGlobal->warn("Hero %s already has artifact at %d, omitting giving artifact %d", getNameTranslated(), slot.toEnum(), aid.toEnum());
 			}

+ 5 - 1
lib/mapObjects/CGObjectInstance.cpp

@@ -35,10 +35,14 @@ CGObjectInstance::CGObjectInstance():
 	tempOwner(PlayerColor::UNFLAGGABLE),
 	blockVisit(false)
 {
+	logGlobal->debug("Created object at %d", ptrdiff_t(this));
 }
 
 //must be instantiated in .cpp file for access to complete types of all member fields
-CGObjectInstance::~CGObjectInstance() = default;
+CGObjectInstance::~CGObjectInstance()
+{
+	logGlobal->debug("Deleted object %d at %d", id.getNum(), ptrdiff_t(this));
+}
 
 MapObjectID CGObjectInstance::getObjGroupIndex() const
 {

+ 3 - 0
lib/mapping/CMap.cpp

@@ -181,6 +181,9 @@ CMap::~CMap()
 	for(auto quest : quests)
 		quest.dellNull();
 
+	for(auto artInstance : artInstances)
+		artInstance.dellNull();
+
 	resetStaticData();
 }
 

+ 2 - 2
lib/networkPacks/NetPacksLib.cpp

@@ -932,7 +932,7 @@ void SetMovePoints::applyGs(CGameState * gs) const
 void FoWChange::applyGs(CGameState *gs)
 {
 	TeamState * team = gs->getPlayerTeam(player);
-	auto fogOfWarMap = team->fogOfWarMap;
+	auto & fogOfWarMap = team->fogOfWarMap;
 	for(const int3 & t : tiles)
 		(*fogOfWarMap)[t.z][t.x][t.y] = mode != ETileVisibility::HIDDEN;
 
@@ -1327,7 +1327,7 @@ void TryMoveHero::applyGs(CGameState *gs)
 		gs->map->addBlockVisTiles(h);
 	}
 
-	auto fogOfWarMap = gs->getPlayerTeam(h->getOwner())->fogOfWarMap;
+	auto & fogOfWarMap = gs->getPlayerTeam(h->getOwner())->fogOfWarMap;
 	for(const int3 & t : fowRevealed)
 		(*fogOfWarMap)[t.z][t.x][t.y] = 1;
 }

+ 1 - 1
lib/pathfinder/NodeStorage.cpp

@@ -27,7 +27,7 @@ void NodeStorage::initialize(const PathfinderOptions & options, const CGameState
 	int3 pos;
 	const PlayerColor player = out.hero->tempOwner;
 	const int3 sizes = gs->getMapSize();
-	const auto fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(player)->fogOfWarMap;
+	const auto & fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(player)->fogOfWarMap;
 
 	//make 200% sure that these are loop invariants (also a bit shorter code), let compiler do the rest(loop unswitching)
 	const bool useFlying = options.useFlying;

+ 2 - 2
lib/pathfinder/PathfinderUtil.h

@@ -19,11 +19,11 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 namespace PathfinderUtil
 {
-	using FoW = std::shared_ptr<const boost::multi_array<ui8, 3>>;
+	using FoW = std::unique_ptr<boost::multi_array<ui8, 3>>;
 	using ELayer = EPathfindingLayer;
 
 	template<EPathfindingLayer::Type layer>
-	EPathAccessibility evaluateAccessibility(const int3 & pos, const TerrainTile & tinfo, FoW fow, const PlayerColor player, const CGameState * gs)
+	EPathAccessibility evaluateAccessibility(const int3 & pos, const TerrainTile & tinfo, const FoW & fow, const PlayerColor player, const CGameState * gs)
 	{
 		if(!(*fow)[pos.z][pos.x][pos.y])
 			return EPathAccessibility::BLOCKED;

+ 1 - 1
lib/serializer/BinaryDeserializer.h

@@ -292,7 +292,7 @@ public:
 		{
 			typedef typename std::remove_pointer<T>::type npT;
 			typedef typename std::remove_const<npT>::type ncpT;
-			data = ClassObjectCreator<ncpT>::invoke();
+			data = ClassObjectCreator<ncpT>::invoke();// !!!
 			ptrAllocated(data, pid);
 			load(*data);
 		}

+ 1 - 1
lib/spells/AdventureSpellMechanics.cpp

@@ -600,7 +600,7 @@ ESpellCastResult ViewMechanics::applyAdventureEffects(SpellCastEnvironment * env
 
 	const auto spellLevel = parameters.caster->getSpellSchoolLevel(owner);
 
-	const auto fowMap = env->getCb()->getPlayerTeam(parameters.caster->getCasterOwner())->fogOfWarMap;
+	const auto & fowMap = env->getCb()->getPlayerTeam(parameters.caster->getCasterOwner())->fogOfWarMap;
 
 	for(const CGObjectInstance * obj : env->getMap()->objects)
 	{

+ 1 - 1
server/CGameHandler.cpp

@@ -860,7 +860,7 @@ void CGameHandler::onNewTurn()
 				fw.mode = ETileVisibility::REVEALED;
 				fw.player = player;
 				// find all hidden tiles
-				const auto fow = getPlayerTeam(player)->fogOfWarMap;
+				const auto & fow = getPlayerTeam(player)->fogOfWarMap;
 
 				auto shape = fow->shape();
 				for(size_t z = 0; z < shape[0]; z++)