From 9269a0ecc7267822bc5ac5af95ad4977bdc94fec Mon Sep 17 00:00:00 2001 From: ShadowNinja Date: Thu, 29 Oct 2015 14:48:10 -0400 Subject: [PATCH] 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 | 6 +++++ src/guiEngine.cpp | 10 ++++---- src/main.cpp | 22 +++++++++++------ src/mods.cpp | 1 + src/mods.h | 18 -------------- src/script/common/c_types.h | 4 ++-- src/script/cpp_api/s_async.cpp | 8 +++++-- src/script/cpp_api/s_base.cpp | 20 ++++++---------- src/script/cpp_api/s_base.h | 6 ++--- src/server.cpp | 43 ++++++++++------------------------ 10 files changed, 58 insertions(+), 80 deletions(-) diff --git a/src/exceptions.h b/src/exceptions.h index 6bf832828..4f18f70d9 100644 --- a/src/exceptions.h +++ b/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: */ diff --git a/src/guiEngine.cpp b/src/guiEngine.cpp index c616bc322..84bc8488e 100644 --- a/src/guiEngine.cpp +++ b/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; diff --git a/src/main.cpp b/src/main.cpp index 1fa9243fa..b24ece4bd 100644 --- a/src/main.cpp +++ b/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; } diff --git a/src/mods.cpp b/src/mods.cpp index 90e0816d9..be6e1e5d3 100644 --- a/src/mods.cpp +++ b/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 &symbols) diff --git a/src/mods.h b/src/mods.h index f35bd18db..12576516d 100644 --- a/src/mods.h +++ b/src/mods.h @@ -26,29 +26,11 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #include -#include #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; diff --git a/src/script/common/c_types.h b/src/script/common/c_types.h index 706470737..056f30251 100644 --- a/src/script/common/c_types.h +++ b/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) {} }; diff --git a/src/script/cpp_api/s_async.cpp b/src/script/cpp_api/s_async.cpp index 171632633..9bf3fcf49 100644 --- a/src/script/cpp_api/s_async.cpp +++ b/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); diff --git a/src/script/cpp_api/s_base.cpp b/src/script/cpp_api/s_base.cpp index 78b70e499..b40d31533 100644 --- a/src/script/cpp_api/s_base.cpp +++ b/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). diff --git a/src/script/cpp_api/s_base.h b/src/script/cpp_api/s_base.h index d490f7dfd..20f4bc11b 100644 --- a/src/script/cpp_api/s_base.h +++ b/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); diff --git a/src/server.cpp b/src/server.cpp index 09675dae3..327591c60 100644 --- a/src/server.cpp +++ b/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::iterator i = m_mods.begin(); - i != m_mods.end(); ++i) { - const ModSpec &mod = *i; + for (std::vector::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); } }