fix: signal handling (#1375)

* fix: signal handling

* fix: flush WorldServer logger before main loop

* fix: consolidate signal code

dAuthServer/AuthServer.cpp

@@ -1,6 +1,7 @@
 #include <iostream>
 #include <string>
 #include <ctime>
+#include <csignal>
 #include <chrono>
 #include <thread>
 
@@ -28,7 +29,7 @@ namespace Game {
 	Logger* logger = nullptr;
 	dServer* server = nullptr;
 	dConfig* config = nullptr;
-	bool shouldShutdown = false;
+	Game::signal_t lastSignal = 0;
 	std::mt19937 randomEngine;
 }
 
@@ -42,6 +43,9 @@ int main(int argc, char** argv) {
 	Diagnostics::SetProcessFileName(argv[0]);
 	Diagnostics::Initialize();
 
+	std::signal(SIGINT, Game::OnSignal);
+	std::signal(SIGTERM, Game::OnSignal);
+
 	//Create all the objects we need to run our service:
 	Game::logger = SetupLogger();
 	if (!Game::logger) return EXIT_FAILURE;
@@ -74,6 +78,7 @@ int main(int argc, char** argv) {
 		masterIP = masterInfo->ip;
 		masterPort = masterInfo->port;
 	}
+	LOG("Master is at %s:%d", masterIP.c_str(), masterPort);
 
 	Game::randomEngine = std::mt19937(time(0));
 
@@ -83,7 +88,7 @@ int main(int argc, char** argv) {
 	if (Game::config->GetValue("max_clients") != "") maxClients = std::stoi(Game::config->GetValue("max_clients"));
 	if (Game::config->GetValue("port") != "") ourPort = std::atoi(Game::config->GetValue("port").c_str());
 
-	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, false, true, Game::logger, masterIP, masterPort, ServerType::Auth, Game::config, &Game::shouldShutdown);
+	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, false, true, Game::logger, masterIP, masterPort, ServerType::Auth, Game::config, &Game::lastSignal);
 
 	//Run it until server gets a kill message from Master:
 	auto t = std::chrono::high_resolution_clock::now();
@@ -96,13 +101,16 @@ int main(int argc, char** argv) {
 
 	AuthPackets::LoadClaimCodes();
 
-	while (!Game::shouldShutdown) {
+	Game::logger->Flush(); // once immediately before main loop
+	while (!Game::ShouldShutdown()) {
 		//Check if we're still connected to master:
 		if (!Game::server->GetIsConnectedToMaster()) {
 			framesSinceMasterDisconnect++;
 
-			if (framesSinceMasterDisconnect >= authFramerate)
+			if (framesSinceMasterDisconnect >= authFramerate) {
+				LOG("No connection to master!");
 				break; //Exit our loop, shut down.
+			}
 		} else framesSinceMasterDisconnect = 0;
 
 		//In world we'd update our other systems here.
@@ -141,6 +149,7 @@ int main(int argc, char** argv) {
 		std::this_thread::sleep_until(t);
 	}
 
+	LOG("Exited Main Loop! (signal %d)", Game::lastSignal);
 	//Delete our objects here:
 	Database::Destroy("AuthServer");
 	delete Game::server;

dChatServer/ChatServer.cpp

@@ -33,7 +33,7 @@ namespace Game {
 	dConfig* config = nullptr;
 	dChatFilter* chatFilter = nullptr;
 	AssetManager* assetManager = nullptr;
-	bool shouldShutdown = false;
+	Game::signal_t lastSignal = 0;
 	std::mt19937 randomEngine;
 	PlayerContainer playerContainer;
 }
@@ -48,6 +48,9 @@ int main(int argc, char** argv) {
 	Diagnostics::SetProcessFileName(argv[0]);
 	Diagnostics::Initialize();
 
+	std::signal(SIGINT, Game::OnSignal);
+	std::signal(SIGTERM, Game::OnSignal);
+
 	//Create all the objects we need to run our service:
 	Game::logger = SetupLogger();
 	if (!Game::logger) return EXIT_FAILURE;
@@ -101,7 +104,7 @@ int main(int argc, char** argv) {
 	if (Game::config->GetValue("max_clients") != "") maxClients = std::stoi(Game::config->GetValue("max_clients"));
 	if (Game::config->GetValue("port") != "") ourPort = std::atoi(Game::config->GetValue("port").c_str());
 
-	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, false, true, Game::logger, masterIP, masterPort, ServerType::Chat, Game::config, &Game::shouldShutdown);
+	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, false, true, Game::logger, masterIP, masterPort, ServerType::Chat, Game::config, &Game::lastSignal);
 
 	Game::chatFilter = new dChatFilter(Game::assetManager->GetResPath().string() + "/chatplus_en_us", bool(std::stoi(Game::config->GetValue("dont_generate_dcf"))));
 
@@ -118,7 +121,8 @@ int main(int argc, char** argv) {
 	uint32_t framesSinceMasterDisconnect = 0;
 	uint32_t framesSinceLastSQLPing = 0;
 
-	while (!Game::shouldShutdown) {
+	Game::logger->Flush(); // once immediately before main loop
+	while (!Game::ShouldShutdown()) {
 		//Check if we're still connected to master:
 		if (!Game::server->GetIsConnectedToMaster()) {
 			framesSinceMasterDisconnect++;

dCommon/CMakeLists.txt

@@ -5,6 +5,7 @@ set(DCOMMON_SOURCES
 		"dConfig.cpp"
 		"Diagnostics.cpp"
 		"Logger.cpp"
+		"Game.cpp"
 		"GeneralUtils.cpp"
 		"LDFFormat.cpp"
 		"MD5.cpp"

dCommon/Game.cpp

@@ -0,0 +1,7 @@
+#include "Game.h"
+
+namespace Game {
+	void OnSignal(int signal) {
+		lastSignal = signal;
+	}
+}

dCommon/Game.h

@@ -2,6 +2,7 @@
 
 #include <string>
 #include <random>
+#include <csignal>
 
 class dServer;
 class Logger;
@@ -16,6 +17,7 @@ class dZoneManager;
 class PlayerContainer;
 
 namespace Game {
+	using signal_t = volatile std::sig_atomic_t;
 	extern Logger* logger;
 	extern dServer* server;
 	extern InstanceManager* im;
@@ -25,9 +27,14 @@ namespace Game {
 	extern RakPeerInterface* chatServer;
 	extern AssetManager* assetManager;
 	extern SystemAddress chatSysAddr;
-	extern bool shouldShutdown;
+	extern signal_t lastSignal;
 	extern EntityManager* entityManager;
 	extern dZoneManager* zoneManager;
 	extern PlayerContainer playerContainer;
 	extern std::string projectVersion;
+
+	inline bool ShouldShutdown() {
+		return lastSignal != 0;
+	}
+	void OnSignal(int signal);
 }

dCommon/Logger.cpp

@@ -6,6 +6,8 @@
 #include <stdarg.h>
 
 Writer::~Writer() {
+	// Flush before we close
+	Flush();
 	// Dont try to close stdcout...
 	if (!m_Outfile || m_IsConsoleWriter) return;
 

dMasterServer/InstanceManager.cpp

@@ -134,7 +134,7 @@ void InstanceManager::RemoveInstance(Instance* instance) {
 		if (m_Instances[i] == instance) {
 			instance->SetShutdownComplete(true);
 
-			if (!Game::shouldShutdown) RedirectPendingRequests(instance);
+			if (!Game::ShouldShutdown()) RedirectPendingRequests(instance);
 
 			delete m_Instances[i];
 

dMasterServer/MasterServer.cpp

@@ -47,12 +47,13 @@ namespace Game {
 	InstanceManager* im = nullptr;
 	dConfig* config = nullptr;
 	AssetManager* assetManager = nullptr;
-	bool shouldShutdown = false;
+	Game::signal_t lastSignal = 0;
+	bool universeShutdownRequested = false;
 	std::mt19937 randomEngine;
 } //namespace Game
 
 bool shutdownSequenceStarted = false;
-void ShutdownSequence(int32_t signal = -1);
+int ShutdownSequence(int32_t signal = -1);
 int32_t FinalizeShutdown(int32_t signal = -1);
 Logger* SetupLogger();
 void HandlePacket(Packet* packet);
@@ -73,8 +74,8 @@ int main(int argc, char** argv) {
 
 	//Triggers the shutdown sequence at application exit
 	std::atexit([]() { ShutdownSequence(); });
-	signal(SIGINT, [](int32_t signal) { ShutdownSequence(EXIT_FAILURE); });
-	signal(SIGTERM, [](int32_t signal) { ShutdownSequence(EXIT_FAILURE); });
+	std::signal(SIGINT, Game::OnSignal);
+	std::signal(SIGTERM, Game::OnSignal);
 
 	//Create all the objects we need to run our service:
 	Game::logger = SetupLogger();
@@ -286,7 +287,7 @@ int main(int argc, char** argv) {
 	if (Game::config->GetValue("max_clients") != "") maxClients = std::stoi(Game::config->GetValue("max_clients"));
 	if (Game::config->GetValue("port") != "") ourPort = std::stoi(Game::config->GetValue("port"));
 
-	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, true, false, Game::logger, "", 0, ServerType::Master, Game::config, &Game::shouldShutdown);
+	Game::server = new dServer(Game::config->GetValue("external_ip"), ourPort, 0, maxClients, true, false, Game::logger, "", 0, ServerType::Master, Game::config, &Game::lastSignal);
 
 	//Query for the database for a server labeled "master"
 
@@ -321,7 +322,8 @@ int main(int argc, char** argv) {
 	uint32_t framesSinceLastSQLPing = 0;
 	uint32_t framesSinceKillUniverseCommand = 0;
 
-	while (true) {
+	Game::logger->Flush();
+	while (!Game::ShouldShutdown()) {
 		//In world we'd update our other systems here.
 
 		//Check for packets here:
@@ -355,10 +357,10 @@ int main(int argc, char** argv) {
 			framesSinceLastSQLPing++;
 
 		//10m shutdown for universe kill command
-		if (Game::shouldShutdown) {
+		if (Game::universeShutdownRequested) {
 			if (framesSinceKillUniverseCommand >= shutdownUniverseTime) {
 				//Break main loop and exit
-				break;
+				Game::lastSignal = -1;
 			} else
 				framesSinceKillUniverseCommand++;
 		}
@@ -402,7 +404,7 @@ int main(int argc, char** argv) {
 		t += std::chrono::milliseconds(masterFrameDelta);
 		std::this_thread::sleep_until(t);
 	}
-	return FinalizeShutdown(EXIT_SUCCESS);
+	return ShutdownSequence(EXIT_SUCCESS);
 }
 
 Logger* SetupLogger() {
@@ -799,7 +801,7 @@ void HandlePacket(Packet* packet) {
 
 		case eMasterMessageType::SHUTDOWN_UNIVERSE: {
 			LOG("Received shutdown universe command, shutting down in 10 minutes.");
-			Game::shouldShutdown = true;
+			Game::universeShutdownRequested = true;
 			break;
 		}
 
@@ -809,9 +811,11 @@ void HandlePacket(Packet* packet) {
 	}
 }
 
-void ShutdownSequence(int32_t signal) {
+int ShutdownSequence(int32_t signal) {
+	LOG("Recieved Signal %d", signal);
 	if (shutdownSequenceStarted) {
-		return;
+		LOG("Duplicate Shutdown Sequence");
+		return -1;
 	}
 
 	if (!Game::im) {
@@ -820,7 +824,7 @@ void ShutdownSequence(int32_t signal) {
 
 	Game::im->SetIsShuttingDown(true);
 	shutdownSequenceStarted = true;
-	Game::shouldShutdown = true;
+	Game::lastSignal = -1;
 
 	{
 		CBITSTREAM;
@@ -889,7 +893,7 @@ void ShutdownSequence(int32_t signal) {
 		}
 	}
 
-	FinalizeShutdown(signal);
+	return FinalizeShutdown(signal);
 }
 
 int32_t FinalizeShutdown(int32_t signal) {

dMasterServer/Start.cpp

@@ -5,7 +5,7 @@
 #include "BinaryPathFinder.h"
 
 void StartChatServer() {
-	if (Game::shouldShutdown) {
+	if (Game::ShouldShutdown()) {
 		LOG("Currently shutting down.  Chat will not be restarted.");
 		return;
 	}
@@ -24,7 +24,7 @@ void StartChatServer() {
 }
 
 void StartAuthServer() {
-	if (Game::shouldShutdown) {
+	if (Game::ShouldShutdown()) {
 		LOG("Currently shutting down.  Auth will not be restarted.");
 		return;
 	}

dNet/AuthPackets.cpp

@@ -55,7 +55,9 @@ void AuthPackets::SendHandshake(dServer* server, const SystemAddress& sysAddr, c
 	RakNet::BitStream bitStream;
 	BitStreamUtils::WriteHeader(bitStream, eConnectionType::SERVER, eServerMessageType::VERSION_CONFIRM);
 	uint32_t netVersion;
-	if (!GeneralUtils::TryParse(Game::config->GetValue("client_net_version"), netVersion)) {
+	const std::string& expectedVersion = Game::config->GetValue("client_net_version");
+	LOG("Expected Version: '%s'", expectedVersion.c_str());
+	if (!GeneralUtils::TryParse(expectedVersion, netVersion)) {
 		LOG("Failed to parse client_net_version. Cannot authenticate to %s:%i", nextServerIP.c_str(), nextServerPort);
 		return;
 	}

dNet/dServer.cpp

@@ -39,7 +39,7 @@ class ReplicaReceiever : public ReceiveDownloadCompleteInterface {
 	}
 } ReceiveDownloadCompleteCB;
 
-dServer::dServer(const std::string& ip, int port, int instanceID, int maxConnections, bool isInternal, bool useEncryption, Logger* logger, const std::string masterIP, int masterPort, ServerType serverType, dConfig* config, bool* shouldShutdown, unsigned int zoneID) {
+dServer::dServer(const std::string& ip, int port, int instanceID, int maxConnections, bool isInternal, bool useEncryption, Logger* logger, const std::string masterIP, int masterPort, ServerType serverType, dConfig* config, Game::signal_t* lastSignal, unsigned int zoneID) {
 	mIP = ip;
 	mPort = port;
 	mZoneID = zoneID;
@@ -55,7 +55,7 @@ dServer::dServer(const std::string& ip, int port, int instanceID, int maxConnect
 	mReplicaManager = nullptr;
 	mServerType = serverType;
 	mConfig = config;
-	mShouldShutdown = shouldShutdown;
+	mShouldShutdown = lastSignal;
 	//Attempt to start our server here:
 	mIsOkay = Startup();
 
@@ -75,7 +75,9 @@ dServer::dServer(const std::string& ip, int port, int instanceID, int maxConnect
 	//Connect to master if we are not master:
 	if (serverType != ServerType::Master) {
 		SetupForMasterConnection();
-		ConnectToMaster();
+		if (!ConnectToMaster()) {
+			LOG("Failed ConnectToMaster!");
+		}
 	}
 
 	//Set up Replica if we're a world server:
@@ -129,7 +131,7 @@ Packet* dServer::ReceiveFromMaster() {
 					break;
 				}
 				case eMasterMessageType::SHUTDOWN:
-					*mShouldShutdown = true;
+					*mShouldShutdown = -2;
 					break;
 
 				//When we handle these packets in World instead dServer, we just return the packet's pointer.
@@ -236,10 +238,12 @@ void dServer::Shutdown() {
 void dServer::SetupForMasterConnection() {
 	mMasterSocketDescriptor = SocketDescriptor(uint16_t(mPort + 1), 0);
 	mMasterPeer = RakNetworkFactory::GetRakPeerInterface();
-	mMasterPeer->Startup(1, 30, &mMasterSocketDescriptor, 1);
+	bool ret = mMasterPeer->Startup(1, 30, &mMasterSocketDescriptor, 1);
+	if (!ret) LOG("Failed MasterPeer Startup!");
 }
 
 bool dServer::ConnectToMaster() {
+	//LOG("Connection to Master %s:%d", mMasterIP.c_str(), mMasterPort);
 	return mMasterPeer->Connect(mMasterIP.c_str(), mMasterPort, "3.25 DARKFLAME1", 15);
 }