Merge #6577

6577: Quit client when existing config file cannot be loaded, add `restart` command to client, properly uninitialize all client components r=def- a=Robyt3



## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [X] Considered possible null pointers and out of bounds array indexing
- [X] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>

src/engine/client/client.cpp

@@ -5,6 +5,7 @@
 
 #include <climits>
 #include <new>
+#include <stack>
 #include <tuple>
 
 #include <base/hash.h>
@@ -3314,10 +3315,8 @@ void CClient::Run()
 				s_SavedConfig = true;
 			}
 
-			IOHANDLE File = m_pStorage->OpenFile(m_aDDNetInfoTmp, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
-			if(File)
+			if(m_pStorage->FileExists(m_aDDNetInfoTmp, IStorage::TYPE_SAVE))
 			{
-				io_close(File);
 				m_pStorage->RemoveFile(m_aDDNetInfoTmp, IStorage::TYPE_SAVE);
 			}
 
@@ -3480,6 +3479,12 @@ void CClient::Con_Quit(IConsole::IResult *pResult, void *pUserData)
 	pSelf->Quit();
 }
 
+void CClient::Con_Restart(IConsole::IResult *pResult, void *pUserData)
+{
+	CClient *pSelf = (CClient *)pUserData;
+	pSelf->Restart();
+}
+
 void CClient::Con_Minimize(IConsole::IResult *pResult, void *pUserData)
 {
 	CClient *pSelf = (CClient *)pUserData;
@@ -4392,9 +4397,10 @@ void CClient::RegisterCommands()
 	m_pConsole->Register("dummy_disconnect", "", CFGFLAG_CLIENT, Con_DummyDisconnect, this, "Disconnect dummy");
 	m_pConsole->Register("dummy_reset", "", CFGFLAG_CLIENT, Con_DummyResetInput, this, "Reset dummy");
 
-	m_pConsole->Register("quit", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Quit, this, "Quit Teeworlds");
-	m_pConsole->Register("exit", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Quit, this, "Quit Teeworlds");
-	m_pConsole->Register("minimize", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Minimize, this, "Minimize Teeworlds");
+	m_pConsole->Register("quit", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Quit, this, "Quit the client");
+	m_pConsole->Register("exit", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Quit, this, "Quit the client");
+	m_pConsole->Register("restart", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Restart, this, "Restart the client");
+	m_pConsole->Register("minimize", "", CFGFLAG_CLIENT | CFGFLAG_STORE, Con_Minimize, this, "Minimize the client");
 	m_pConsole->Register("connect", "r[host|ip]", CFGFLAG_CLIENT, Con_Connect, this, "Connect to the specified host/ip");
 	m_pConsole->Register("disconnect", "", CFGFLAG_CLIENT, Con_Disconnect, this, "Disconnect from the server");
 	m_pConsole->Register("ping", "", CFGFLAG_CLIENT, Con_Ping, this, "Ping the current server");
@@ -4542,7 +4548,6 @@ int main(int argc, const char **argv)
 	CCmdlineFix CmdlineFix(&argc, &argv);
 
 	bool Silent = false;
-	bool RandInitFailed = false;
 
 	for(int i = 1; i < argc; i++)
 	{
@@ -4577,17 +4582,42 @@ int main(int argc, const char **argv)
 	vpLoggers.push_back(pFutureAssertionLogger);
 	log_set_global_logger(log_logger_collection(std::move(vpLoggers)).release());
 
-	if(secure_random_init() != 0)
-	{
-		RandInitFailed = true;
-	}
+	std::stack<std::function<void()>> CleanerFunctions;
+	std::function<void()> PerformCleanup = [&CleanerFunctions]() mutable {
+		while(!CleanerFunctions.empty())
+		{
+			CleanerFunctions.top()();
+			CleanerFunctions.pop();
+		}
+	};
+	std::function<void()> PerformFinalCleanup = []() {
+#ifdef CONF_PLATFORM_ANDROID
+		// properly close this native thread, so globals are destructed
+		std::exit(0);
+#endif
+	};
+	std::function<void()> PerformAllCleanup = [PerformCleanup, PerformFinalCleanup]() mutable {
+		PerformCleanup();
+		PerformFinalCleanup();
+	};
+
+	const bool RandInitFailed = secure_random_init() != 0;
+	if(!RandInitFailed)
+		CleanerFunctions.push([]() { secure_random_uninit(); });
 
 	NotificationsInit();
+	CleanerFunctions.push([]() { NotificationsUninit(); });
 
 	CClient *pClient = CreateClient();
 	IKernel *pKernel = IKernel::Create();
 	pKernel->RegisterInterface(pClient, false);
 	pClient->RegisterInterfaces();
+	CleanerFunctions.push([pKernel, pClient]() {
+		pKernel->Shutdown();
+		delete pKernel;
+		pClient->~CClient();
+		free(pClient);
+	});
 
 	const std::thread::id MainThreadId = std::this_thread::get_id();
 	dbg_assert_set_handler([MainThreadId, pClient](const char *pMsg) {
@@ -4599,6 +4629,7 @@ int main(int argc, const char **argv)
 		char aMessage[512];
 		str_format(aMessage, sizeof(aMessage), "An assertion error occured. Please write down or take a screenshot of the following information and report this error.\nPlease also share the assert log which you should find in the 'dumps' folder in your config directory.\n\n%s\n\nPlatform: %s\nGame version: %s %s\nOS version: %s", pMsg, CONF_PLATFORM_STRING, GAME_RELEASE_VERSION, GIT_SHORTREV_HASH != nullptr ? GIT_SHORTREV_HASH : "", aVersionStr);
 		pClient->ShowMessageBox("Assertion Error", aMessage);
+		// Client will crash due to assertion, don't call PerformAllCleanup in this inconsistent state
 	});
 
 	// create the components
@@ -4629,6 +4660,7 @@ int main(int argc, const char **argv)
 		const char *pError = "Failed to initialize the secure RNG.";
 		dbg_msg("secure", "%s", pError);
 		pClient->ShowMessageBox("Secure RNG Error", pError);
+		PerformAllCleanup();
 		return -1;
 	}
 
@@ -4663,9 +4695,7 @@ int main(int argc, const char **argv)
 			const char *pError = "Failed to register an interface.";
 			dbg_msg("client", "%s", pError);
 			pClient->ShowMessageBox("Kernel Error", pError);
-			delete pKernel;
-			pClient->~CClient();
-			free(pClient);
+			PerformAllCleanup();
 			return -1;
 		}
 	}
@@ -4683,18 +4713,21 @@ int main(int argc, const char **argv)
 	pClient->InitInterfaces();
 
 	// execute config file
-	IOHANDLE File = pStorage->OpenFile(CONFIG_FILE, IOFLAG_READ, IStorage::TYPE_ALL);
-	if(File)
+	if(pStorage->FileExists(CONFIG_FILE, IStorage::TYPE_ALL))
 	{
-		io_close(File);
-		pConsole->ExecuteFile(CONFIG_FILE);
+		if(!pConsole->ExecuteFile(CONFIG_FILE))
+		{
+			const char *pError = "Failed to load config from '" CONFIG_FILE "'.";
+			dbg_msg("client", "%s", pError);
+			pClient->ShowMessageBox("Config File Error", pError);
+			PerformAllCleanup();
+			return -1;
+		}
 	}
 
 	// execute autoexec file
-	File = pStorage->OpenFile(AUTOEXEC_CLIENT_FILE, IOFLAG_READ, IStorage::TYPE_ALL);
-	if(File)
+	if(pStorage->FileExists(AUTOEXEC_CLIENT_FILE, IStorage::TYPE_ALL))
 	{
-		io_close(File);
 		pConsole->ExecuteFile(AUTOEXEC_CLIENT_FILE);
 	}
 	else // fallback
@@ -4756,41 +4789,30 @@ int main(int argc, const char **argv)
 		str_format(aError, sizeof(aError), "Unable to initialize SDL base: %s", SDL_GetError());
 		dbg_msg("client", "%s", aError);
 		pClient->ShowMessageBox("SDL Error", aError);
+		PerformAllCleanup();
 		return -1;
 	}
-
-#ifndef CONF_PLATFORM_ANDROID
-	atexit(SDL_Quit);
-#endif
+	CleanerFunctions.push([]() { SDL_Quit(); });
 
 	// run the client
 	dbg_msg("client", "starting...");
 	pClient->Run();
 
-	bool Restarting = pClient->State() == CClient::STATE_RESTARTING;
+	const bool Restarting = pClient->State() == CClient::STATE_RESTARTING;
+	char aRestartBinaryPath[IO_MAX_PATH_LENGTH];
+	if(Restarting)
+	{
+		pStorage->GetBinaryPath(PLAT_CLIENT_EXEC, aRestartBinaryPath, sizeof(aRestartBinaryPath));
+	}
 
-	pClient->~CClient();
-	free(pClient);
-
-	NotificationsUninit();
-	secure_random_uninit();
+	PerformCleanup();
 
 	if(Restarting)
 	{
-		char aBuf[512];
-		shell_execute(pStorage->GetBinaryPath(PLAT_CLIENT_EXEC, aBuf, sizeof aBuf));
+		shell_execute(aRestartBinaryPath);
 	}
 
-	pKernel->Shutdown();
-	delete pKernel;
-
-	// shutdown SDL
-	SDL_Quit();
-
-#ifdef CONF_PLATFORM_ANDROID
-	// properly close this native thread, so globals are destructed
-	std::exit(0);
-#endif
+	PerformFinalCleanup();
 
 	return 0;
 }

src/engine/client/client.h

@@ -436,6 +436,7 @@ public:
 	static void Con_DummyResetInput(IConsole::IResult *pResult, void *pUserData);
 
 	static void Con_Quit(IConsole::IResult *pResult, void *pUserData);
+	static void Con_Restart(IConsole::IResult *pResult, void *pUserData);
 	static void Con_DemoPlay(IConsole::IResult *pResult, void *pUserData);
 	static void Con_DemoSpeed(IConsole::IResult *pResult, void *pUserData);
 	static void Con_Minimize(IConsole::IResult *pResult, void *pUserData);

src/engine/console.h

@@ -109,7 +109,7 @@ public:
 	virtual void ExecuteLine(const char *pStr, int ClientID = -1, bool InterpretSemicolons = true) = 0;
 	virtual void ExecuteLineFlag(const char *pStr, int FlasgMask, int ClientID = -1, bool InterpretSemicolons = true) = 0;
 	virtual void ExecuteLineStroked(int Stroke, const char *pStr, int ClientID = -1, bool InterpretSemicolons = true) = 0;
-	virtual void ExecuteFile(const char *pFilename, int ClientID = -1, bool LogFailure = false, int StorageType = IStorage::TYPE_ALL) = 0;
+	virtual bool ExecuteFile(const char *pFilename, int ClientID = -1, bool LogFailure = false, int StorageType = IStorage::TYPE_ALL) = 0;
 
 	virtual char *Format(char *pBuf, int Size, const char *pFrom, const char *pStr) = 0;
 	virtual void Print(int Level, const char *pFrom, const char *pStr, ColorRGBA PrintColor = gs_ConsoleDefaultColor) const = 0;

src/engine/server/main.cpp

@@ -153,10 +153,8 @@ int main(int argc, const char **argv)
 	pServer->RegisterCommands();
 
 	// execute autoexec file
-	IOHANDLE File = pStorage->OpenFile(AUTOEXEC_SERVER_FILE, IOFLAG_READ, IStorage::TYPE_ALL);
-	if(File)
+	if(pStorage->FileExists(AUTOEXEC_SERVER_FILE, IStorage::TYPE_ALL))
 	{
-		io_close(File);
 		pConsole->ExecuteFile(AUTOEXEC_SERVER_FILE);
 	}
 	else // fallback

src/engine/shared/console.cpp

@@ -622,15 +622,15 @@ void CConsole::ExecuteLineFlag(const char *pStr, int FlagMask, int ClientID, boo
 	m_FlagMask = Temp;
 }
 
-void CConsole::ExecuteFile(const char *pFilename, int ClientID, bool LogFailure, int StorageType)
+bool CConsole::ExecuteFile(const char *pFilename, int ClientID, bool LogFailure, int StorageType)
 {
 	// make sure that this isn't being executed already
 	for(CExecFile *pCur = m_pFirstExec; pCur; pCur = pCur->m_pPrev)
 		if(str_comp(pFilename, pCur->m_pFilename) == 0)
-			return;
+			return false;
 
 	if(!m_pStorage)
-		return;
+		return false;
 
 	// push this one to the stack
 	CExecFile ThisFile;
@@ -642,20 +642,22 @@ void CConsole::ExecuteFile(const char *pFilename, int ClientID, bool LogFailure,
 	// exec the file
 	IOHANDLE File = m_pStorage->OpenFile(pFilename, IOFLAG_READ | IOFLAG_SKIP_BOM, StorageType);
 
-	char aBuf[128];
+	bool Success = false;
+	char aBuf[32 + IO_MAX_PATH_LENGTH];
 	if(File)
 	{
-		char *pLine;
-		CLineReader Reader;
-
 		str_format(aBuf, sizeof(aBuf), "executing '%s'", pFilename);
 		Print(IConsole::OUTPUT_LEVEL_STANDARD, "console", aBuf);
+
+		CLineReader Reader;
 		Reader.Init(File);
 
+		char *pLine;
 		while((pLine = Reader.Get()))
 			ExecuteLine(pLine, ClientID);
 
 		io_close(File);
+		Success = true;
 	}
 	else if(LogFailure)
 	{
@@ -664,6 +666,7 @@ void CConsole::ExecuteFile(const char *pFilename, int ClientID, bool LogFailure,
 	}
 
 	m_pFirstExec = pPrev;
+	return Success;
 }
 
 void CConsole::Con_Echo(IResult *pResult, void *pUserData)

src/engine/shared/console.h

@@ -214,7 +214,7 @@ public:
 	bool LineIsValid(const char *pStr) override;
 	void ExecuteLine(const char *pStr, int ClientID = -1, bool InterpretSemicolons = true) override;
 	void ExecuteLineFlag(const char *pStr, int FlagMask, int ClientID = -1, bool InterpretSemicolons = true) override;
-	void ExecuteFile(const char *pFilename, int ClientID = -1, bool LogFailure = false, int StorageType = IStorage::TYPE_ALL) override;
+	bool ExecuteFile(const char *pFilename, int ClientID = -1, bool LogFailure = false, int StorageType = IStorage::TYPE_ALL) override;
 
 	char *Format(char *pBuf, int Size, const char *pFrom, const char *pStr) override;
 	void Print(int Level, const char *pFrom, const char *pStr, ColorRGBA PrintColor = gs_ConsoleDefaultColor) const override;

src/engine/shared/storage.cpp

@@ -289,19 +289,15 @@ public:
 				char aBuf[IO_MAX_PATH_LENGTH];
 				str_copy(m_aBinarydir, pArgv0, Pos + 1);
 				str_format(aBuf, sizeof(aBuf), "%s/" PLAT_SERVER_EXEC, m_aBinarydir);
-				IOHANDLE File = io_open(aBuf, IOFLAG_READ);
-				if(File)
+				if(fs_is_file(aBuf))
 				{
-					io_close(File);
 					return;
 				}
 #if defined(CONF_PLATFORM_MACOS)
 				str_append(m_aBinarydir, "/../../../DDNet-Server.app/Contents/MacOS", sizeof(m_aBinarydir));
 				str_format(aBuf, sizeof(aBuf), "%s/" PLAT_SERVER_EXEC, m_aBinarydir);
-				IOHANDLE FileBis = io_open(aBuf, IOFLAG_READ);
-				if(FileBis)
+				if(fs_is_file(aBuf))
 				{
-					io_close(FileBis);
 					return;
 				}
 #endif

src/game/client/components/menus_start.cpp

@@ -149,9 +149,8 @@ void CMenus::RenderStartMenu(CUIRect MainView)
 			{
 				m_ServerProcess.m_Process = shell_execute(aBuf);
 			}
-			else if(IOHANDLE File = io_open(aBuf, IOFLAG_READ))
+			else if(fs_is_file(aBuf))
 			{
-				io_close(File);
 				m_ServerProcess.m_Process = shell_execute(aBuf);
 			}
 			else

src/game/editor/editor.cpp

@@ -4975,10 +4975,8 @@ void CEditor::RenderFileDialog()
 			str_format(m_aFileSaveName, sizeof(m_aFileSaveName), "%s/%s", m_pFileDialogPath, m_FileDialogFileNameInput.GetString());
 			if(!str_comp(m_pFileDialogButtonText, "Save"))
 			{
-				IOHANDLE File = Storage()->OpenFile(m_aFileSaveName, IOFLAG_READ, IStorage::TYPE_SAVE);
-				if(File)
+				if(Storage()->FileExists(m_aFileSaveName, IStorage::TYPE_SAVE))
 				{
-					io_close(File);
 					m_PopupEventType = POPEVENT_SAVE;
 					m_PopupEventActivated = true;
 				}