Refactoring of networking code on server and client

* Avoid server crash on dummy connect / disconnect.
* Avoid server crash when host left from PreGame.
* Server print it's state with name when it's waiting for connection or in pregame.
* Server will use random port if specified port is busy.

client/CMT.cpp

@@ -1273,8 +1273,11 @@ static void mainLoop()
 
 void startGame(StartInfo * options, CConnection *serv/* = nullptr*/)
 {
-	serverAlive.waitWhileTrue();
+	if(!CServerHandler::DO_NOT_START_SERVER)
-	serverAlive.setn(true);
+	{
+		serverAlive.waitWhileTrue();
+		serverAlive.setn(true);
+	}
 
 	if(vm.count("onlyAI"))
 	{

client/Client.cpp

@@ -708,16 +708,13 @@ void CClient::stopConnection()
 			connectionHandler->join();
 
 		logNetwork->infoStream() << "Connection handler thread joined";
-
+		vstd::clear_pointer(connectionHandler);
-		delete connectionHandler;
-		connectionHandler = nullptr;
 	}
 
 	if (serv) //and delete connection
 	{
 		serv->close();
-		delete serv;
+		vstd::clear_pointer(serv);
-		serv = nullptr;
 		logNetwork->warnStream() << "Our socket has been closed.";
 	}
 }
@@ -990,6 +987,7 @@ CConnection * CServerHandler::connectToServer()
 #ifndef VCMI_ANDROID
 	if(!shared->sr->ready)
 		waitForServer();
+	port = boost::lexical_cast<std::string>(shared->sr->port);
 #else
 	waitForServer();
 #endif

lib/Interprocess.h

@@ -19,19 +19,22 @@
 struct ServerReady
 {
 	bool ready;
+	uint16_t port; //ui16?
 	boost::interprocess::interprocess_mutex  mutex;
 	boost::interprocess::interprocess_condition  cond;
 
 	ServerReady()
 	{
 		ready = false;
+		port = 0;
 	}
 
-	void setToTrueAndNotify()
+	void setToTrueAndNotify(uint16_t Port)
 	{
 		{
 			boost::unique_lock<boost::interprocess::interprocess_mutex> lock(mutex); 
 			ready = true;
+			port = Port;
 		}
 		cond.notify_all();
 	}

lib/VCMI_lib.vcxproj

@@ -424,4 +424,4 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>

lib/VCMI_lib.vcxproj.filters

@@ -669,4 +669,4 @@
       <Filter>Header Files</Filter>
     </ClInclude>
   </ItemGroup>
-</Project>
+</Project>

lib/serializer/Connection.cpp

@@ -191,8 +191,7 @@ void CConnection::close()
 	if(socket)
 	{
 		socket->close();
-		delete socket;
+		vstd::clear_pointer(socket);
-		socket = nullptr;
 	}
 }
 

server/CGameHandler.cpp

@@ -2716,10 +2716,16 @@ void CGameHandler::close()
 	}
 	end2 = true;
 
-	//for (CConnection *cc : conns)
+	for (auto & elem : conns)
-	//	if (cc && cc->socket && cc->socket->is_open())
+	{
-	//		cc->socket->close();
+		if(!elem->isOpen())
-	//exit(0);
+			continue;
+
+		boost::unique_lock<boost::mutex> lock(*(elem)->wmx);
+		elem->close();
+		elem->connected = false;
+	}
+	exit(0);
 }
 
 bool CGameHandler::arrangeStacks(ObjectInstanceID id1, ObjectInstanceID id2, ui8 what, SlotID p1, SlotID p2, si32 val, PlayerColor player)

server/CVCMIServer.cpp

@@ -45,7 +45,6 @@ std::string NAME = GameConstants::VCMI_VERSION + std::string(" (") + NAME_AFFIX
 namespace intpr = boost::interprocess;
 #endif
 bool end2 = false;
-int port = 3030;
 
 boost::program_options::variables_map cmdLineOptions;
 
@@ -106,6 +105,10 @@ void CPregameServer::handleConnection(CConnection *cpc)
 				auto unlock = vstd::makeUnlockGuard(mx);
 				while(state == RUNNING) boost::this_thread::sleep(boost::posix_time::milliseconds(50));
 			}
+			else if(quitting) // Server must be stopped if host is leaving from lobby to avoid crash
+			{
+				end2 = true;
+			}
 		}
 	}
 	catch (const std::exception& e)
@@ -206,20 +209,29 @@ void CPregameServer::connectionAccepted(const boost::system::error_code& ec)
 		return;
 	}
 
-	logNetwork->info("We got a new connection! :)");
+	try
-	CConnection *pc = new CConnection(upcomingConnection, NAME);
+	{
-	initConnection(pc);
+		logNetwork->info("We got a new connection! :)");
-	upcomingConnection = nullptr;
+		std::string name = NAME;
+		CConnection *pc = new CConnection(upcomingConnection, name.append(" STATE_PREGAME"));
+		initConnection(pc);
+		upcomingConnection = nullptr;
 
-	startListeningThread(pc);
+		startListeningThread(pc);
 
-	*pc << (ui8)pc->connectionID << curmap;
+		*pc << (ui8)pc->connectionID << curmap;
 
-	announceTxt(pc->name + " joins the game");
+		announceTxt(pc->name + " joins the game");
-	auto pj = new PlayerJoined();
+		auto pj = new PlayerJoined();
-	pj->playerName = pc->name;
+		pj->playerName = pc->name;
-	pj->connectionID = pc->connectionID;
+		pj->connectionID = pc->connectionID;
-	toAnnounce.push_back(pj);
+		toAnnounce.push_back(pj);
+	}
+	catch(std::exception& e)
+	{
+		upcomingConnection = nullptr;
+		logNetwork->info("I guess it was just my imagination!");
+	}
 
 	start_async_accept();
 }
@@ -314,9 +326,23 @@ void CPregameServer::startListeningThread(CConnection * pc)
 }
 
 CVCMIServer::CVCMIServer()
-: io(new boost::asio::io_service()), acceptor(new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port))), firstConnection(nullptr)
+	: port(3030), io(new boost::asio::io_service()), firstConnection(nullptr)
 {
 	logNetwork->trace("CVCMIServer created!");
+	if(cmdLineOptions.count("port"))
+		port = cmdLineOptions["port"].as<ui16>();
+	logNetwork->info("Port %d will be used", port);
+	try
+	{
+		acceptor = new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port));
+	}
+	catch(...)
+	{
+		logNetwork->info("Port %d is busy, trying to use random port instead", port);
+		acceptor = new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), 0));
+		port = acceptor->local_endpoint().port();
+	}
+	logNetwork->info("Listening for connections at port %d", port);
 }
 CVCMIServer::~CVCMIServer()
 {
@@ -413,50 +439,62 @@ void CVCMIServer::start()
 #endif
 
 	boost::system::error_code error;
-	logNetwork->info("Listening for connections at port %d", acceptor->local_endpoint().port());
+	for (;;)
-	auto s = new boost::asio::ip::tcp::socket(acceptor->get_io_service());
+	{
-	boost::thread acc(std::bind(vaccept,acceptor,s,&error));
+		try
-#ifndef VCMI_ANDROID
+		{
-	sr->setToTrueAndNotify();
+			auto s = new boost::asio::ip::tcp::socket(acceptor->get_io_service());
-	delete mr;
+			boost::thread acc(std::bind(vaccept,acceptor,s,&error));
+#ifdef VCMI_ANDROID
+			{ // in block to clean-up vm helper after use, because we don't need to keep this thread attached to vm
+				CAndroidVMHelper envHelper;
+				envHelper.callStaticVoidMethod(CAndroidVMHelper::NATIVE_METHODS_DEFAULT_CLASS, "onServerReady");
+				logNetwork->info("Sending server ready message to client");
+			}
 #else
-	{ // in block to clean-up vm helper after use, because we don't need to keep this thread attached to vm
+			sr->setToTrueAndNotify(port);
-		CAndroidVMHelper envHelper;
+			delete mr;
-		envHelper.callStaticVoidMethod(CAndroidVMHelper::NATIVE_METHODS_DEFAULT_CLASS, "onServerReady");
-		logNetwork->info("Sending server ready message to client");
-	}
 #endif
 
-	acc.join();
+			acc.join();
-	if (error)
+			if (error)
-	{
+			{
-		logNetwork->warnStream() << "Got connection but there is an error " << error;
+				logNetwork->warnStream()<<"Got connection but there is an error " << error;
-		return;
+				return;
-	}
+			}
-	logNetwork->info("We've accepted someone... ");
+			logNetwork->info("We've accepted someone... ");
-	firstConnection = new CConnection(s, NAME);
+			std::string name = NAME;
-	logNetwork->info("Got connection!");
+			firstConnection = new CConnection(s, name.append(" STATE_WAITING"));
-	while (!end2)
+			logNetwork->info("Got connection!");
-	{
+			while(!end2)
-		ui8 mode;
+			{
-		*firstConnection >> mode;
+				ui8 mode;
-		switch (mode)
+				*firstConnection >> mode;
+				switch (mode)
+				{
+				case 0:
+					firstConnection->close();
+					exit(0);
+				case 1:
+					firstConnection->close();
+					return;
+				case 2:
+					newGame();
+					break;
+				case 3:
+					loadGame();
+					break;
+				case 4:
+					newPregame();
+					break;
+				}
+			}
+			break;
+		}
+		catch(std::exception& e)
 		{
-		case 0:
+			vstd::clear_pointer(firstConnection);
-			firstConnection->close();
+			logNetwork->info("I guess it was just my imagination!");
-			exit(0);
-		case 1:
-			firstConnection->close();
-			return;
-		case 2:
-			newGame();
-			break;
-		case 3:
-			loadGame();
-			break;
-		case 4:
-			newPregame();
-			break;
 		}
 	}
 }
@@ -507,7 +545,7 @@ static void handleCommandOptions(int argc, char *argv[])
 	opts.add_options()
 		("help,h", "display help and exit")
 		("version,v", "display version information and exit")
-		("port", po::value<int>()->default_value(3030), "port at which server will listen to connections from client")
+		("port", po::value<ui16>(), "port at which server will listen to connections from client")
 		("resultsFile", po::value<std::string>()->default_value("./results.txt"), "file to which the battle result will be appended. Used only in the DUEL mode.");
 
 	if(argc > 1)
@@ -584,12 +622,7 @@ int main(int argc, char** argv)
 	logConfig.configureDefault();
 	logGlobal->info(NAME);
 
-
 	handleCommandOptions(argc, argv);
-	if (cmdLineOptions.count("port"))
-		port = cmdLineOptions["port"].as<int>();
-	logNetwork->info("Port %d will be used.", port);
-
 	preinitDLL(console);
 	settings.init();
 	logConfig.configure();
@@ -630,11 +663,9 @@ int main(int argc, char** argv)
 	CAndroidVMHelper envHelper;
 	envHelper.callStaticVoidMethod(CAndroidVMHelper::NATIVE_METHODS_DEFAULT_CLASS, "killServer");
 #endif
-	delete VLC;
+	vstd::clear_pointer(VLC);
-	VLC = nullptr;
 	CResourceHandler::clear();
-
+	return 0;
-  return 0;
 }
 
 #ifdef VCMI_ANDROID

server/CVCMIServer.h

@@ -43,6 +43,7 @@ typedef boost::asio::basic_stream_socket < boost::asio::ip::tcp , boost::asio::s
 
 class CVCMIServer
 {
+	ui16 port;
 	boost::asio::io_service *io;
 	TAcceptor * acceptor;