Fix various memory errors related to server and connections.

* keep connection when restarting scenario
* wrong double free in Connection
* multiple use after free when stopping server
* double free of StartInfo

client/CServerHandler.cpp

@@ -108,7 +108,7 @@ void CServerHandler::resetStateForLobby(const StartInfo::EMode mode, const std::
 	th = make_unique<CStopWatch>();
 	packsForLobbyScreen.clear();
 	c.reset();
-	si.reset(new StartInfo());
+	si = std::make_shared<StartInfo>();
 	playerNames.clear();
 	si->difficulty = 1;
 	si->mode = mode;
@@ -475,11 +475,13 @@ void CServerHandler::endGameplay(bool closeConnection)
 	client->endGame();
 	vstd::clear_pointer(client);
 
+	if(closeConnection)
+	{
 		// Game is ending
 		// Tell the network thread to reach a stable state
 		CSH->sendClientDisconnecting();
 		logNetwork->info("Closed connection.");
-
+	}
 	if(CMM)
 	{
 		GH.curInt = CMM;

lib/serializer/Connection.cpp

@@ -58,7 +58,7 @@ void CConnection::init()
 }
 
 CConnection::CConnection(std::string host, ui16 port, std::string Name, std::string UUID)
-	: iser(this), oser(this), io_service(std::make_shared<asio::io_service>()), connectionID(0), name(Name), uuid(UUID)
+	: io_service(std::make_shared<asio::io_service>()), iser(this), oser(this), name(Name), uuid(UUID), connectionID(0)
 {
 	int i;
 	boost::system::error_code error = asio::error::host_not_found;
@@ -111,12 +111,12 @@ connerror1:
 	throw std::runtime_error("Can't establish connection :(");
 }
 CConnection::CConnection(std::shared_ptr<TSocket> Socket, std::string Name, std::string UUID)
-	: iser(this), oser(this), socket(Socket), io_service(&Socket->get_io_service()), connectionID(0), name(Name), uuid(UUID)
+	: iser(this), oser(this), socket(Socket), name(Name), uuid(UUID), connectionID(0)
 {
 	init();
 }
-CConnection::CConnection(std::shared_ptr<TAcceptor> acceptor, std::shared_ptr<boost::asio::io_service> Io_service, std::string Name, std::string UUID)
-	: iser(this), oser(this), connectionID(0), name(Name), uuid(UUID)
+CConnection::CConnection(std::shared_ptr<TAcceptor> acceptor, std::shared_ptr<boost::asio::io_service> io_service, std::string Name, std::string UUID)
+	: io_service(io_service), iser(this), oser(this), name(Name), uuid(UUID), connectionID(0)
 {
 	boost::system::error_code error = asio::error::host_not_found;
 	socket = std::make_shared<tcp::socket>(*io_service);

lib/serializer/Connection.h

@@ -49,13 +49,13 @@ typedef boost::asio::basic_socket_acceptor<boost::asio::ip::tcp, boost::asio::so
 class DLL_LINKAGE CConnection
 	: public IBinaryReader, public IBinaryWriter, public std::enable_shared_from_this<CConnection>
 {
-	CConnection();
-
 	void init();
 	void reportState(vstd::CLoggerBase * out) override;
 
 	int write(const void * data, unsigned size) override;
 	int read(void * data, unsigned size) override;
+
+	std::shared_ptr<boost::asio::io_service> io_service; //can be empty if connection made from socket
 public:
 	BinaryDeserializer iser;
 	BinarySerializer oser;
@@ -66,7 +66,6 @@ public:
 	bool connected;
 	bool myEndianess, contactEndianess; //true if little endian, if endianness is different we'll have to revert received multi-byte vars
 	std::string contactUuid;
-	std::shared_ptr<boost::asio::io_service> io_service;
 	std::string name; //who uses this connection
 	std::string uuid;
 

server/CVCMIServer.cpp

@@ -41,6 +41,7 @@
 #include "../lib/logging/CBasicLogConfigurator.h"
 #include "../lib/CConfigHandler.h"
 #include "../lib/ScopeGuard.h"
+#include "../lib/serializer/CMemorySerializer.h"
 
 #include "../lib/UnlockGuard.h"
 
@@ -145,18 +146,17 @@ CVCMIServer::CVCMIServer(boost::program_options::variables_map & opts)
 
 CVCMIServer::~CVCMIServer()
 {
-
-	for(CPackForLobby * pack : announceQueue)
-		delete pack;
-
 	announceQueue.clear();
+
+	if(announceLobbyThread)
+		announceLobbyThread->join();
 }
 
 void CVCMIServer::run()
 {
 	if(!restartGameplay)
 	{
-		boost::thread(&CVCMIServer::threadAnnounceLobby, this);
+		this->announceLobbyThread = vstd::make_unique<boost::thread>(&CVCMIServer::threadAnnounceLobby, this);
 #ifndef VCMI_ANDROID
 		if(cmdLineOptions.count("enable-shm"))
 		{
@@ -195,7 +195,7 @@ void CVCMIServer::threadAnnounceLobby()
 			boost::unique_lock<boost::recursive_mutex> myLock(mx);
 			while(!announceQueue.empty())
 			{
-				announcePack(announceQueue.front());
+				announcePack(std::move(announceQueue.front()));
 				announceQueue.pop_front();
 			}
 			if(state != EServerState::LOBBY)
@@ -224,7 +224,7 @@ void CVCMIServer::prepareToStartGame()
 		// FIXME: dirry hack to make sure old CGameHandler::run is finished
 		boost::this_thread::sleep(boost::posix_time::milliseconds(1000));
 	}
-
+	state = EServerState::GAMEPLAY_STARTING;
 	gh = std::make_shared<CGameHandler>(this);
 	switch(si->mode)
 	{
@@ -306,7 +306,7 @@ void CVCMIServer::threadHandleClient(std::shared_ptr<CConnection> c)
 			CPack * pack = c->retrievePack();
 			if(auto lobbyPack = dynamic_ptr_cast<CPackForLobby>(pack))
 			{
-				handleReceivedPack(lobbyPack);
+				handleReceivedPack(std::unique_ptr<CPackForLobby>(lobbyPack));
 			}
 			else if(auto serverPack = dynamic_ptr_cast<CPackForServer>(pack))
 			{
@@ -333,56 +333,53 @@ void CVCMIServer::threadHandleClient(std::shared_ptr<CConnection> c)
 
 	boost::unique_lock<boost::recursive_mutex> queueLock(mx);
 //	if(state != ENDING_AND_STARTING_GAME)
+	if(c->connected)
 	{
-		auto lcd = new LobbyClientDisconnected();
+		auto lcd = vstd::make_unique<LobbyClientDisconnected>();
 		lcd->c = c;
 		lcd->clientId = c->connectionID;
-		handleReceivedPack(lcd);
+		handleReceivedPack(std::move(lcd));
 	}
 
 	logNetwork->info("Thread listening for %s ended", c->toString());
 	c->handler.reset();
 }
 
-void CVCMIServer::handleReceivedPack(CPackForLobby * pack)
+void CVCMIServer::handleReceivedPack(std::unique_ptr<CPackForLobby> pack)
 {
-	CBaseForServerApply * apply = applier->getApplier(typeList.getTypeID(pack));
-	if(apply->applyOnServerBefore(this, pack))
-		addToAnnounceQueue(pack);
-	else
-		delete pack;
+	CBaseForServerApply * apply = applier->getApplier(typeList.getTypeID(pack.get()));
+	if(apply->applyOnServerBefore(this, pack.get()))
+		addToAnnounceQueue(std::move(pack));
 }
 
-void CVCMIServer::announcePack(CPackForLobby * pack)
+void CVCMIServer::announcePack(std::unique_ptr<CPackForLobby> pack)
 {
 	for(auto c : connections)
 	{
 		// FIXME: we need to avoid senting something to client that not yet get answer for LobbyClientConnected
 		// Until UUID set we only pass LobbyClientConnected to this client
-		if(c->uuid == uuid && !dynamic_cast<LobbyClientConnected *>(pack))
+		if(c->uuid == uuid && !dynamic_cast<LobbyClientConnected *>(pack.get()))
 			continue;
 
-		c->sendPack(pack);
+		c->sendPack(pack.get());
 	}
 
-	applier->getApplier(typeList.getTypeID(pack))->applyOnServerAfter(this, pack);
-
-	delete pack;
+	applier->getApplier(typeList.getTypeID(pack.get()))->applyOnServerAfter(this, pack.get());
 }
 
 void CVCMIServer::announceTxt(const std::string & txt, const std::string & playerName)
 {
 	logNetwork->info("%s says: %s", playerName, txt);
-	auto cm = new LobbyChatMessage();
+	auto cm = vstd::make_unique<LobbyChatMessage>();
 	cm->playerName = playerName;
 	cm->message = txt;
-	addToAnnounceQueue(cm);
+	addToAnnounceQueue(std::move(cm));
 }
 
-void CVCMIServer::addToAnnounceQueue(CPackForLobby * pack)
+void CVCMIServer::addToAnnounceQueue(std::unique_ptr<CPackForLobby> pack)
 {
 	boost::unique_lock<boost::recursive_mutex> queueLock(mx);
-	announceQueue.push_back(pack);
+	announceQueue.push_back(std::move(pack));
 }
 
 bool CVCMIServer::passHost(int toConnectionId)
@@ -479,7 +476,7 @@ void CVCMIServer::updateStartInfoOnMapChange(std::shared_ptr<CMapInfo> mapInfo,
 	si->playerInfos.clear();
 	if(mi->scenarioOptionsOfSave)
 	{
-		si = std::shared_ptr<StartInfo>(mi->scenarioOptionsOfSave);
+		si = CMemorySerializer::deepCopy(*mi->scenarioOptionsOfSave);
 		si->mode = StartInfo::LOAD_GAME;
 		if(si->campState)
 			campaignMap = si->campState->currentMap.get();
@@ -563,9 +560,9 @@ void CVCMIServer::updateAndPropagateLobbyState()
 		}
 	}
 
-	auto lus = new LobbyUpdateState();
+	auto lus = vstd::make_unique<LobbyUpdateState>();
 	lus->state = *this;
-	addToAnnounceQueue(lus);
+	addToAnnounceQueue(std::move(lus));
 }
 
 void CVCMIServer::setPlayer(PlayerColor clickedColor)

server/CVCMIServer.h

@@ -44,9 +44,10 @@ class CVCMIServer : public LobbyInfo
 	std::shared_ptr<boost::asio::io_service> io;
 	std::shared_ptr<TAcceptor> acceptor;
 	std::shared_ptr<TSocket> upcomingConnection;
-	std::list<CPackForLobby *> announceQueue;
+	std::list<std::unique_ptr<CPackForLobby>> announceQueue;
 	boost::recursive_mutex mx;
 	std::shared_ptr<CApplier<CBaseForServerApply>> applier;
+	std::unique_ptr<boost::thread> announceLobbyThread;
 
 public:
 	std::shared_ptr<CGameHandler> gh;
@@ -69,13 +70,13 @@ public:
 	void connectionAccepted(const boost::system::error_code & ec);
 	void threadHandleClient(std::shared_ptr<CConnection> c);
 	void threadAnnounceLobby();
-	void handleReceivedPack(CPackForLobby * pack);
+	void handleReceivedPack(std::unique_ptr<CPackForLobby> pack);
 
-	void announcePack(CPackForLobby * pack);
+	void announcePack(std::unique_ptr<CPackForLobby> pack);
 	bool passHost(int toConnectionId);
 
 	void announceTxt(const std::string & txt, const std::string & playerName = "system");
-	void addToAnnounceQueue(CPackForLobby * pack);
+	void addToAnnounceQueue(std::unique_ptr<CPackForLobby> pack);
 
 	void setPlayerConnectedId(PlayerSettings & pset, ui8 player) const;
 	void updateStartInfoOnMapChange(std::shared_ptr<CMapInfo> mapInfo, std::shared_ptr<CMapGenOptions> mapGenOpt = {});

server/NetPacksLobbyServer.cpp

@@ -75,6 +75,8 @@ bool LobbyClientDisconnected::checkClientPermissions(CVCMIServer * srv) const
 bool LobbyClientDisconnected::applyOnServer(CVCMIServer * srv)
 {
 	srv->clientDisconnected(c);
+	c->close();
+	c->connected = false;
 	return true;
 }
 
@@ -100,10 +102,10 @@ void LobbyClientDisconnected::applyOnServerAfterAnnounce(CVCMIServer * srv)
 	}
 	else if(c == srv->hostClient)
 	{
-		auto ph = new LobbyChangeHost();
+		auto ph = vstd::make_unique<LobbyChangeHost>();
 		auto newHost = *RandomGeneratorUtil::nextItem(srv->connections, CRandomGenerator::getDefault());
 		ph->newHostConnectionId = newHost->connectionID;
-		srv->addToAnnounceQueue(ph);
+		srv->addToAnnounceQueue(std::move(ph));
 	}
 	srv->updateAndPropagateLobbyState();
 }
@@ -175,7 +177,6 @@ bool LobbyStartGame::applyOnServer(CVCMIServer * srv)
 		return false;
 	}
 	// Server will prepare gamestate and we announce StartInfo to clients
-	srv->state = EServerState::GAMEPLAY_STARTING;
 	srv->prepareToStartGame();
 	initializedStartInfo = std::make_shared<StartInfo>(*srv->gh->getStartInfo(true));
 	return true;