Fix server crashing on Lua errors

Previously, the server called FATAL_ERROR when a Lua error occured.
This caused a (mostly useless) core dump.
The server now simply throws an exception, which is caught and printed before
exiting with a non-zero return value.
This also fixes a number of instances where errors were logged multiple times.

src/exceptions.h

@@ -125,6 +125,12 @@ public:
 	PrngException(std::string s): BaseException(s) {}
 };
 
+class ModError : public BaseException {
+public:
+	ModError(const std::string &s): BaseException(s) {}
+};
+
+
 /*
 	Some "old-style" interrupts:
 */

src/guiEngine.cpp

@@ -238,13 +238,13 @@ bool GUIEngine::loadMainMenuScript()
 	}
 
 	std::string script = porting::path_share + DIR_DELIM "builtin" + DIR_DELIM "init.lua";
-	if (m_script->loadScript(script)) {
+	try {
+		m_script->loadScript(script);
 		// Menu script loaded
 		return true;
-	} else {
-		infostream
-			<< "GUIEngine: execution of menu script in: \""
-			<< m_scriptdir << "\" failed!" << std::endl;
+	} catch (const ModError &e) {
+		errorstream << "GUIEngine: execution of menu script failed: "
+			<< e.what() << std::endl;
 	}
 
 	return false;

src/main.cpp

@@ -816,14 +816,22 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
 	if (cmd_args.exists("migrate"))
 		return migrate_database(game_params, cmd_args);
 
-	// Create server
-	Server server(game_params.world_path, game_params.game_spec, false,
-		bind_addr.isIPv6());
-	server.start(bind_addr);
+	try {
+		// Create server
+		Server server(game_params.world_path, game_params.game_spec, false,
+			bind_addr.isIPv6());
+		server.start(bind_addr);
 
-	// Run server
-	bool &kill = *porting::signal_handler_killstatus();
-	dedicated_server_loop(server, kill);
+		// Run server
+		bool &kill = *porting::signal_handler_killstatus();
+		dedicated_server_loop(server, kill);
+	} catch (const ModError &e) {
+		errorstream << "ModError: " << e.what() << std::endl;
+		return false;
+	} catch (const ServerError &e) {
+		errorstream << "ServerError: " << e.what() << std::endl;
+		return false;
+	}
 
 	return true;
 }

src/mods.cpp

@@ -27,6 +27,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "settings.h"
 #include "strfnd.h"
 #include "convert_json.h"
+#include "exceptions.h"
 
 static bool parseDependsLine(std::istream &is,
 		std::string &dep, std::set<char> &symbols)

src/mods.h

@@ -26,29 +26,11 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <vector>
 #include <string>
 #include <map>
-#include <exception>
 #include "json/json.h"
 #include "config.h"
 
 #define MODNAME_ALLOWED_CHARS "abcdefghijklmnopqrstuvwxyz0123456789_"
 
-class ModError : public std::exception
-{
-public:
-	ModError(const std::string &s)
-	{
-		m_s = "ModError: ";
-		m_s += s;
-	}
-	virtual ~ModError() throw()
-	{}
-	virtual const char * what() const throw()
-	{
-		return m_s.c_str();
-	}
-	std::string m_s;
-};
-
 struct ModSpec
 {
 	std::string name;

src/script/common/c_types.h

@@ -52,10 +52,10 @@ public:
 	}
 };
 
-class LuaError : public ServerError
+class LuaError : public ModError
 {
 public:
-	LuaError(const std::string &s) : ServerError(s) {}
+	LuaError(const std::string &s) : ModError(s) {}
 };
 
 

src/script/cpp_api/s_async.cpp

@@ -243,8 +243,12 @@ void* AsyncWorkerThread::run()
 	lua_State *L = getStack();
 
 	std::string script = getServer()->getBuiltinLuaPath() + DIR_DELIM + "init.lua";
-	if (!loadScript(script)) {
-		FATAL_ERROR("execution of async base environment failed!");
+	try {
+		loadScript(script);
+	} catch (const ModError &e) {
+		errorstream << "Execution of async base environment failed: "
+			<< e.what() << std::endl;
+		FATAL_ERROR("Execution of async base environment failed");
 	}
 
 	int error_handler = PUSH_ERROR_HANDLER(L);

src/script/cpp_api/s_base.cpp

@@ -119,15 +119,15 @@ ScriptApiBase::~ScriptApiBase()
 	lua_close(m_luastack);
 }
 
-bool ScriptApiBase::loadMod(const std::string &script_path,
-		const std::string &mod_name, std::string *error)
+void ScriptApiBase::loadMod(const std::string &script_path,
+		const std::string &mod_name)
 {
 	ModNameStorer mod_name_storer(getStack(), mod_name);
 
-	return loadScript(script_path, error);
+	loadScript(script_path);
 }
 
-bool ScriptApiBase::loadScript(const std::string &script_path, std::string *error)
+void ScriptApiBase::loadScript(const std::string &script_path)
 {
 	verbosestream << "Loading and running script from " << script_path << std::endl;
 
@@ -144,17 +144,11 @@ bool ScriptApiBase::loadScript(const std::string &script_path, std::string *erro
 	ok = ok && !lua_pcall(L, 0, 0, error_handler);
 	if (!ok) {
 		std::string error_msg = lua_tostring(L, -1);
-		if (error)
-			*error = error_msg;
-		errorstream << "========== ERROR FROM LUA ===========" << std::endl
-			<< "Failed to load and run script from " << std::endl
-			<< script_path << ":" << std::endl << std::endl
-			<< error_msg << std::endl << std::endl
-			<< "======= END OF ERROR FROM LUA ========" << std::endl;
-		lua_pop(L, 1); // Pop error message from stack
+		lua_pop(L, 2); // Pop error message and error handler
+		throw ModError("Failed to load and run script from " +
+				script_path + ":\n" + error_msg);
 	}
 	lua_pop(L, 1); // Pop error handler
-	return ok;
 }
 
 // Push the list of callbacks (a lua table).

src/script/cpp_api/s_base.h

@@ -63,9 +63,9 @@ public:
 	ScriptApiBase();
 	virtual ~ScriptApiBase();
 
-	bool loadMod(const std::string &script_path, const std::string &mod_name,
-		std::string *error=NULL);
-	bool loadScript(const std::string &script_path, std::string *error=NULL);
+	// These throw a ModError on failure
+	void loadMod(const std::string &script_path, const std::string &mod_name);
+	void loadScript(const std::string &script_path);
 
 	void runCallbacksRaw(int nargs,
 		RunCallbacksMode mode, const char *fxn);

src/server.cpp

@@ -276,11 +276,8 @@ Server::Server(
 	m_script = new GameScripting(this);
 
 	std::string script_path = getBuiltinLuaPath() + DIR_DELIM "init.lua";
-	std::string error_msg;
 
-	if (!m_script->loadMod(script_path, BUILTIN_MOD_NAME, &error_msg))
-		throw ModError("Failed to load and run " + script_path
-				+ "\nError from Lua:\n" + error_msg);
+	m_script->loadMod(script_path, BUILTIN_MOD_NAME);
 
 	// Print mods
 	infostream << "Server: Loading mods: ";
@@ -291,26 +288,18 @@ Server::Server(
 	}
 	infostream << std::endl;
 	// Load and run "mod" scripts
-	for (std::vector<ModSpec>::iterator i = m_mods.begin();
-			i != m_mods.end(); ++i) {
-		const ModSpec &mod = *i;
+	for (std::vector<ModSpec>::iterator it = m_mods.begin();
+			it != m_mods.end(); ++it) {
+		const ModSpec &mod = *it;
 		if (!string_allowed(mod.name, MODNAME_ALLOWED_CHARS)) {
-			std::ostringstream err;
-			err << "Error loading mod \"" << mod.name
-					<< "\": mod_name does not follow naming conventions: "
-					<< "Only chararacters [a-z0-9_] are allowed." << std::endl;
-			errorstream << err.str().c_str();
-			throw ModError(err.str());
+			throw ModError("Error loading mod \"" + mod.name +
+				"\": Mod name does not follow naming conventions: "
+				"Only chararacters [a-z0-9_] are allowed.");
 		}
-		std::string script_path = mod.path + DIR_DELIM "init.lua";
+		std::string script_path = mod.path + DIR_DELIM + "init.lua";
 		infostream << "  [" << padStringRight(mod.name, 12) << "] [\""
 				<< script_path << "\"]" << std::endl;
-		if (!m_script->loadMod(script_path, mod.name, &error_msg)) {
-			errorstream << "Server: Failed to load and run "
-					<< script_path << std::endl;
-			throw ModError("Failed to load and run " + script_path
-					+ "\nError from Lua:\n" + error_msg);
-		}
+		m_script->loadMod(script_path, mod.name);
 	}
 
 	// Read Textures and calculate sha1 sums
@@ -483,7 +472,7 @@ void Server::step(float dtime)
 {
 	DSTACK(FUNCTION_NAME);
 	// Limit a bit
-	if(dtime > 2.0)
+	if (dtime > 2.0)
 		dtime = 2.0;
 	{
 		MutexAutoLock lock(m_step_dtime_mutex);
@@ -491,19 +480,13 @@ void Server::step(float dtime)
 	}
 	// Throw if fatal error occurred in thread
 	std::string async_err = m_async_fatal_error.get();
-	if(async_err != "") {
-		if (m_simple_singleplayer_mode) {
-			throw ServerError(async_err);
-		}
-		else {
+	if (!async_err.empty()) {
+		if (!m_simple_singleplayer_mode) {
 			m_env->kickAllPlayers(SERVER_ACCESSDENIED_CRASH,
 				g_settings->get("kick_msg_crash"),
 				g_settings->getBool("ask_reconnect_on_crash"));
-			errorstream << "UNRECOVERABLE error occurred. Stopping server. "
-					<< "Please fix the following error:" << std::endl
-					<< async_err << std::endl;
-			FATAL_ERROR(async_err.c_str());
 		}
+		throw ServerError(async_err);
 	}
 }