From 76141a0de1c9db55fd8238020cf5de4c144f36b8 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Mon, 10 Nov 2014 09:58:01 -0500 Subject: [PATCH] new code for lua protected calls in lua_kernel_base In initial commit of the new lua kernels, I introduced a problem by trying to use luaW_pcall and lua_pcall interchangeably via polymorphism. This doesn't work because their return types don't match, and it's alot of work to change luaW_pcall syntax. Besides this there's no reason we can't use the custom error handler everywhere. This commits adds protected_call and load_string functions to lua kernel base. These are an intended replacement for luaW_pcall, and replace the "run" function. They do better error reporting and allow to specify an error handler. The error reporting is very flexible -- by default we select a an error reporting function associated polymorphically to the lua kernel, so the in-game lua kernel can send chat messages, and others can do something else. However an arbitrary handler may be specified, and exceptions instead of logging may also be requested. --- src/game_errors.hpp | 8 ++ src/scripting/application_lua_kernel.cpp | 3 +- src/scripting/game_lua_kernel.cpp | 32 ++---- src/scripting/game_lua_kernel.hpp | 2 +- src/scripting/lua_kernel_base.cpp | 130 +++++++++++++++++------ src/scripting/lua_kernel_base.hpp | 24 +++-- 6 files changed, 135 insertions(+), 64 deletions(-) diff --git a/src/game_errors.hpp b/src/game_errors.hpp index e56b47dd0ad..35023f3823a 100644 --- a/src/game_errors.hpp +++ b/src/game_errors.hpp @@ -47,6 +47,14 @@ struct game_error : public error { game_error(const std::string& msg) : error("game_error: " + msg) {} }; +/** + * Error used to report an error in a lua script or in the lua interpreter. + */ +struct lua_error : public error { + lua_error(const std::string& msg) : error("lua_error: " + msg) {} + lua_error(const std::string& msg, const std::string& context) : error(context + ": " + msg) {} +}; + /** * Exception used to signal that the user has decided to abort a game, * and to load another game instead. diff --git a/src/scripting/application_lua_kernel.cpp b/src/scripting/application_lua_kernel.cpp index e4317e03409..ee31765c41c 100644 --- a/src/scripting/application_lua_kernel.cpp +++ b/src/scripting/application_lua_kernel.cpp @@ -104,8 +104,7 @@ void application_lua_kernel::call_script(const config & event_cfg) { luaW_pushconfig(L, event_cfg); //push the config as an argument - pcall_fcn_ptr pcall = pcall_fcn(); - if (!pcall(L, 1, 0)) //call the script from protected mode, there is one argument and we expect no return values. + if (!luaW_pcall(L, 1, 0)) //call the script from protected mode, there is one argument and we expect no return values. { WRN_LUA << "Got an error when executing script:\n" << lua_tostring(L,-1) << std::endl; } diff --git a/src/scripting/game_lua_kernel.cpp b/src/scripting/game_lua_kernel.cpp index 8d1337b3d45..b4dc5b9098a 100644 --- a/src/scripting/game_lua_kernel.cpp +++ b/src/scripting/game_lua_kernel.cpp @@ -153,6 +153,13 @@ void extract_preload_scripts(config const &game_config) preload_config = game_config.child("game_config"); } +void LuaKernel::log_error(char const * msg, char const * context) +{ + lua_kernel_base::log_error(msg, context); + chat_message(context, msg); +} + + namespace { /** * Stack storing the queued_event objects needed for calling WML actions. @@ -3717,14 +3724,6 @@ LuaKernel::LuaKernel(const config &cfg) lua_setfield(L, -2, "theme_items"); lua_pop(L, 1); - // Store the error handler. - lua_pushlightuserdata(L - , executeKey); - lua_getglobal(L, "debug"); - lua_getfield(L, -1, "traceback"); - lua_remove(L, -2); - lua_rawset(L, LUA_REGISTRYINDEX); - lua_settop(L, 0); } @@ -3795,10 +3794,10 @@ void LuaKernel::initialize() // Execute the preload scripts. game_config::load_config(preload_config); BOOST_FOREACH(const config &cfg, preload_scripts) { - execute(cfg["code"].str().c_str(), 0, 0); + run(cfg["code"].str().c_str()); } BOOST_FOREACH(const config &cfg, level_.child_range("lua")) { - execute(cfg["code"].str().c_str(), 0, 0); + run(cfg["code"].str().c_str()); } load_game(); @@ -3886,8 +3885,7 @@ void LuaKernel::save_game(config &cfg) * and the extra UMC ones. */ const std::string m = "Tag is already used: [" + i->key + "]"; - chat_message("Lua error", m); - ERR_LUA << m << '\n'; + log_error(m.c_str()); v.erase(i); continue; } @@ -3979,8 +3977,7 @@ bool LuaKernel::run_filter(char const *name, unit const &u) if(!luaW_getglobal(L, name, NULL)) { std::string message = std::string() + "function " + name + " not found"; - chat_message("Lua SUF Error", message); - ERR_LUA << "Lua SUF Error: " << message << std::endl; + log_error(message.c_str(), "Lua SUF Error"); //we pushed nothing and can safeley return. return false; } @@ -3998,13 +3995,6 @@ bool LuaKernel::run_filter(char const *name, unit const &u) return b; } -// This is needed because default args don't work through function pointers -static int luaW_pcall_default_args(lua_State * L, int a, int b) { - return luaW_pcall(L,a,b,false); -} - -pcall_fcn_ptr LuaKernel::pcall_fcn() { return &luaW_pcall_default_args; } //this causes the run() function to use luaW_pcall instead of lua_pcall - ai::lua_ai_context* LuaKernel::create_lua_ai_context(char const *code, ai::engine_lua *engine) { return ai::lua_ai_context::create(mState,code,engine); diff --git a/src/scripting/game_lua_kernel.hpp b/src/scripting/game_lua_kernel.hpp index 4eb30cc7395..ef65df15ab4 100644 --- a/src/scripting/game_lua_kernel.hpp +++ b/src/scripting/game_lua_kernel.hpp @@ -47,7 +47,7 @@ public: game_events::queued_event const &); bool run_filter(char const *name, unit const &u); - virtual pcall_fcn_ptr pcall_fcn(); + virtual void log_error(char const* msg, char const* context = "Lua error"); ai::lua_ai_context* create_lua_ai_context(char const *code, ai::engine_lua *engine); ai::lua_ai_action_handler* create_lua_ai_action_handler(char const *code, ai::lua_ai_context &context); diff --git a/src/scripting/lua_kernel_base.cpp b/src/scripting/lua_kernel_base.cpp index 1fee3d50ec2..6ad4870062b 100644 --- a/src/scripting/lua_kernel_base.cpp +++ b/src/scripting/lua_kernel_base.cpp @@ -20,6 +20,7 @@ #include "lua/lauxlib.h" #include "lua/lua.h" #include "lua/lualib.h" +#include "game_errors.hpp" #ifdef DEBUG_LUA #include "scripting/debug_lua.hpp" @@ -29,6 +30,7 @@ #include #include +#include static lg::log_domain log_scripting_lua("scripting/lua"); #define LOG_LUA LOG_STREAM(info, log_scripting_lua) @@ -82,6 +84,14 @@ lua_kernel_base::lua_kernel_base() } lua_pop(L, 1); + // Store the error handler. + lua_pushlightuserdata(L + , executeKey); + lua_getglobal(L, "debug"); + lua_getfield(L, -1, "traceback"); + lua_remove(L, -2); + lua_rawset(L, LUA_REGISTRYINDEX); + lua_settop(L, 0); } @@ -90,51 +100,105 @@ lua_kernel_base::~lua_kernel_base() lua_close(mState); } -// This is needed because lua_pcall is actually a macro -static int lua_pcall_fcn(lua_State * L, int a, int b) +void lua_kernel_base::log_error(char const * msg, char const * context) { - return lua_pcall(L,a,b,0); + ERR_LUA << context << ": " << msg; } -// In the "base" configuration we just want to call lua_pcall when running scripts. -pcall_fcn_ptr lua_kernel_base::pcall_fcn() { return &lua_pcall_fcn; } +void lua_kernel_base::throw_exception(char const * msg, char const * context) +{ + throw game::lua_error(msg, context); +} -/** - * Runs a script on a stack containing @a nArgs arguments. - * @return true if the script was successful and @a nRets return values are available. - */ -bool lua_kernel_base::execute(char const *prog, int nArgs, int nRets) +bool lua_kernel_base::protected_call(int nArgs, int nRets) +{ + error_handler eh = boost::bind(&lua_kernel_base::log_error, this, _1, _2 ); + return protected_call(nArgs, nRets, eh); +} + +bool lua_kernel_base::load_string(char const * prog) +{ + error_handler eh = boost::bind(&lua_kernel_base::log_error, this, _1, _2 ); + return load_string(prog, eh); +} + +bool lua_kernel_base::protected_call(int nArgs, int nRets, error_handler e_h) { lua_State *L = mState; - // Compile script into a variadic function. - int res = luaL_loadstring(L, prog); - if (res) - { - char const *m = lua_tostring(L, -1); - chat_message("Lua error", m); - ERR_LUA << m << '\n'; - lua_pop(L, 1); - return true; + // Load the error handler before the function and its arguments. + lua_pushlightuserdata(L + , executeKey); + + lua_rawget(L, LUA_REGISTRYINDEX); + lua_insert(L, -2 - nArgs); + + int error_handler_index = lua_gettop(L) - nArgs - 1; + + // Call the function. + int errcode = lua_pcall(L, nArgs, nRets, -2 - nArgs); + tlua_jailbreak_exception::rethrow(); + + // Remove the error handler. + lua_remove(L, error_handler_index); + + if (errcode != LUA_OK) { + std::string message = lua_tostring(L, -1); + + std::string context = "When executing, "; + if (errcode == LUA_ERRRUN) { + context += "Lua runtime error: "; + } else if (errcode == LUA_ERRERR) { + context += "Lua error in attached debugger: "; + } else if (errcode == LUA_ERRMEM) { + context += "Lua out of memory error: "; + } else if (errcode == LUA_ERRGCMM) { + context += "Lua error in garbage collection metamethod: "; + } else { + context += "unknown lua error: "; + } + + e_h(message.c_str(), context.c_str()); + + return false; } - // Place the function before its arguments. - if (nArgs) - lua_insert(L, -1 - nArgs); - - pcall_fcn_ptr f = pcall_fcn(); - return f(L, nArgs, nRets); + return true; } +bool lua_kernel_base::load_string(char const * prog, error_handler e_h) +{ + int errcode = luaL_loadstring(mState, prog); + if (errcode != LUA_OK) { + std::string msg = lua_tostring(mState, -1); + + std::string context = "When parsing a string to lua, "; + + if (errcode == LUA_ERRSYNTAX) { + msg += " a syntax error: "; + } else if(errcode == LUA_ERRMEM){ + msg += " a memory error: "; + } else if(errcode == LUA_ERRGCMM) { + msg += " an error in garbage collection metamethod: "; + } else { + msg += " an unknown error: "; + } + + e_h(msg.c_str(), context.c_str()); + + return false; + } + return true; +} + +// Call load_string and protected call. Make them throw exceptions, and if we catch one, reformat it with signature for this function and log it. void lua_kernel_base::run(const char * prog) { - if (execute(prog, 0, 0)) { - lua_State *L = mState; - - char const *m = lua_tostring(L, -1); - ERR_LUA << "lua_kernel::run(): " << m << '\n'; - lua_pop(L,1); - - //execute("print(debug.traceback())",0,0); + try { + error_handler eh = boost::bind(&lua_kernel_base::throw_exception, this, _1, _2 ); + load_string(prog, eh); + protected_call(0, 0, eh); + } catch (game::lua_error & e) { + lua_kernel_base::log_error(e.what(), "In function lua_kernel::run()"); } } diff --git a/src/scripting/lua_kernel_base.hpp b/src/scripting/lua_kernel_base.hpp index 2c09d2b6c7d..8fe53ed8d93 100644 --- a/src/scripting/lua_kernel_base.hpp +++ b/src/scripting/lua_kernel_base.hpp @@ -16,25 +16,35 @@ #define SCRIPTING_LUA_KERNEL_BASE_HPP #include // for string +#include "utils/boost_function_guarded.hpp" struct lua_State; -typedef int (*pcall_fcn_ptr)(lua_State *, int, int); - class lua_kernel_base { -protected: - lua_State *mState; - bool execute(char const *, int, int); public: lua_kernel_base(); virtual ~lua_kernel_base(); - /** Runs a plain script. */ + /** Runs a plain script. Doesn't throw lua_error.*/ void run(char const *prog); void load_package(); - virtual pcall_fcn_ptr pcall_fcn(); //when running scripts, in the "base" kernel type we should just use pcall. But for the in-game kernel, we want to call the luaW_pcall function instead which extends it using things specific to that api, and returns errors on a WML channel + virtual void log_error(char const* msg, char const* context = "Lua error"); + virtual void throw_exception(char const* msg, char const* context = "Lua error"); //throws game::lua_error + + typedef boost::function error_handler; + +protected: + lua_State *mState; + + // Execute a protected call. Error handler is called in case of an error, using syntax for log_error and throw_exception above. Returns true if successful. + bool protected_call(int nArgs, int nRets, error_handler); + // Load a string onto the stack as a function. Returns true if successful, error handler is called if not. + bool load_string(char const * prog, error_handler); + + virtual bool protected_call(int nArgs, int nRets); // select default error handler polymorphically + virtual bool load_string(char const * prog); // select default error handler polymorphically }; #endif