Merge pull request #2427 from cuberite/fixes

Fixed and improved restarting

CIbuild.sh

@@ -22,6 +22,7 @@ echo "Building..."
 make -j 2;
 make -j 2 test ARGS="-V";
 cd MCServer/;
-if [ "$TRAVIS_MCSERVER_BUILD_TYPE" != "COVERAGE" ]
+if [ "$TRAVIS_MCSERVER_BUILD_TYPE" != "COVERAGE" ]; then
-	then echo stop | $MCSERVER_PATH;
+	echo restart | $MCSERVER_PATH;
+	echo stop | $MCSERVER_PATH;
 fi

src/OSSupport/NetworkSingleton.cpp

@@ -18,8 +18,36 @@
 
 
 
-cNetworkSingleton::cNetworkSingleton(void):
+cNetworkSingleton::cNetworkSingleton() :
-	m_HasTerminated(false)
+	m_HasTerminated(true)
+{
+}
+
+
+
+
+
+cNetworkSingleton::~cNetworkSingleton()
+{
+	// Check that Terminate has been called already:
+	ASSERT(m_HasTerminated);
+}
+
+
+
+
+
+cNetworkSingleton & cNetworkSingleton::Get(void)
+{
+	static cNetworkSingleton Instance;
+	return Instance;
+}
+
+
+
+
+
+void cNetworkSingleton::Initialise(void)
 {
 	// Windows: initialize networking:
 	#ifdef _WIN32
@@ -64,26 +92,7 @@ cNetworkSingleton::cNetworkSingleton(void):
 
 	// Create the event loop thread:
 	m_EventLoopThread = std::thread(RunEventLoop, this);
-}
+	m_HasTerminated = false;
-
-
-
-
-
-cNetworkSingleton::~cNetworkSingleton()
-{
-	// Check that Terminate has been called already:
-	ASSERT(m_HasTerminated);
-}
-
-
-
-
-
-cNetworkSingleton & cNetworkSingleton::Get(void)
-{
-	static cNetworkSingleton Instance;
-	return Instance;
 }
 
 

src/OSSupport/NetworkSingleton.h

@@ -44,13 +44,18 @@ typedef std::vector<cIPLookupPtr> cIPLookupPtrs;
 class cNetworkSingleton
 {
 public:
+	cNetworkSingleton();
 	~cNetworkSingleton();
 
 	/** Returns the singleton instance of this class */
 	static cNetworkSingleton & Get(void);
 
+	/** Initialises all network-related threads.
+	To be called on first run or after app restart. */
+	void Initialise(void);
+
 	/** Terminates all network-related threads.
-	To be used only on app shutdown.
+	To be used only on app shutdown or restart.
 	MSVC runtime requires that the LibEvent networking be shut down before the main() function is exitted; this is the way to do it. */
 	void Terminate(void);
 
@@ -122,10 +127,6 @@ protected:
 	/** The thread in which the main LibEvent loop runs. */
 	std::thread m_EventLoopThread;
 
-
-	/** Initializes the LibEvent internals. */
-	cNetworkSingleton(void);
-
 	/** Converts LibEvent-generated log events into log messages in MCS log. */
 	static void LogCallback(int a_Severity, const char * a_Msg);
 

src/Root.cpp

@@ -26,10 +26,10 @@
 #include <iostream>
 
 #ifdef _WIN32
-	#include <conio.h>
 	#include <psapi.h>
 #elif defined(__linux__)
 	#include <fstream>
+	#include <signal.h>
 #elif defined(__APPLE__)
 	#include <mach/mach.h>
 #endif
@@ -39,7 +39,6 @@
 
 
 cRoot * cRoot::s_Root = nullptr;
-bool cRoot::m_ShouldStop = false;
 
 
 
@@ -53,10 +52,10 @@ cRoot::cRoot(void) :
 	m_FurnaceRecipe(nullptr),
 	m_WebAdmin(nullptr),
 	m_PluginManager(nullptr),
-	m_MojangAPI(nullptr),
+	m_MojangAPI(nullptr)
-	m_bRestart(false)
 {
 	s_Root = this;
+	m_InputThreadRunFlag.clear();
 }
 
 
@@ -76,24 +75,22 @@ void cRoot::InputThread(cRoot & a_Params)
 {
 	cLogCommandOutputCallback Output;
 
-	while (!cRoot::m_ShouldStop && !a_Params.m_bRestart && !m_TerminateEventRaised && std::cin.good())
+	while (a_Params.m_InputThreadRunFlag.test_and_set() && std::cin.good())
 	{
 		AString Command;
 		std::getline(std::cin, Command);
 		if (!Command.empty())
 		{
+			// Execute and clear command string when submitted
 			a_Params.ExecuteConsoleCommand(TrimString(Command), Output);
 		}
 	}
 
-	if (m_TerminateEventRaised || !std::cin.good())
-	{
 	// We have come here because the std::cin has received an EOF / a terminate signal has been sent, and the server is still running
-		// Stop the server:
+	if (!std::cin.good())
-		if (!m_RunAsService)  // Dont kill if running as a service
 	{
-			a_Params.m_ShouldStop = true;
+		// Stop the server:
-		}
+		a_Params.QueueExecuteConsoleCommand("stop");
 	}
 }
 
@@ -101,7 +98,7 @@ void cRoot::InputThread(cRoot & a_Params)
 
 
 
-void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
+void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> a_OverridesRepo)
 {
 	#ifdef _WIN32
 		HWND hwnd = GetConsoleWindow();
@@ -127,12 +124,7 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 	#endif
 
 	cDeadlockDetect dd;
-
-	m_ShouldStop = false;
-	while (!m_ShouldStop)
-	{
 	auto BeginTime = std::chrono::steady_clock::now();
-		m_bRestart = false;
 
 	LoadGlobalSettings();
 
@@ -149,7 +141,7 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 		IniFile->AddHeaderComment(" Most of the settings here can be configured using the webadmin interface, if enabled in webadmin.ini");
 		IniFile->AddHeaderComment(" See: http://wiki.mc-server.org/doku.php?id=configure:settings.ini for further configuration help");
 	}
-		auto settingsRepo = cpp14::make_unique<cOverridesSettingsRepository>(std::move(IniFile), std::move(overridesRepo));
+	auto settingsRepo = cpp14::make_unique<cOverridesSettingsRepository>(std::move(IniFile), std::move(a_OverridesRepo));
 
 	LOG("Starting server...");
 	m_MojangAPI = new cMojangAPI;
@@ -168,7 +160,7 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 	LOGD("Loading settings...");
 	m_RankManager.reset(new cRankManager());
 	m_RankManager->Initialize(*m_MojangAPI);
-		m_CraftingRecipes = new cCraftingRecipes;
+	m_CraftingRecipes = new cCraftingRecipes();
 	m_FurnaceRecipe   = new cFurnaceRecipe();
 
 	LOGD("Loading worlds...");
@@ -205,8 +197,8 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 			LOGD("Starting InputThread...");
 			try
 			{
+				m_InputThreadRunFlag.test_and_set();
 				m_InputThread = std::thread(InputThread, std::ref(*this));
-				m_InputThread.detach();
 			}
 			catch (std::system_error & a_Exception)
 			{
@@ -223,14 +215,19 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 			EnableMenuItem(hmenu, SC_CLOSE, MF_ENABLED);  // Re-enable close button
 		#endif
 
-			while (!m_ShouldStop && !m_bRestart && !m_TerminateEventRaised)  // These are modified by external threads
+		for (;;)
 		{
-				std::this_thread::sleep_for(std::chrono::seconds(1));
+			m_StopEvent.Wait();
-			}
 
-			if (m_TerminateEventRaised)
+			if (m_TerminateEventRaised && m_RunAsService)
 			{
-				m_ShouldStop = true;
+				// Dont kill if running as a service
+				m_TerminateEventRaised = false;
+			}
+			else
+			{
+				break;
+			}
 		}
 
 		// Stop the server:
@@ -238,11 +235,7 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 
 		LOG("Shutting down server...");
 		m_Server->Shutdown();
-		}  // if (m_Server->Start())
+	}  // if (m_Server->Start()
-		else
-		{
-			m_ShouldStop = true;
-		}
 
 	delete m_MojangAPI; m_MojangAPI = nullptr;
 
@@ -274,9 +267,54 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 	LOG("Cleaning up...");
 	delete m_Server; m_Server = nullptr;
 
+	m_InputThreadRunFlag.clear();
+	#ifdef _WIN32
+		DWORD Length;
+		INPUT_RECORD Record
+		{
+			static_cast<WORD>(KEY_EVENT),
+			{
+				{
+					TRUE,
+					1,
+					VK_RETURN,
+					MapVirtualKey(VK_RETURN, MAPVK_VK_TO_VSC),
+					{ { VK_RETURN } },
+					0
+				}
+			}
+		};
+
+		// Can't kill the input thread since it breaks cin (getline doesn't block / receive input on restart)
+		// Apparently no way to unblock getline
+		// Only thing I can think of for now
+		if (WriteConsoleInput(GetStdHandle(STD_INPUT_HANDLE), &Record, 1, &Length) == 0)
+		{
+			LOGWARN("Couldn't notify the input thread; the server will hang before shutdown!");
+			m_TerminateEventRaised = true;
+			m_InputThread.detach();
+		}
+		else
+		{
+			m_InputThread.join();
+		}
+	#else
+		if (pthread_kill(m_InputThread.native_handle(), SIGKILL) != 0)
+		{
+			LOGWARN("Couldn't notify the input thread; the server will hang before shutdown!");
+			m_TerminateEventRaised = true;
+			m_InputThread.detach();
+		}
+	#endif
+
+	if (m_TerminateEventRaised)
+	{
 		LOG("Shutdown successful!");
 	}
-
+	else
+	{
+		LOG("Shutdown successful - restarting...");
+	}
 	LOG("--- Stopped Log ---");
 
 	cLogger::GetInstance().DetachListener(consoleLogListener);
@@ -475,19 +513,9 @@ void cRoot::TickCommands(void)
 
 void cRoot::QueueExecuteConsoleCommand(const AString & a_Cmd, cCommandOutputCallback & a_Output)
 {
-	// Some commands are built-in:
-	if (a_Cmd == "stop")
-	{
-		m_ShouldStop = true;
-	}
-	else if (a_Cmd == "restart")
-	{
-		m_bRestart = true;
-	}
-
 	// Put the command into a queue (Alleviates FS #363):
 	cCSLock Lock(m_CSPendingCommands);
-	m_PendingCommands.push_back(cCommand(a_Cmd, &a_Output));
+	m_PendingCommands.emplace_back(a_Cmd, &a_Output);
 }
 
 
@@ -507,15 +535,18 @@ void cRoot::QueueExecuteConsoleCommand(const AString & a_Cmd)
 
 void cRoot::ExecuteConsoleCommand(const AString & a_Cmd, cCommandOutputCallback & a_Output)
 {
-	// cRoot handles stopping and restarting due to our access to controlling variables
+	// Some commands are built-in:
 	if (a_Cmd == "stop")
 	{
-		m_ShouldStop = true;
+		m_TerminateEventRaised = true;
+		m_StopEvent.Set();
+		m_InputThreadRunFlag.clear();
 		return;
 	}
 	else if (a_Cmd == "restart")
 	{
-		m_bRestart = true;
+		m_StopEvent.Set();
+		m_InputThreadRunFlag.clear();
 		return;
 	}
 

src/Root.h

@@ -7,6 +7,7 @@
 #include "Defines.h"
 #include "RankManager.h"
 #include <thread>
+#include <atomic>
 
 
 
@@ -48,13 +49,12 @@ public:
 
 	static bool m_TerminateEventRaised;
 	static bool m_RunAsService;
-	static bool m_ShouldStop;
 
 
 	cRoot(void);
 	~cRoot();
 
-	void Start(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo);
+	void Start(std::unique_ptr<cSettingsRepositoryInterface> a_OverridesRepo);
 
 	// tolua_begin
 	cServer * GetServer(void) { return m_Server; }
@@ -200,6 +200,8 @@ private:
 	cCommandQueue    m_PendingCommands;
 
 	std::thread m_InputThread;
+	cEvent m_StopEvent;
+	std::atomic_flag m_InputThreadRunFlag;
 
 	cServer *        m_Server;
 	cMonsterConfig * m_MonsterConfig;
@@ -215,8 +217,6 @@ private:
 
 	cHTTPServer m_HTTPServer;
 
-	bool m_bRestart;
-
 
 	void LoadGlobalSettings();
 

src/main.cpp

@@ -22,9 +22,6 @@
 /** If something has told the server to stop; checked periodically in cRoot */
 bool cRoot::m_TerminateEventRaised = false;
 
-/** Set to true when the server terminates, so our CTRL handler can then tell the OS to close the console. */
-static bool g_ServerTerminated = false;
-
 /** If set to true, the protocols will log each player's incoming (C->S) communication to a per-connection logfile */
 bool g_ShouldLogCommIn;
 
@@ -72,7 +69,7 @@ Synchronize this with Server.cpp to enable the "dumpmem" console command. */
 void NonCtrlHandler(int a_Signal)
 {
 	LOGD("Terminate event raised from std::signal");
-	cRoot::m_TerminateEventRaised = true;
+	cRoot::Get()->QueueExecuteConsoleCommand("stop");
 
 	switch (a_Signal)
 	{
@@ -189,13 +186,11 @@ LONG WINAPI LastChanceExceptionFilter(__in struct _EXCEPTION_POINTERS * a_Except
 // Handle CTRL events in windows, including console window close
 BOOL CtrlHandler(DWORD fdwCtrlType)
 {
-	cRoot::m_TerminateEventRaised = true;
+	cRoot::Get()->QueueExecuteConsoleCommand("stop");
 	LOGD("Terminate event raised from the Windows CtrlHandler");
 
-	while (!g_ServerTerminated)
+	std::this_thread::sleep_for(std::chrono::seconds(10));  // Delay as much as possible to try to get the server to shut down cleanly - 10 seconds given by Windows
-	{
+	// Returning from main() automatically aborts this handler thread
-		std::this_thread::sleep_for(std::chrono::milliseconds(50));  // Delay as much as possible to try to get the server to shut down cleanly
-	}
 
 	return TRUE;
 }
@@ -206,29 +201,22 @@ BOOL CtrlHandler(DWORD fdwCtrlType)
 
 
 ////////////////////////////////////////////////////////////////////////////////
-// universalMain - Main startup logic for both standard running and as a service
+// UniversalMain - Main startup logic for both standard running and as a service
 
-void universalMain(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
+void UniversalMain(std::unique_ptr<cSettingsRepositoryInterface> a_OverridesRepo)
 {
-	#ifdef _WIN32
-	if (!SetConsoleCtrlHandler((PHANDLER_ROUTINE)CtrlHandler, TRUE))
-	{
-		LOGERROR("Could not install the Windows CTRL handler!");
-	}
-	#endif
-
 	// Initialize logging subsystem:
 	cLogger::InitiateMultithreading();
 
 	// Initialize LibEvent:
-	cNetworkSingleton::Get();
+	cNetworkSingleton::Get().Initialise();
 
 	#if !defined(ANDROID_NDK)
 	try
 	#endif
 	{
 		cRoot Root;
-		Root.Start(std::move(overridesRepo));
+		Root.Start(std::move(a_OverridesRepo));
 	}
 	#if !defined(ANDROID_NDK)
 	catch (std::exception & e)
@@ -241,8 +229,6 @@ void universalMain(std::unique_ptr<cSettingsRepositoryInterface> overridesRepo)
 	}
 	#endif
 
-	g_ServerTerminated = true;
-
 	// Shutdown all of LibEvent:
 	cNetworkSingleton::Get().Terminate();
 }
@@ -259,8 +245,11 @@ DWORD WINAPI serviceWorkerThread(LPVOID lpParam)
 {
 	UNREFERENCED_PARAMETER(lpParam);
 
+	while (!cRoot::m_TerminateEventRaised)
+	{
 		// Do the normal startup
-	universalMain(cpp14::make_unique<cMemorySettingsRepository>());
+		UniversalMain(cpp14::make_unique<cMemorySettingsRepository>());
+	}
 
 	return ERROR_SUCCESS;
 }
@@ -274,8 +263,7 @@ DWORD WINAPI serviceWorkerThread(LPVOID lpParam)
 
 void serviceSetState(DWORD acceptedControls, DWORD newState, DWORD exitCode)
 {
-	SERVICE_STATUS serviceStatus;
+	SERVICE_STATUS serviceStatus = {};
-	ZeroMemory(&serviceStatus, sizeof(SERVICE_STATUS));
 	serviceStatus.dwCheckPoint = 0;
 	serviceStatus.dwControlsAccepted = acceptedControls;
 	serviceStatus.dwCurrentState = newState;
@@ -302,11 +290,10 @@ void WINAPI serviceCtrlHandler(DWORD CtrlCode)
 	{
 		case SERVICE_CONTROL_STOP:
 		{
-			cRoot::m_ShouldStop = true;
+			cRoot::Get()->QueueExecuteConsoleCommand("stop");
 			serviceSetState(0, SERVICE_STOP_PENDING, 0);
 			break;
 		}
-
 		default:
 		{
 			break;
@@ -365,7 +352,7 @@ void WINAPI serviceMain(DWORD argc, TCHAR *argv[])
 
 
 
-std::unique_ptr<cMemorySettingsRepository> parseArguments(int argc, char **argv)
+std::unique_ptr<cMemorySettingsRepository> ParseArguments(int argc, char **argv)
 {
 	try
 	{
@@ -484,7 +471,13 @@ int main(int argc, char **argv)
 		#endif  // SIGABRT_COMPAT
 	#endif
 
-	auto argsRepo = parseArguments(argc, argv);
+	
+	#ifdef _WIN32
+		if (!SetConsoleCtrlHandler((PHANDLER_ROUTINE)CtrlHandler, TRUE))
+		{
+			LOGERROR("Could not install the Windows CTRL handler!");
+		}
+	#endif
 
 	// Attempt to run as a service
 	if (cRoot::m_RunAsService)
@@ -522,13 +515,19 @@ int main(int argc, char **argv)
 			close(STDOUT_FILENO);
 			close(STDERR_FILENO);
 
-			universalMain(std::move(argsRepo));
+			while (!cRoot::m_TerminateEventRaised)
+			{
+				UniversalMain(std::move(ParseArguments(argc, argv)));
+			}
 		#endif
 	}
 	else
+	{
+		while (!cRoot::m_TerminateEventRaised)
 		{
 			// Not running as a service, do normal startup
-		universalMain(std::move(argsRepo));
+			UniversalMain(std::move(ParseArguments(argc, argv)));
+		}
 	}
 
 	#if defined(_MSC_VER) && defined(_DEBUG) && defined(ENABLE_LEAK_FINDER)

tests/Network/Google.cpp

@@ -118,6 +118,7 @@ static void DoTest(void)
 
 int main()
 {
+	cNetworkSingleton::Get().Initialise();
 	DoTest();
 
 	cNetworkSingleton::Get().Terminate();