From 9269a0ecc7267822bc5ac5af95ad4977bdc94fec Mon Sep 17 00:00:00 2001
From: ShadowNinja <shadowninja@minetest.net>
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 | 24 ++++++++++++-------
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, 59 insertions(+), 81 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);
-
- // Run server
- bool &kill = *porting::signal_handler_killstatus();
- dedicated_server_loop(server, kill);
+ 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);
+ } 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<char> &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 <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;
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<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);
}
}
--
2.25.1