Merge pull request #5658 from IvanSavenko/memleak_fix

Fix discovered memory leaks & reduce usage of raw pointers

Global.h

@@ -465,14 +465,6 @@ namespace vstd
 		}
 	};
 
-	//deleted pointer and sets it to nullptr
-	template <typename T>
-	void clear_pointer(T* &ptr)
-	{
-		delete ptr;
-		ptr = nullptr;
-	}
-
 	template <typename Container>
 	typename Container::const_reference circularAt(const Container &r, size_t index)
 	{

client/CServerHandler.cpp

@@ -57,6 +57,7 @@
 #include "LobbyClientNetPackVisitors.h"
 
 #include <vcmi/events/EventBus.h>
+#include <SDL_thread.h>
 
 #include <boost/lexical_cast.hpp>
 
@@ -114,9 +115,13 @@ void CServerHandler::threadRunNetwork()
 	}
 	catch (const TerminationRequestedException &)
 	{
+		// VCMI can run SDL methods on network thread, leading to usage of thread-local storage by SDL
+		// Such storage needs to be cleaned up manually for threads that were not created by SDL
+		SDL_TLSCleanup();
 		logGlobal->info("Terminating network thread");
 		return;
 	}
+	SDL_TLSCleanup();
 	logGlobal->info("Ending network thread");
 }
 

client/HeroMovementController.cpp

@@ -312,7 +312,7 @@ void HeroMovementController::updateMovementSound(const CGHeroInstance * h, int3
 			ENGINE->sound().stopSound(currentMovementSoundChannel);
 
 		if(!currentMovementSoundName.empty())
-			currentMovementSoundChannel = ENGINE->sound().playSound(currentMovementSoundName, -1, true);
+			currentMovementSoundChannel = ENGINE->sound().playSoundLooped(currentMovementSoundName);
 		else
 			currentMovementSoundChannel = -1;
 	}

client/battle/BattleAnimationClasses.cpp

@@ -357,7 +357,7 @@ bool MovementAnimation::init()
 
 	if (moveSoundHandler == -1)
 	{
-		moveSoundHandler = ENGINE->sound().playSound(stack->unitType()->sounds.move, -1);
+		moveSoundHandler = ENGINE->sound().playSoundLooped(stack->unitType()->sounds.move);
 	}
 
 	Point begPosition = owner.stacksController->getStackPositionAtHex(prevHex, stack);

client/eventsSDL/InputHandler.cpp

@@ -405,23 +405,28 @@ int InputHandler::getNumTouchFingers() const
 
 void InputHandler::dispatchMainThread(const std::function<void()> & functor)
 {
-	auto heapFunctor = new std::function<void()>(functor);
+	auto heapFunctor = std::make_unique<std::function<void()>>(functor);
 
 	SDL_Event event;
 	event.user.type = SDL_USEREVENT;
 	event.user.code = 0;
-	event.user.data1 = static_cast <void*>(heapFunctor);
+	event.user.data1 = nullptr;
 	event.user.data2 = nullptr;
 	SDL_PushEvent(&event);
+
+	// NOTE: approach with dispatchedTasks container is a bit excessive
+	// used mostly to prevent false-positives leaks in analyzers
+	dispatchedTasks.push(std::move(heapFunctor));
 }
 
 void InputHandler::handleUserEvent(const SDL_UserEvent & current)
 {
-	auto heapFunctor = static_cast<std::function<void()>*>(current.data1);
+	std::unique_ptr<std::function<void()>> task;
 
-	(*heapFunctor)();
+	if (!dispatchedTasks.try_pop(task))
+		throw std::runtime_error("InputHandler::handleUserEvent received without active task!");
 
-	delete heapFunctor;
+	(*task)();
 }
 
 const Point & InputHandler::getCursorPosition() const

client/eventsSDL/InputHandler.h

@@ -12,6 +12,8 @@
 
 #include "../lib/Rect.h"
 
+#include <tbb/concurrent_queue.h>
+
 enum class EUserEvent;
 enum class MouseButton;
 union SDL_Event;
@@ -33,6 +35,7 @@ enum class InputMode
 class InputHandler
 {
 	std::vector<SDL_Event> eventsQueue;
+	tbb::concurrent_queue<std::unique_ptr<std::function<void()>>> dispatchedTasks;
 	std::mutex eventsMutex;
 
 	Point cursorPosition;

client/lobby/CCampaignInfoScreen.cpp

@@ -23,8 +23,8 @@
 CCampaignInfoScreen::CCampaignInfoScreen()
 {
 	OBJECT_CONSTRUCTION;
-	localSi = new StartInfo(*GAME->interface()->cb->getStartInfo());
-	localMi = new CMapInfo();
+	localSi = std::make_unique<StartInfo>(*GAME->interface()->cb->getStartInfo());
+	localMi = std::make_unique<CMapInfo>();
 	localMi->mapHeader = std::unique_ptr<CMapHeader>(new CMapHeader(*GAME->interface()->cb->getMapHeader()));
 
 	screenType = ESelectionScreen::scenarioInfo;
@@ -32,17 +32,13 @@ CCampaignInfoScreen::CCampaignInfoScreen()
 	updateAfterStateChange();
 }
 
-CCampaignInfoScreen::~CCampaignInfoScreen()
-{
-	vstd::clear_pointer(localSi);
-	vstd::clear_pointer(localMi);
-}
+CCampaignInfoScreen::~CCampaignInfoScreen() = default;
 
 const CMapInfo * CCampaignInfoScreen::getMapInfo()
 {
-	return localMi;
+	return localMi.get();
 }
 const StartInfo * CCampaignInfoScreen::getStartInfo()
 {
-	return localSi;
+	return localSi.get();
 }

client/lobby/CCampaignInfoScreen.h

@@ -14,8 +14,8 @@
 
 class CCampaignInfoScreen : public CBonusSelection, ISelectionScreenInfo
 {
-	const StartInfo * localSi;
-	CMapInfo * localMi;
+	std::unique_ptr<StartInfo> localSi;
+	std::unique_ptr<CMapInfo> localMi;
 
 public:
 	CCampaignInfoScreen();
@@ -23,4 +23,4 @@ public:
 
 	const CMapInfo * getMapInfo() override;
 	const StartInfo * getStartInfo() override;
-};
+};

client/lobby/CScenarioInfoScreen.cpp

@@ -33,8 +33,8 @@ CScenarioInfoScreen::CScenarioInfoScreen()
 	pos.h = 600;
 	pos = center();
 
-	localSi = new StartInfo(*GAME->interface()->cb->getStartInfo());
-	localMi = new CMapInfo();
+	localSi = std::make_unique<StartInfo>(*GAME->interface()->cb->getStartInfo());
+	localMi = std::make_unique<CMapInfo>();
 	localMi->mapHeader = std::unique_ptr<CMapHeader>(new CMapHeader(*GAME->interface()->cb->getMapHeader()));
 
 	screenType = ESelectionScreen::scenarioInfo;
@@ -49,18 +49,14 @@ CScenarioInfoScreen::CScenarioInfoScreen()
 	buttonBack = std::make_shared<CButton>(Point(584, 535), AnimationPath::builtin("SCNRBACK.DEF"), LIBRARY->generaltexth->zelp[105], [this](){ close();}, EShortcut::GLOBAL_CANCEL);
 }
 
-CScenarioInfoScreen::~CScenarioInfoScreen()
-{
-	vstd::clear_pointer(localSi);
-	vstd::clear_pointer(localMi);
-}
+CScenarioInfoScreen::~CScenarioInfoScreen() = default;
 
 const CMapInfo * CScenarioInfoScreen::getMapInfo()
 {
-	return localMi;
+	return localMi.get();
 }
 
 const StartInfo * CScenarioInfoScreen::getStartInfo()
 {
-	return localSi;
+	return localSi.get();
 }

client/lobby/CScenarioInfoScreen.h

@@ -14,14 +14,13 @@
 /// Scenario information screen shown during the game
 class CScenarioInfoScreen : public WindowBase, public ISelectionScreenInfo
 {
+	std::unique_ptr<StartInfo> localSi;
+	std::unique_ptr<CMapInfo> localMi;
 public:
 	std::shared_ptr<CButton> buttonBack;
 	std::shared_ptr<InfoCard> card;
 	std::shared_ptr<OptionsTab> opt;
 
-	const StartInfo * localSi;
-	CMapInfo * localMi;
-
 	CScenarioInfoScreen();
 	~CScenarioInfoScreen();
 

client/lobby/OptionsTab.cpp

@@ -378,7 +378,7 @@ void OptionsTab::CPlayerOptionTooltipBox::genTownWindow()
 	genHeader();
 	labelAssociatedCreatures = std::make_shared<CLabel>(pos.w / 2 + 8, 122, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, LIBRARY->generaltexth->allTexts[79]);
 	std::vector<std::shared_ptr<CComponent>> components;
-	const CTown * town = (*LIBRARY->townh)[factionIndex]->town;
+	const CTown * town = (*LIBRARY->townh)[factionIndex]->town.get();
 
 	for(auto & elem : town->creatures)
 	{

client/media/CSoundHandler.cpp

@@ -52,6 +52,11 @@ CSoundHandler::CSoundHandler():
 	}
 }
 
+void CSoundHandler::MixChunkDeleter::operator()(Mix_Chunk * ptr)
+{
+	Mix_FreeChunk(ptr);
+}
+
 CSoundHandler::~CSoundHandler()
 {
 	if(isInitialized())
@@ -59,36 +64,28 @@ CSoundHandler::~CSoundHandler()
 		Mix_ChannelFinished(nullptr);
 		Mix_HaltChannel(-1);
 
-		for(auto & chunk : soundChunks)
-		{
-			if(chunk.second.first)
-				Mix_FreeChunk(chunk.second.first);
-		}
-
-		for(auto & chunk : soundChunksRaw)
-		{
-			if(chunk.second.first)
-				Mix_FreeChunk(chunk.second.first);
-		}
+		soundChunks.clear();
+		uncachedPlayingChunks.clear();
 	}
 }
 
+Mix_Chunk * CSoundHandler::getSoundChunkCached(const AudioPath & sound)
+{
+	if (soundChunks.find(sound) == soundChunks.end())
+		soundChunks[sound].first = getSoundChunk(sound);
+
+	return soundChunks[sound].first.get();
+}
+
 // Allocate an SDL chunk and cache it.
-Mix_Chunk * CSoundHandler::GetSoundChunk(const AudioPath & sound, bool cache)
+CSoundHandler::MixChunkPtr CSoundHandler::getSoundChunk(const AudioPath & sound)
 {
 	try
 	{
-		if(cache && soundChunks.find(sound) != soundChunks.end())
-			return soundChunks[sound].first;
-
 		auto data = CResourceHandler::get()->load(sound.addPrefix("SOUNDS/"))->readAll();
 		SDL_RWops * ops = SDL_RWFromMem(data.first.get(), data.second);
 		Mix_Chunk * chunk = Mix_LoadWAV_RW(ops, 1); // will free ops
-
-		if(cache)
-			soundChunks.insert({sound, std::make_pair(chunk, std::move(data.first))});
-
-		return chunk;
+		return MixChunkPtr(chunk);
 	}
 	catch(std::exception & e)
 	{
@@ -97,22 +94,15 @@ Mix_Chunk * CSoundHandler::GetSoundChunk(const AudioPath & sound, bool cache)
 	}
 }
 
-Mix_Chunk * CSoundHandler::GetSoundChunk(std::pair<std::unique_ptr<ui8[]>, si64> & data, bool cache)
+CSoundHandler::MixChunkPtr CSoundHandler::getSoundChunk(std::pair<std::unique_ptr<ui8[]>, si64> & data)
 {
 	try
 	{
 		std::vector<ui8> startBytes = std::vector<ui8>(data.first.get(), data.first.get() + std::min(static_cast<si64>(100), data.second));
 
-		if(cache && soundChunksRaw.find(startBytes) != soundChunksRaw.end())
-			return soundChunksRaw[startBytes].first;
-
 		SDL_RWops * ops = SDL_RWFromMem(data.first.get(), data.second);
 		Mix_Chunk * chunk = Mix_LoadWAV_RW(ops, 1); // will free ops
-
-		if(cache)
-			soundChunksRaw.insert({startBytes, std::make_pair(chunk, std::move(data.first))});
-
-		return chunk;
+		return MixChunkPtr(chunk);
 	}
 	catch(std::exception & e)
 	{
@@ -174,36 +164,53 @@ uint32_t CSoundHandler::getSoundDurationMilliseconds(const AudioPath & sound)
 }
 
 // Plays a sound, and return its channel so we can fade it out later
-int CSoundHandler::playSound(soundBase::soundID soundID, int repeats)
+int CSoundHandler::playSound(soundBase::soundID soundID)
 {
 	assert(soundID < soundBase::sound_after_last);
 	auto sound = AudioPath::builtin(soundsList[soundID]);
 	logGlobal->trace("Attempt to play sound %d with file name %s with cache", soundID, sound.getOriginalName());
 
-	return playSound(sound, repeats, true);
+	return playSoundImpl(sound, 0, true);
+}
+
+int CSoundHandler::playSoundLooped(const AudioPath & sound)
+{
+	return playSoundImpl(sound, -1, true);
 }
 
-int CSoundHandler::playSound(const AudioPath & sound, int repeats, bool cache)
+int CSoundHandler::playSound(const AudioPath & sound)
+{
+	return playSoundImpl(sound, 0, false);
+}
+
+int CSoundHandler::playSoundImpl(const AudioPath & sound, int repeats, bool useCache)
 {
 	if(!isInitialized() || sound.empty())
 		return -1;
 
 	int channel;
-	Mix_Chunk * chunk = GetSoundChunk(sound, cache);
+	MixChunkPtr chunkPtr = getSoundChunk(sound);
+	Mix_Chunk * chunk = nullptr;
+	if (!useCache)
+	{
+		chunkPtr = getSoundChunk(sound);
+		chunk = chunkPtr.get();
+	}
+	else
+		chunk = getSoundChunkCached(sound);
 
 	if(chunk)
 	{
-		channel = Mix_PlayChannel(-1, chunk, repeats);
+		channel = Mix_PlayChannel(-1, chunk, 0);
 		if(channel == -1)
 		{
 			logGlobal->error("Unable to play sound file %s , error %s", sound.getOriginalName(), Mix_GetError());
-			if(!cache)
-				Mix_FreeChunk(chunk);
 		}
-		else if(cache)
-			initCallback(channel);
 		else
-			initCallback(channel, [chunk](){ Mix_FreeChunk(chunk);});
+		{
+			storeChunk(channel, std::move(chunkPtr));
+			initCallback(channel);
+		}
 	}
 	else
 		channel = -1;
@@ -211,22 +218,28 @@ int CSoundHandler::playSound(const AudioPath & sound, int repeats, bool cache)
 	return channel;
 }
 
-int CSoundHandler::playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data, int repeats, bool cache)
+void CSoundHandler::storeChunk(int channel, MixChunkPtr chunk)
+{
+	std::scoped_lock lockGuard(mutexCallbacks);
+	uncachedPlayingChunks[channel] = std::move(chunk);
+}
+
+int CSoundHandler::playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data)
 {
 	int channel = -1;
-	if(Mix_Chunk * chunk = GetSoundChunk(data, cache))
+	auto chunk = getSoundChunk(data);
+	if(chunk)
 	{
-		channel = Mix_PlayChannel(-1, chunk, repeats);
+		channel = Mix_PlayChannel(-1, chunk.get(), 0);
 		if(channel == -1)
 		{
 			logGlobal->error("Unable to play sound, error %s", Mix_GetError());
-			if(!cache)
-				Mix_FreeChunk(chunk);
 		}
-		else if(cache)
-			initCallback(channel);
 		else
-			initCallback(channel, [chunk](){ Mix_FreeChunk(chunk);});
+		{
+			storeChunk(channel, std::move(chunk));
+			initCallback(channel);
+		}
 	}
 	return channel;
 }
@@ -312,6 +325,8 @@ void CSoundHandler::soundFinishedCallback(int channel)
 {
 	std::scoped_lock lockGuard(mutexCallbacks);
 
+	uncachedPlayingChunks.erase(channel);
+
 	if(callbacks.count(channel) == 0)
 		return;
 
@@ -386,7 +401,7 @@ void CSoundHandler::ambientUpdateChannels(std::map<AudioPath, int> soundsArg)
 
 		if(!vstd::contains(ambientChannels, soundId))
 		{
-			int channel = playSound(soundId, -1);
+			int channel = playSoundLooped(soundId);
 			int channelVolume = ambientDistToVolume(distance);
 			channelVolumes[channel] = channelVolume;
 

client/media/CSoundHandler.h

@@ -19,16 +19,29 @@ struct Mix_Chunk;
 class CSoundHandler final : public CAudioBase, public ISoundPlayer
 {
 private:
+	struct MixChunkDeleter
+	{
+		void operator ()(Mix_Chunk *);
+	};
+	using MixChunkPtr = std::unique_ptr<Mix_Chunk, MixChunkDeleter>;
+
 	//update volume on configuration change
 	SettingsListener listener;
 	void onVolumeChange(const JsonNode & volumeNode);
 
-	using CachedChunk = std::pair<Mix_Chunk *, std::unique_ptr<ui8[]>>;
+	using CachedChunk = std::pair<MixChunkPtr, std::unique_ptr<ui8[]>>;
+
+	/// List of all permanently cached sound chunks
 	std::map<AudioPath, CachedChunk> soundChunks;
-	std::map<std::vector<ui8>, CachedChunk> soundChunksRaw;
 
-	Mix_Chunk * GetSoundChunk(const AudioPath & sound, bool cache);
-	Mix_Chunk * GetSoundChunk(std::pair<std::unique_ptr<ui8[]>, si64> & data, bool cache);
+	/// List of all currently playing chunks that are currently playing
+	/// and should be deallocated once channel playback is over
+	/// indexed by channel ID
+	std::map<int, MixChunkPtr> uncachedPlayingChunks;
+
+	MixChunkPtr getSoundChunk(const AudioPath & sound);
+	Mix_Chunk * getSoundChunkCached(const AudioPath & sound);
+	MixChunkPtr getSoundChunk(std::pair<std::unique_ptr<ui8[]>, si64> & data);
 
 	/// have entry for every currently active channel
 	/// vector will be empty if callback was not set
@@ -51,6 +64,8 @@ private:
 
 	void initCallback(int channel, const std::function<void()> & function);
 	void initCallback(int channel);
+	void storeChunk(int channel, MixChunkPtr chunk);
+	int playSoundImpl(const AudioPath & sound, int repeats, bool useCache);
 
 public:
 	CSoundHandler();
@@ -62,9 +77,10 @@ public:
 
 	// Sounds
 	uint32_t getSoundDurationMilliseconds(const AudioPath & sound) final;
-	int playSound(soundBase::soundID soundID, int repeats = 0) final;
-	int playSound(const AudioPath & sound, int repeats = 0, bool cache = false) final;
-	int playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data, int repeats = 0, bool cache = false) final;
+	int playSound(soundBase::soundID soundID) final;
+	int playSound(const AudioPath & sound) final;
+	int playSoundLooped(const AudioPath & sound) final;
+	int playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data) final;
 	int playSoundFromSet(std::vector<soundBase::soundID> & sound_vec) final;
 	void stopSound(int handler) final;
 	void pauseSound(int handler) final;

client/media/CVideoHandler.cpp

@@ -373,6 +373,11 @@ FFMpegStream::~FFMpegStream()
 	avcodec_free_context(&codecContext);
 
 	avformat_close_input(&formatContext);
+
+	// for some reason, buffer is managed (e.g. reallocated) by FFmpeg
+	// however, it must still be freed manually by user
+	if (context->buffer)
+		av_free(context->buffer);
 	av_free(context);
 }
 

client/media/ISoundPlayer.h

@@ -17,9 +17,10 @@ class ISoundPlayer
 public:
 	virtual ~ISoundPlayer() = default;
 
-	virtual int playSound(soundBase::soundID soundID, int repeats = 0) = 0;
-	virtual int playSound(const AudioPath & sound, int repeats = 0, bool cache = false) = 0;
-	virtual int playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data, int repeats = 0, bool cache = false) = 0;
+	virtual int playSound(soundBase::soundID soundID) = 0;
+	virtual int playSound(const AudioPath & sound) = 0;
+	virtual int playSoundLooped(const AudioPath & sound) = 0;
+	virtual int playSound(std::pair<std::unique_ptr<ui8[]>, si64> & data) = 0;
 	virtual int playSoundFromSet(std::vector<soundBase::soundID> & sound_vec) = 0;
 	virtual void stopSound(int handler) = 0;
 	virtual void pauseSound(int handler) = 0;

client/renderSDL/SDLImage.cpp

@@ -448,5 +448,6 @@ void SDLImageShared::savePalette()
 SDLImageShared::~SDLImageShared()
 {
 	SDL_FreeSurface(surf);
-	SDL_FreePalette(originalPalette);
+	if (originalPalette)
+		SDL_FreePalette(originalPalette);
 }

client/renderSDL/SDLImageScaler.cpp

@@ -236,8 +236,8 @@ SDLImageScaler::~SDLImageScaler()
 {
 	ENGINE->dispatchMainThread([surface = intermediate]()
 	{
-		// potentially SDL bug, execute SDL_FreeSurface in main thread to avoid thread races to its internal state
-		// may be fixed somewhere between 2.26.5 - 2.30
+		// SDL_FreeSurface must be executed in main thread to avoid thread races to its internal state
+		// will be no longer necessary in SDL 3
 		SDL_FreeSurface(surface);
 	});
 }

client/renderSDL/ScalableImage.cpp

@@ -90,7 +90,8 @@ ScalableImageParameters::ScalableImageParameters(const SDL_Palette * originalPal
 
 ScalableImageParameters::~ScalableImageParameters()
 {
-	SDL_FreePalette(palette);
+	if (palette)
+		SDL_FreePalette(palette);
 }
 
 void ScalableImageParameters::preparePalette(const SDL_Palette * originalPalette, EImageBlitMode blitMode)

clientapp/EntryPoint.cpp

@@ -416,7 +416,8 @@ int main(int argc, char * argv[])
 	if(!settings["session"]["headless"].Bool())
 	{
 		CMessage::dispose();
-		vstd::clear_pointer(graphics);
+		delete graphics;
+		graphics = nullptr;
 	}
 
 	// must be executed before reset - since unique_ptr resets pointer to null before calling destructor
@@ -424,7 +425,8 @@ int main(int argc, char * argv[])
 
 	ENGINE.reset();
 
-	vstd::clear_pointer(LIBRARY);
+	delete LIBRARY;
+	LIBRARY = nullptr;
 	logConfigurator.deconfigure();
 
 	std::cout << "Ending...\n";

lib/entities/faction/CFaction.cpp

@@ -17,14 +17,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CFaction::~CFaction()
-{
-	if (town)
-	{
-		delete town;
-		town = nullptr;
-	}
-}
+CFaction::CFaction() = default;
+CFaction::~CFaction() = default;
 
 int32_t CFaction::getIndex() const
 {

lib/entities/faction/CFaction.h

@@ -51,14 +51,14 @@ public:
 	/// and for placing heroes directly on boat (in map editor, water prisons & taverns)
 	BoatId boatType = BoatId::CASTLE;
 
-	CTown * town = nullptr; //NOTE: can be null
+	std::unique_ptr<CTown> town; //NOTE: can be null
 
 	ImagePath creatureBg120;
 	ImagePath creatureBg130;
 
 	std::vector<SPuzzleInfo> puzzleMap;
 
-	CFaction() = default;
+	CFaction();
 	~CFaction();
 
 	int32_t getIndex() const override;

lib/entities/faction/CTownHandler.cpp

@@ -38,19 +38,15 @@ const int NAMES_PER_TOWN=16; // number of town names per faction in H3 files. Js
 
 CTownHandler::CTownHandler()
 	: buildingsLibrary(JsonPath::builtin("config/buildingsLibrary"))
-	, randomTown(new CTown())
-	, randomFaction(new CFaction())
+	, randomFaction(std::make_unique<CFaction>())
 {
-	randomFaction->town = randomTown;
-	randomTown->faction = randomFaction;
+	randomFaction->town = std::make_unique<CTown>();
+	randomFaction->town->faction = randomFaction.get();
 	randomFaction->identifier = "random";
 	randomFaction->modScope = "core";
 }
 
-CTownHandler::~CTownHandler()
-{
-	delete randomFaction; // will also delete randomTown
-}
+CTownHandler::~CTownHandler() = default;
 
 JsonNode readBuilding(CLegacyConfigParser & parser)
 {
@@ -767,9 +763,9 @@ std::shared_ptr<CFaction> CTownHandler::loadFromJson(const std::string & scope,
 
 	if (!source["town"].isNull())
 	{
-		faction->town = new CTown();
+		faction->town = std::make_unique<CTown>();
 		faction->town->faction = faction.get();
-		loadTown(faction->town, source["town"]);
+		loadTown(faction->town.get(), source["town"]);
 	}
 	else
 		faction->town = nullptr;
@@ -854,7 +850,7 @@ void CTownHandler::loadRandomFaction()
 {
 	JsonNode randomFactionJson(JsonPath::builtin("config/factions/random.json"));
 	randomFactionJson.setModScope(ModScope::scopeBuiltin(), true);
-	loadBuildings(randomTown, randomFactionJson["random"]["town"]["buildings"]);
+	loadBuildings(randomFaction->town.get(), randomFactionJson["random"]["town"]["buildings"]);
 }
 
 void CTownHandler::loadCustom()

lib/entities/faction/CTownHandler.h

@@ -63,8 +63,7 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase<FactionID, Faction, CFactio
 	void loadRandomFaction();
 
 public:
-	CTown * randomTown;
-	CFaction * randomFaction;
+	std::unique_ptr<CFaction> randomFaction;
 
 	CTownHandler();
 	~CTownHandler();

lib/filesystem/AdapterLoaders.cpp

@@ -157,11 +157,12 @@ std::vector<const ISimpleResourceLoader *> CFilesystemList::getResourcesWithName
 	return ret;
 }
 
-void CFilesystemList::addLoader(ISimpleResourceLoader * loader, bool writeable)
+void CFilesystemList::addLoader(std::unique_ptr<ISimpleResourceLoader> loader, bool writeable)
 {
-	loaders.push_back(std::unique_ptr<ISimpleResourceLoader>(loader));
 	if (writeable)
-		writeableLoaders.insert(loader);
+		writeableLoaders.insert(loader.get());
+
+	loaders.push_back(std::move(loader));
 }
 
 bool CFilesystemList::removeLoader(ISimpleResourceLoader * loader)

lib/filesystem/AdapterLoaders.h

@@ -85,8 +85,8 @@ public:
 	 * @param loader The simple resource loader object to add
 	 * @param writeable - resource shall be treated as writeable
 	 */
-	void addLoader(ISimpleResourceLoader * loader, bool writeable);
-	
+	void addLoader(std::unique_ptr<ISimpleResourceLoader> loader, bool writeable);
+
 	/**
 	 * Removes loader from the loader list
 	 * Take care about memory deallocation

lib/filesystem/CCompressedStream.cpp

@@ -97,7 +97,7 @@ CCompressedStream::CCompressedStream(std::unique_ptr<CInputStream> stream, bool
 	assert(gzipStream);
 
 	// Allocate inflate state
-	inflateState = new z_stream();
+	inflateState = std::make_unique<z_stream>();
 	inflateState->zalloc = Z_NULL;
 	inflateState->zfree = Z_NULL;
 	inflateState->opaque = Z_NULL;
@@ -108,15 +108,14 @@ CCompressedStream::CCompressedStream(std::unique_ptr<CInputStream> stream, bool
 	if (gzip)
 		wbits += 16;
 
-	int ret = inflateInit2(inflateState, wbits);
+	int ret = inflateInit2(inflateState.get(), wbits);
 	if (ret != Z_OK)
 		throw std::runtime_error("Failed to initialize inflate!\n");
 }
 
 CCompressedStream::~CCompressedStream()
 {
-	inflateEnd(inflateState);
-	vstd::clear_pointer(inflateState);
+	inflateEnd(inflateState.get());
 }
 
 si64 CCompressedStream::readMore(ui8 *data, si64 size)
@@ -149,7 +148,7 @@ si64 CCompressedStream::readMore(ui8 *data, si64 size)
 			inflateState->next_in  = compressedBuffer.data();
 		}
 
-		int ret = inflate(inflateState, Z_NO_FLUSH);
+		int ret = inflate(inflateState.get(), Z_NO_FLUSH);
 
 		if (inflateState->avail_in == 0 && gzipStream == nullptr)
 			fileEnded = true;
@@ -179,8 +178,8 @@ si64 CCompressedStream::readMore(ui8 *data, si64 size)
 	// Clean up and return
 	if (fileEnded)
 	{
-		inflateEnd(inflateState);
-		vstd::clear_pointer(inflateState);
+		inflateEnd(inflateState.get());
+		inflateState.reset();
 	}
 	return decompressed;
 }
@@ -190,7 +189,7 @@ bool CCompressedStream::getNextBlock()
 	if (!inflateState)
 		return false;
 
-	if (inflateReset(inflateState) < 0)
+	if (inflateReset(inflateState.get()) < 0)
 		return false;
 
 	reset();

lib/filesystem/CCompressedStream.h

@@ -132,7 +132,7 @@ private:
 	std::vector<ui8> compressedBuffer;
 
 	/** struct with current zlib inflate state */
-	z_stream_s * inflateState;
+	std::unique_ptr<z_stream_s> inflateState;
 
 	enum EState
 	{

lib/filesystem/Filesystem.cpp

@@ -28,12 +28,14 @@ std::map<std::string, ISimpleResourceLoader*> CResourceHandler::knownLoaders = s
 CResourceHandler CResourceHandler::globalResourceHandler;
 
 CFilesystemGenerator::CFilesystemGenerator(std::string prefix, bool extractArchives):
-	filesystem(new CFilesystemList()),
+	filesystem(std::make_unique<CFilesystemList>()),
 	prefix(std::move(prefix)),
 	extractArchives(extractArchives)
 {
 }
 
+CFilesystemGenerator::~CFilesystemGenerator() = default;
+
 CFilesystemGenerator::TLoadFunctorMap CFilesystemGenerator::genFunctorMap()
 {
 	TLoadFunctorMap map;
@@ -72,9 +74,9 @@ void CFilesystemGenerator::loadConfig(const JsonNode & config)
 	}
 }
 
-CFilesystemList * CFilesystemGenerator::getFilesystem()
+std::unique_ptr<CFilesystemList> CFilesystemGenerator::acquireFilesystem()
 {
-	return filesystem;
+	return std::move(filesystem);
 }
 
 void CFilesystemGenerator::loadDirectory(const std::string &mountPoint, const JsonNode & config)
@@ -89,7 +91,7 @@ void CFilesystemGenerator::loadDirectory(const std::string &mountPoint, const Js
 	for(auto & loader : CResourceHandler::get("initial")->getResourcesWithName(resID))
 	{
 		auto filename = loader->getResourceName(resID);
-		filesystem->addLoader(new CFilesystemLoader(mountPoint, *filename, depth), false);
+		filesystem->addLoader(std::make_unique<CFilesystemLoader>(mountPoint, *filename, depth), false);
 	}
 }
 
@@ -98,7 +100,7 @@ void CFilesystemGenerator::loadZipArchive(const std::string &mountPoint, const J
 	std::string URI = prefix + config["path"].String();
 	auto filename = CResourceHandler::get("initial")->getResourceName(ResourcePath(URI, EResType::ARCHIVE_ZIP));
 	if (filename)
-		filesystem->addLoader(new CZipLoader(mountPoint, *filename), false);
+		filesystem->addLoader(std::make_unique<CZipLoader>(mountPoint, *filename), false);
 }
 
 template<EResType archiveType>
@@ -107,7 +109,7 @@ void CFilesystemGenerator::loadArchive(const std::string &mountPoint, const Json
 	std::string URI = prefix + config["path"].String();
 	auto filename = CResourceHandler::get("initial")->getResourceName(ResourcePath(URI, archiveType));
 	if (filename)
-		filesystem->addLoader(new CArchiveLoader(mountPoint, *filename, extractArchives), false);
+		filesystem->addLoader(std::make_unique<CArchiveLoader>(mountPoint, *filename, extractArchives), false);
 }
 
 void CFilesystemGenerator::loadJsonMap(const std::string &mountPoint, const JsonNode & config)
@@ -118,15 +120,15 @@ void CFilesystemGenerator::loadJsonMap(const std::string &mountPoint, const Json
 	{
 		auto configData = CResourceHandler::get("initial")->load(JsonPath::builtin(URI))->readAll();
 		const JsonNode configInitial(reinterpret_cast<std::byte *>(configData.first.get()), configData.second, URI);
-		filesystem->addLoader(new CMappedFileLoader(mountPoint, configInitial), false);
+		filesystem->addLoader(std::make_unique<CMappedFileLoader>(mountPoint, configInitial), false);
 	}
 }
 
-ISimpleResourceLoader * CResourceHandler::createInitial()
+std::unique_ptr<ISimpleResourceLoader> CResourceHandler::createInitial()
 {
 	//temporary filesystem that will be used to initialize main one.
 	//used to solve several case-sensivity issues like Mp3 vs MP3
-	auto * initialLoader = new CFilesystemList();
+	auto initialLoader = std::make_unique<CFilesystemList>();
 
 	//recurse only into specific directories
 	auto recurseInDir = [&](const std::string & URI, int depth)
@@ -138,8 +140,8 @@ ISimpleResourceLoader * CResourceHandler::createInitial()
 			auto filename = loader->getResourceName(ID);
 			if (filename)
 			{
-				auto * dir = new CFilesystemLoader(URI + '/', *filename, depth, true);
-				initialLoader->addLoader(dir, false);
+				auto dir = std::make_unique<CFilesystemLoader>(URI + '/', *filename, depth, true);
+				initialLoader->addLoader(std::move(dir), false);
 			}
 		}
 	};
@@ -147,9 +149,9 @@ ISimpleResourceLoader * CResourceHandler::createInitial()
 	for (auto & path : VCMIDirs::get().dataPaths())
 	{
 		if (boost::filesystem::is_directory(path)) // some of system-provided paths may not exist
-			initialLoader->addLoader(new CFilesystemLoader("", path, 1, true), false);
+			initialLoader->addLoader(std::make_unique<CFilesystemLoader>("", path, 1, true), false);
 	}
-	initialLoader->addLoader(new CFilesystemLoader("", VCMIDirs::get().userDataPath(), 0, true), false);
+	initialLoader->addLoader(std::make_unique<CFilesystemLoader>("", VCMIDirs::get().userDataPath(), 0, true), false);
 
 	recurseInDir("CONFIG", 0);// look for configs
 	recurseInDir("DATA", 0); // look for archives
@@ -178,18 +180,21 @@ void CResourceHandler::initialize()
 	if (globalResourceHandler.rootLoader)
 		return;
 
+	auto savesLoader = std::make_unique<CFilesystemLoader>("SAVES/", VCMIDirs::get().userSavePath());
+	auto configLoader = std::make_unique<CFilesystemLoader>("CONFIG/", VCMIDirs::get().userConfigPath());
+
 	globalResourceHandler.rootLoader = std::make_unique<CFilesystemList>();
 	knownLoaders["root"] = globalResourceHandler.rootLoader.get();
-	knownLoaders["saves"] = new CFilesystemLoader("SAVES/", VCMIDirs::get().userSavePath());
-	knownLoaders["config"] = new CFilesystemLoader("CONFIG/", VCMIDirs::get().userConfigPath());
+	knownLoaders["saves"] = savesLoader.get();
+	knownLoaders["config"] = configLoader.get();
 
-	auto * localFS = new CFilesystemList();
-	localFS->addLoader(knownLoaders["saves"], true);
-	localFS->addLoader(knownLoaders["config"], true);
+	auto localFS = std::make_unique<CFilesystemList>();
+	localFS->addLoader(std::move(savesLoader), true);
+	localFS->addLoader(std::move(configLoader), true);
 
 	addFilesystem("root", "initial", createInitial());
-	addFilesystem("root", "data", new CFilesystemList());
-	addFilesystem("root", "local", localFS);
+	addFilesystem("root", "data", std::make_unique<CFilesystemList>());
+	addFilesystem("root", "local", std::move(localFS));
 }
 
 void CResourceHandler::destroy()
@@ -218,26 +223,24 @@ void CResourceHandler::load(const std::string &fsConfigURI, bool extractArchives
 	addFilesystem("data", ModScope::scopeBuiltin(), createFileSystem("", fsConfig["filesystem"], extractArchives));
 }
 
-void CResourceHandler::addFilesystem(const std::string & parent, const std::string & identifier, ISimpleResourceLoader * loader)
+void CResourceHandler::addFilesystem(const std::string & parent, const std::string & identifier, std::unique_ptr<ISimpleResourceLoader> loader)
 {
 	if(knownLoaders.count(identifier) != 0)
 	{
 		logMod->error("[CRITICAL] Virtual filesystem %s already loaded!", identifier);
-		delete loader;
 		return;
 	}
 
 	if(knownLoaders.count(parent) == 0)
 	{
 		logMod->error("[CRITICAL] Parent virtual filesystem %s for %s not found!", parent, identifier);
-		delete loader;
 		return;
 	}
 
 	auto * list = dynamic_cast<CFilesystemList *>(knownLoaders.at(parent));
 	assert(list);
-	list->addLoader(loader, false);
-	knownLoaders[identifier] = loader;
+	knownLoaders[identifier] = loader.get();
+	list->addLoader(std::move(loader), false);
 }
 
 bool CResourceHandler::removeFilesystem(const std::string & parent, const std::string & identifier)
@@ -255,11 +258,11 @@ bool CResourceHandler::removeFilesystem(const std::string & parent, const std::s
 	return true;
 }
 
-ISimpleResourceLoader * CResourceHandler::createFileSystem(const std::string & prefix, const JsonNode &fsConfig, bool extractArchives)
+std::unique_ptr<ISimpleResourceLoader> CResourceHandler::createFileSystem(const std::string & prefix, const JsonNode &fsConfig, bool extractArchives)
 {
 	CFilesystemGenerator generator(prefix, extractArchives);
 	generator.loadConfig(fsConfig);
-	return generator.getFilesystem();
+	return generator.acquireFilesystem();
 }
 
 VCMI_LIB_NAMESPACE_END

lib/filesystem/Filesystem.h

@@ -19,12 +19,12 @@ class CFilesystemList;
 class JsonNode;
 
 /// Helper class that allows generation of a ISimpleResourceLoader entry out of Json config(s)
-class DLL_LINKAGE CFilesystemGenerator
+class DLL_LINKAGE CFilesystemGenerator : boost::noncopyable
 {
 	using TLoadFunctor = std::function<void(const std::string &, const JsonNode &)>;
 	using TLoadFunctorMap = std::map<std::string, TLoadFunctor>;
 
-	CFilesystemList * filesystem;
+	std::unique_ptr<CFilesystemList> filesystem;
 	std::string prefix;
 
 	template<EResType archiveType>
@@ -38,13 +38,14 @@ public:
 	/// prefix = prefix that will be given to file entries in all nodes of this filesystem
 	/// extractArchives = Specifies if Original H3 archives should be extracted to a separate folder
 	CFilesystemGenerator(std::string prefix, bool extractArchives = false);
+	~CFilesystemGenerator();
 
 	/// loads configuration from json
 	/// config - configuration to load, using format of "filesystem" entry in config/filesystem.json
 	void loadConfig(const JsonNode & config);
 
 	/// returns generated filesystem
-	CFilesystemList * getFilesystem();
+	std::unique_ptr<CFilesystemList> acquireFilesystem();
 
 	/** Specifies if Original H3 archives should be extracted to a separate folder **/
 	bool extractArchives;
@@ -61,7 +62,7 @@ class DLL_LINKAGE CResourceHandler
 	 * @brief createInitial - creates instance of initial loader
 	 * that contains data necessary to load main FS
 	 */
-	static ISimpleResourceLoader * createInitial();
+	static std::unique_ptr<ISimpleResourceLoader> createInitial();
 
 public:
 	/**
@@ -98,7 +99,7 @@ public:
 	 * @param identifier name of this loader by which it can be retrieved later
 	 * @param loader resource loader to add
 	 */
-	static void addFilesystem(const std::string & parent, const std::string & identifier, ISimpleResourceLoader * loader);
+	static void addFilesystem(const std::string & parent, const std::string & identifier, std::unique_ptr<ISimpleResourceLoader> loader);
 	
 	/**
 	 * @brief removeFilesystem removes previously added filesystem from global resource holder
@@ -114,7 +115,7 @@ public:
 	 * @param fsConfig - configuration to load
 	 * @return generated filesystem that contains all config entries
 	 */
-	static ISimpleResourceLoader * createFileSystem(const std::string &prefix, const JsonNode & fsConfig, bool extractArchives = false);
+	static std::unique_ptr<ISimpleResourceLoader> createFileSystem(const std::string &prefix, const JsonNode & fsConfig, bool extractArchives = false);
 
 	~CResourceHandler() = default;
 private:

lib/gameState/InfoAboutArmy.cpp

@@ -71,10 +71,10 @@ void InfoAboutArmy::initFromArmy(const CArmedInstance *Army, bool detailed)
 
 void InfoAboutHero::assign(const InfoAboutHero & iah)
 {
-	vstd::clear_pointer(details);
+	details.reset();;
 	InfoAboutArmy::operator = (iah);
 
-	details = (iah.details ? new Details(*iah.details) : nullptr);
+	details = iah.details;
 	hclass = iah.hclass;
 	portraitSource = iah.portraitSource;
 }
@@ -92,11 +92,6 @@ InfoAboutHero::InfoAboutHero(const CGHeroInstance * h, InfoAboutHero::EInfoLevel
 	initFromHero(h, infoLevel);
 }
 
-InfoAboutHero::~InfoAboutHero()
-{
-	vstd::clear_pointer(details);
-}
-
 InfoAboutHero & InfoAboutHero::operator=(const InfoAboutHero & iah)
 {
 	assign(iah);
@@ -110,7 +105,7 @@ int32_t InfoAboutHero::getIconIndex() const
 
 void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLevel infoLevel)
 {
-	vstd::clear_pointer(details);
+	details.reset();
 	if(!h)
 		return;
 
@@ -125,7 +120,7 @@ void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLe
 	if(detailed)
 	{
 		//include details about hero
-		details = new Details();
+		details = Details();
 		details->luck = h->luckVal();
 		details->morale = h->moraleVal();
 		details->mana = h->mana;
@@ -143,7 +138,6 @@ void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLe
 }
 
 InfoAboutTown::InfoAboutTown():
-	details(nullptr),
 	tType(nullptr),
 	built(0),
 	fortLevel(0)
@@ -152,7 +146,6 @@ InfoAboutTown::InfoAboutTown():
 }
 
 InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed):
-	details(nullptr),
 	tType(nullptr),
 	built(0),
 	fortLevel(0)
@@ -160,11 +153,6 @@ InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed):
 	initFromTown(t, detailed);
 }
 
-InfoAboutTown::~InfoAboutTown()
-{
-	vstd::clear_pointer(details);
-}
-
 void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 {
 	initFromArmy(t, detailed);
@@ -174,12 +162,12 @@ void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 	name = t->getNameTranslated();
 	tType = t->getTown();
 
-	vstd::clear_pointer(details);
+	details.reset();
 
 	if(detailed)
 	{
 		//include details about hero
-		details = new Details();
+		details = Details();
 		TResources income = t->dailyIncome();
 		details->goldIncome = income[EGameResID::GOLD];
 		details->customRes = t->hasBuilt(BuildingID::RESOURCE_SILO);

lib/gameState/InfoAboutArmy.h

@@ -52,7 +52,7 @@ public:
 		si32 mana, manaLimit, luck, morale;
 	};
 
-	Details * details = nullptr;
+	std::optional<Details> details;
 
 	const CHeroClass *hclass;
 	HeroTypeID portraitSource;
@@ -67,7 +67,6 @@ public:
 	InfoAboutHero();
 	InfoAboutHero(const InfoAboutHero & iah);
 	InfoAboutHero(const CGHeroInstance *h, EInfoLevel infoLevel);
-	~InfoAboutHero();
 
 	InfoAboutHero & operator=(const InfoAboutHero & iah);
 
@@ -84,7 +83,9 @@ struct DLL_LINKAGE InfoAboutTown : public InfoAboutArmy
 		bool customRes;
 		bool garrisonedHero;
 
-	} *details;
+	};
+
+	std::optional<Details> details;
 
 	const CTown *tType;
 
@@ -93,7 +94,6 @@ struct DLL_LINKAGE InfoAboutTown : public InfoAboutArmy
 
 	InfoAboutTown();
 	InfoAboutTown(const CGTownInstance *t, bool detailed);
-	~InfoAboutTown();
 	void initFromTown(const CGTownInstance *t, bool detailed);
 };
 

lib/mapObjects/CGTownInstance.cpp

@@ -1145,9 +1145,9 @@ const CFaction * CGTownInstance::getFaction() const
 const CTown * CGTownInstance::getTown() const
 {
 	if(ID == Obj::RANDOM_TOWN)
-		return LIBRARY->townh->randomTown;
+		return LIBRARY->townh->randomFaction->town.get();
 
-	return getFaction()->town;
+	return getFaction()->town.get();
 }
 
 FactionID CGTownInstance::getFactionID() const

lib/modding/CModHandler.cpp

@@ -71,7 +71,7 @@ static std::string getModDirectory(const TModID & modName)
 	return "MODS/" + result;
 }
 
-static ISimpleResourceLoader * genModFilesystem(const std::string & modName, const JsonNode & conf)
+static std::unique_ptr<ISimpleResourceLoader> genModFilesystem(const std::string & modName, const JsonNode & conf)
 {
 	static const JsonNode defaultFS = genDefaultFS();
 
@@ -87,20 +87,20 @@ void CModHandler::loadModFilesystems()
 
 	const auto & activeMods = modManager->getActiveMods();
 
-	std::map<TModID, ISimpleResourceLoader *> modFilesystems;
+	std::map<TModID, std::unique_ptr<ISimpleResourceLoader>> modFilesystems;
 
 	for(const TModID & modName : activeMods)
 		modFilesystems[modName] = genModFilesystem(modName, getModInfo(modName).getFilesystemConfig());
 
-	for(const TModID & modName : activeMods)
-		if (modName != "core") // virtual mod 'core' has no filesystem on its own - shared with base install
-			CResourceHandler::addFilesystem("data", modName, modFilesystems[modName]);
-
 	if (settings["mods"]["validation"].String() == "full")
 		checkModFilesystemsConflicts(modFilesystems);
+
+	for(const TModID & modName : activeMods)
+		if (modName != "core") // virtual mod 'core' has no filesystem on its own - shared with base install
+			CResourceHandler::addFilesystem("data", modName, std::move(modFilesystems[modName]));
 }
 
-void CModHandler::checkModFilesystemsConflicts(const std::map<TModID, ISimpleResourceLoader *> & modFilesystems)
+void CModHandler::checkModFilesystemsConflicts(const std::map<TModID, std::unique_ptr<ISimpleResourceLoader>> & modFilesystems)
 {
 	for(const auto & [leftName, leftFilesystem] : modFilesystems)
 	{

lib/modding/CModHandler.h

@@ -27,7 +27,7 @@ class DLL_LINKAGE CModHandler final : boost::noncopyable
 	std::set<std::string> validationPassed;
 
 	void loadTranslation(const TModID & modName);
-	void checkModFilesystemsConflicts(const std::map<TModID, ISimpleResourceLoader *> & modFilesystems);
+	void checkModFilesystemsConflicts(const std::map<TModID, std::unique_ptr<ISimpleResourceLoader>> & modFilesystems);
 
 	bool isModValidationNeeded(const ModDescription & mod) const;
 

lib/network/NetworkConnection.cpp

@@ -12,9 +12,9 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-NetworkConnection::NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, const std::shared_ptr<NetworkContext> & context)
+NetworkConnection::NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, NetworkContext & context)
 	: socket(socket)
-	, timer(std::make_shared<NetworkTimer>(*context))
+	, timer(std::make_shared<NetworkTimer>(context))
 	, listener(listener)
 {
 	socket->set_option(boost::asio::ip::tcp::no_delay(true));
@@ -208,7 +208,7 @@ void NetworkConnection::close()
 	//NOTE: ignoring error code, intended
 }
 
-InternalConnection::InternalConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkContext> & context)
+InternalConnection::InternalConnection(INetworkConnectionListener & listener, NetworkContext & context)
 	: io(context)
 	, listener(listener)
 {
@@ -216,7 +216,7 @@ InternalConnection::InternalConnection(INetworkConnectionListener & listener, co
 
 void InternalConnection::receivePacket(const std::vector<std::byte> & message)
 {
-	boost::asio::post(*io, [self = std::static_pointer_cast<InternalConnection>(shared_from_this()), message](){
+	boost::asio::post(io, [self = std::static_pointer_cast<InternalConnection>(shared_from_this()), message](){
 		if (self->connectionActive)
 			self->listener.onPacketReceived(self, message);
 	});
@@ -224,7 +224,7 @@ void InternalConnection::receivePacket(const std::vector<std::byte> & message)
 
 void InternalConnection::disconnect()
 {
-	boost::asio::post(*io, [self = std::static_pointer_cast<InternalConnection>(shared_from_this())](){
+	boost::asio::post(io, [self = std::static_pointer_cast<InternalConnection>(shared_from_this())](){
 		self->listener.onDisconnected(self, "Internal connection has been terminated");
 		self->otherSideWeak.reset();
 		self->connectionActive = false;

lib/network/NetworkConnection.h

@@ -38,7 +38,7 @@ class NetworkConnection final : public INetworkConnection, public std::enable_sh
 	void onDataSent(const boost::system::error_code & ec);
 
 public:
-	NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, const std::shared_ptr<NetworkContext> & context);
+	NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, NetworkContext & context);
 
 	void start();
 	void close() override;
@@ -49,11 +49,11 @@ public:
 class InternalConnection final : public IInternalConnection, public std::enable_shared_from_this<InternalConnection>
 {
 	std::weak_ptr<IInternalConnection> otherSideWeak;
-	std::shared_ptr<NetworkContext> io;
+	NetworkContext & io;
 	INetworkConnectionListener & listener;
 	bool connectionActive = false;
 public:
-	InternalConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkContext> & context);
+	InternalConnection(INetworkConnectionListener & listener, NetworkContext & context);
 
 	void receivePacket(const std::vector<std::byte> & message) override;
 	void disconnect() override;

lib/network/NetworkHandler.cpp

@@ -21,12 +21,12 @@ std::unique_ptr<INetworkHandler> INetworkHandler::createHandler()
 }
 
 NetworkHandler::NetworkHandler()
-	: io(std::make_shared<NetworkContext>())
+	: io(std::make_unique<NetworkContext>())
 {}
 
 std::unique_ptr<INetworkServer> NetworkHandler::createServerTCP(INetworkServerListener & listener)
 {
-	return std::make_unique<NetworkServer>(listener, io);
+	return std::make_unique<NetworkServer>(listener, *io);
 }
 
 void NetworkHandler::connectToRemote(INetworkClientListener & listener, const std::string & host, uint16_t port)
@@ -50,7 +50,7 @@ void NetworkHandler::connectToRemote(INetworkClientListener & listener, const st
 				listener.onConnectionFailed(error.message());
 				return;
 			}
-			auto connection = std::make_shared<NetworkConnection>(listener, socket, io);
+			auto connection = std::make_shared<NetworkConnection>(listener, socket, *io);
 			connection->start();
 
 			listener.onConnectionEstablished(connection);
@@ -75,7 +75,7 @@ void NetworkHandler::createTimer(INetworkTimerListener & listener, std::chrono::
 
 void NetworkHandler::createInternalConnection(INetworkClientListener & listener, INetworkServer & server)
 {
-	auto localConnection = std::make_shared<InternalConnection>(listener, io);
+	auto localConnection = std::make_shared<InternalConnection>(listener, *io);
 
 	server.receiveInternalConnection(localConnection);
 

lib/network/NetworkHandler.h

@@ -15,7 +15,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class NetworkHandler : public INetworkHandler
 {
-	std::shared_ptr<NetworkContext> io;
+	std::unique_ptr<NetworkContext> io;
 
 public:
 	NetworkHandler();

lib/network/NetworkServer.cpp

@@ -13,7 +13,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-NetworkServer::NetworkServer(INetworkServerListener & listener, const std::shared_ptr<NetworkContext> & context)
+NetworkServer::NetworkServer(INetworkServerListener & listener, NetworkContext & context)
 	: io(context)
 	, listener(listener)
 {
@@ -21,13 +21,13 @@ NetworkServer::NetworkServer(INetworkServerListener & listener, const std::share
 
 uint16_t NetworkServer::start(uint16_t port)
 {
-	acceptor = std::make_shared<NetworkAcceptor>(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port));
+	acceptor = std::make_shared<NetworkAcceptor>(io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port));
 	return startAsyncAccept();
 }
 
 uint16_t NetworkServer::startAsyncAccept()
 {
-	auto upcomingConnection = std::make_shared<NetworkSocket>(*io);
+	auto upcomingConnection = std::make_shared<NetworkSocket>(io);
 	acceptor->async_accept(*upcomingConnection, [this, upcomingConnection](const auto & ec) { connectionAccepted(upcomingConnection, ec); });
 	return acceptor->local_endpoint().port();
 }

lib/network/NetworkServer.h

@@ -15,7 +15,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class NetworkServer : public INetworkConnectionListener, public INetworkServer
 {
-	std::shared_ptr<NetworkContext> io;
+	NetworkContext & io;
 	std::shared_ptr<NetworkAcceptor> acceptor;
 	std::set<std::shared_ptr<INetworkConnection>> connections;
 
@@ -27,7 +27,7 @@ class NetworkServer : public INetworkConnectionListener, public INetworkServer
 	void onDisconnected(const std::shared_ptr<INetworkConnection> & connection, const std::string & errorMessage) override;
 	void onPacketReceived(const std::shared_ptr<INetworkConnection> & connection, const std::vector<std::byte> & message) override;
 public:
-	NetworkServer(INetworkServerListener & listener, const std::shared_ptr<NetworkContext> & context);
+	NetworkServer(INetworkServerListener & listener, NetworkContext & context);
 
 	void receiveInternalConnection(std::shared_ptr<IInternalConnection> remoteConnection) override;
 

mapeditor/helper.cpp

@@ -32,9 +32,9 @@ std::unique_ptr<CMap> Helper::openMapInternal(const QString & filenameSelect)
 	ResourcePath resId("MAPEDITOR/" + fname, EResType::MAP);
 	
 	//addFilesystem takes care about memory deallocation if case of failure, no memory leak here
-	auto * mapEditorFilesystem = new CFilesystemLoader("MAPEDITOR/", fdir, 0);
+	auto mapEditorFilesystem = std::make_unique<CFilesystemLoader>("MAPEDITOR/", fdir, 0);
 	CResourceHandler::removeFilesystem("local", "mapEditor");
-	CResourceHandler::addFilesystem("local", "mapEditor", mapEditorFilesystem);
+	CResourceHandler::addFilesystem("local", "mapEditor", std::move(mapEditorFilesystem));
 	
 	if(!CResourceHandler::get("mapEditor")->existsResource(resId))
 		throw std::runtime_error("Cannot open map from this folder");
@@ -65,9 +65,9 @@ std::shared_ptr<CampaignState> Helper::openCampaignInternal(const QString & file
 	ResourcePath resId("MAPEDITOR/" + fname, EResType::CAMPAIGN);
 	
 	//addFilesystem takes care about memory deallocation if case of failure, no memory leak here
-	auto * mapEditorFilesystem = new CFilesystemLoader("MAPEDITOR/", fdir, 0);
+	auto mapEditorFilesystem = std::make_unique<CFilesystemLoader>("MAPEDITOR/", fdir, 0);
 	CResourceHandler::removeFilesystem("local", "mapEditor");
-	CResourceHandler::addFilesystem("local", "mapEditor", mapEditorFilesystem);
+	CResourceHandler::addFilesystem("local", "mapEditor", std::move(mapEditorFilesystem));
 	
 	if(!CResourceHandler::get("mapEditor")->existsResource(resId))
 		throw std::runtime_error("Cannot open campaign from this folder");

mapeditor/mapsettings/victoryconditions.cpp

@@ -424,7 +424,7 @@ void VictoryConditions::on_victoryComboBox_currentIndexChanged(int index)
 		case 3: { //EventCondition::HAVE_BUILDING
 			victoryTypeWidget = new QComboBox;
 			ui->victoryParamsLayout->addWidget(victoryTypeWidget);
-			auto * ctown = LIBRARY->townh->randomTown;
+			auto * ctown = LIBRARY->townh->randomFaction->town.get();
 			for(int bId : ctown->getAllBuildings())
 				victoryTypeWidget->addItem(QString::fromStdString(defaultBuildingIdConversion(BuildingID(bId))), QVariant::fromValue(bId));
 

serverapp/EntryPoint.cpp

@@ -100,7 +100,7 @@ int main(int argc, const char * argv[])
 	}
 
 	logConfigurator.deconfigure();
-	vstd::clear_pointer(LIBRARY);
 
+	delete LIBRARY;
 	return 0;
 }

test/CVcmiTestConfig.cpp

@@ -32,14 +32,14 @@ void CVcmiTestConfig::SetUp()
 	auto path = boost::filesystem::current_path();
 	path+= "/" + TEST_DATA_DIR;
 	if(boost::filesystem::exists(path)){
-		auto loader = new CFilesystemLoader("test/", TEST_DATA_DIR);
-		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(loader, false);
+		auto loader = std::make_unique<CFilesystemLoader>("test/", TEST_DATA_DIR);
+		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(std::move(loader), false);
 
-		loader = new CFilesystemLoader("scripts/test/erm/", TEST_DATA_DIR+"erm/");
-		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(loader, false);
+		loader = std::make_unique<CFilesystemLoader>("scripts/test/erm/", TEST_DATA_DIR+"erm/");
+		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(std::move(loader), false);
 
-		loader = new CFilesystemLoader("scripts/test/lua/", TEST_DATA_DIR+"lua/");
-		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(loader, false);
+		loader = std::make_unique<CFilesystemLoader>("scripts/test/lua/", TEST_DATA_DIR+"lua/");
+		dynamic_cast<CFilesystemList*>(CResourceHandler::get("core"))->addLoader(std::move(loader), false);
 
 	}
 }

test/entity/CFactionTest.cpp

@@ -34,9 +34,9 @@ protected:
 TEST_F(CFactionTest, HasTown)
 {
 	EXPECT_FALSE(subject->hasTown());
-	subject->town = new CTown();
+	subject->town = std::make_unique<CTown>();
 	EXPECT_TRUE(subject->hasTown());
-	vstd::clear_pointer(subject->town);
+	subject->town.reset();
 	EXPECT_FALSE(subject->hasTown());
 }
 
@@ -56,7 +56,7 @@ TEST_F(CFactionTest, RegistersIcons)
 		registarCb(std::forward<decltype(PH1)>(PH1), std::forward<decltype(PH2)>(PH2), std::forward<decltype(PH3)>(PH3), std::forward<decltype(PH4)>(PH4));
 	};
 
-	subject->town = new CTown();
+	subject->town = std::make_unique<CTown>();
 
 	CTown::ClientInfo & info = subject->town->clientInfo;