Merge pull request #4130 from IvanSavenko/races_fixes

Fixes for discovered uninitialized memory access and thread data races

AI/Nullkiller/Analyzers/ObjectClusterizer.h

@@ -17,10 +17,10 @@ namespace NKAI
 
 struct ClusterObjectInfo
 {
-	float priority;
-	float movementCost;
-	uint64_t danger;
-	uint8_t turn;
+	float priority = 0.f;
+	float movementCost = 0.f;
+	uint64_t danger = 0;
+	uint8_t turn = 0;
 };
 
 struct ObjectInstanceIDHash

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -1203,7 +1203,10 @@ void AINodeStorage::calculateTownPortalTeleportations(std::vector<CGPathNode *>
 	std::vector<const ChainActor *> actorsVector(actorsOfInitial.begin(), actorsOfInitial.end());
 	tbb::concurrent_vector<CGPathNode *> output;
 
-	if(actorsVector.size() * initialNodes.size() > 1000)
+// TODO: re-enable after fixing thread races. See issue for details:
+// https://github.com/vcmi/vcmi/pull/4130
+#if 0
+	if (actorsVector.size() * initialNodes.size() > 1000)
 	{
 		tbb::parallel_for(tbb::blocked_range<size_t>(0, actorsVector.size()), [&](const tbb::blocked_range<size_t> & r)
 			{
@@ -1216,6 +1219,7 @@ void AINodeStorage::calculateTownPortalTeleportations(std::vector<CGPathNode *>
 		std::copy(output.begin(), output.end(), std::back_inserter(initialNodes));
 	}
 	else
+#endif
 	{
 		for(auto actor : actorsVector)
 		{

client/gui/CGuiHandler.cpp

@@ -118,9 +118,9 @@ void CGuiHandler::renderFrame()
 
 		if (settings["video"]["showfps"].Bool())
 			drawFPSCounter();
-	}
 
-	SDL_UpdateTexture(screenTexture, nullptr, screen->pixels, screen->pitch);
+		SDL_UpdateTexture(screenTexture, nullptr, screen->pixels, screen->pitch);
+	}
 
 	SDL_RenderClear(mainRenderer);
 	SDL_RenderCopy(mainRenderer, screenTexture, nullptr, nullptr);

lib/bonuses/Bonus.h

@@ -64,7 +64,7 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	BonusSubtypeID subtype;
 
 	BonusSource source = BonusSource::OTHER; //source type" uses BonusSource values - what gave that bonus
-	BonusSource targetSourceType;//Bonuses of what origin this amplifies, uses BonusSource values. Needed for PERCENT_TO_TARGET_TYPE.
+	BonusSource targetSourceType = BonusSource::OTHER;//Bonuses of what origin this amplifies, uses BonusSource values. Needed for PERCENT_TO_TARGET_TYPE.
 	si32 val = 0;
 	BonusSourceID sid; //source id: id of object/artifact/spell
 	BonusValueType valType = BonusValueType::ADDITIVE_VALUE;

lib/bonuses/BonusSelector.cpp

@@ -51,8 +51,15 @@ namespace Selector
 		return seffectRange;
 	}
 
-	DLL_LINKAGE CWillLastTurns turns;
-	DLL_LINKAGE CWillLastDays days;
+	DLL_LINKAGE CWillLastTurns turns(int turns)
+	{
+		return CWillLastTurns(turns);
+	}
+
+	DLL_LINKAGE CWillLastDays days(int days)
+	{
+		return CWillLastDays(days);
+	}
 
 	CSelector DLL_LINKAGE typeSubtype(BonusType Type, BonusSubtypeID Subtype)
 	{

lib/bonuses/BonusSelector.h

@@ -81,8 +81,11 @@ public:
 
 class DLL_LINKAGE CWillLastTurns
 {
-public:
 	int turnsRequested;
+public:
+	CWillLastTurns(int turnsRequested):
+		turnsRequested(turnsRequested)
+	{}
 
 	bool operator()(const Bonus *bonus) const
 	{
@@ -90,18 +93,17 @@ public:
 			|| !Bonus::NTurns(bonus) //so do every not expriing after N-turns effect
 			|| bonus->turnsRemain > turnsRequested;
 	}
-	CWillLastTurns& operator()(const int &setVal)
-	{
-		turnsRequested = setVal;
-		return *this;
-	}
 };
 
 class DLL_LINKAGE CWillLastDays
 {
-public:
 	int daysRequested;
 
+public:
+	CWillLastDays(int daysRequested):
+		daysRequested(daysRequested)
+	{}
+
 	bool operator()(const Bonus *bonus) const
 	{
 		if(daysRequested <= 0 || Bonus::Permanent(bonus) || Bonus::OneBattle(bonus))
@@ -112,14 +114,8 @@ public:
 		{
 			return bonus->turnsRemain > daysRequested;
 		}
-
 		return false; // TODO: ONE_WEEK need support for turnsRemain, but for now we'll exclude all unhandled durations
 	}
-	CWillLastDays& operator()(const int &setVal)
-	{
-		daysRequested = setVal;
-		return *this;
-	}
 };
 
 
@@ -131,8 +127,8 @@ namespace Selector
 	extern DLL_LINKAGE const CSelectFieldEqual<BonusSource> & sourceType();
 	extern DLL_LINKAGE const CSelectFieldEqual<BonusSource> & targetSourceType();
 	extern DLL_LINKAGE const CSelectFieldEqual<BonusLimitEffect> & effectRange();
-	extern DLL_LINKAGE CWillLastTurns turns;
-	extern DLL_LINKAGE CWillLastDays days;
+	CWillLastTurns DLL_LINKAGE turns(int turns);
+	CWillLastDays DLL_LINKAGE days(int days);
 
 	CSelector DLL_LINKAGE typeSubtype(BonusType Type, BonusSubtypeID Subtype);
 	CSelector DLL_LINKAGE typeSubtypeInfo(BonusType type, BonusSubtypeID subtype, const CAddInfo & info);

lib/bonuses/Limiters.h

@@ -117,7 +117,7 @@ class DLL_LINKAGE HasAnotherBonusLimiter : public ILimiter //applies only to nod
 public:
 	BonusType type;
 	BonusSubtypeID subtype;
-	BonusSource source;
+	BonusSource source = BonusSource::OTHER;
 	BonusSourceID sid;
 	bool isSubtypeRelevant; //check for subtype only if this is true
 	bool isSourceRelevant; //check for bonus source only if this is true

lib/serializer/Connection.cpp

@@ -77,6 +77,7 @@ void CConnection::sendPack(const CPack * pack)
 	if (!connectionPtr)
 		throw std::runtime_error("Attempt to send packet on a closed connection!");
 
+	packWriter->buffer.clear();
 	*serializer & pack;
 
 	logNetwork->trace("Sending a pack of type %s", typeid(*pack).name());