From 6a1424f2b18520f40ba8cfd12f7988f6b33db9a6 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sat, 28 Aug 2021 12:15:12 +0200 Subject: [PATCH] Async-related script cleanups --- builtin/async/init.lua | 10 +-- src/gui/guiEngine.cpp | 7 -- src/gui/guiEngine.h | 4 -- src/script/cpp_api/s_async.cpp | 108 ++++++++++++++++-------------- src/script/cpp_api/s_async.h | 39 ++++++----- src/script/cpp_api/s_base.cpp | 4 -- src/script/cpp_api/s_base.h | 5 +- src/script/cpp_api/s_security.cpp | 21 ++---- src/script/lua_api/l_mainmenu.cpp | 11 +-- src/script/lua_api/l_server.cpp | 29 +------- src/script/lua_api/l_server.h | 6 -- src/script/lua_api/l_util.cpp | 30 +++++++++ src/script/lua_api/l_util.h | 6 ++ src/script/scripting_mainmenu.cpp | 6 +- src/script/scripting_mainmenu.h | 5 +- 15 files changed, 135 insertions(+), 156 deletions(-) diff --git a/builtin/async/init.lua b/builtin/async/init.lua index 1b2549685..3803994d6 100644 --- a/builtin/async/init.lua +++ b/builtin/async/init.lua @@ -1,16 +1,10 @@ core.log("info", "Initializing Asynchronous environment") -function core.job_processor(serialized_func, serialized_param) - local func = loadstring(serialized_func) +function core.job_processor(func, serialized_param) local param = core.deserialize(serialized_param) - local retval = nil - if type(func) == "function" then - retval = core.serialize(func(param)) - else - core.log("error", "ASYNC WORKER: Unable to deserialize function") - end + local retval = core.serialize(func(param)) return retval or core.serialize(nil) end diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp index 694baf482..b3808535c 100644 --- a/src/gui/guiEngine.cpp +++ b/src/gui/guiEngine.cpp @@ -614,10 +614,3 @@ void GUIEngine::stopSound(s32 handle) { m_sound_manager->stopSound(handle); } - -/******************************************************************************/ -unsigned int GUIEngine::queueAsync(const std::string &serialized_func, - const std::string &serialized_params) -{ - return m_script->queueAsync(serialized_func, serialized_params); -} diff --git a/src/gui/guiEngine.h b/src/gui/guiEngine.h index 70abce181..d7e6485ef 100644 --- a/src/gui/guiEngine.h +++ b/src/gui/guiEngine.h @@ -175,10 +175,6 @@ public: return m_scriptdir; } - /** pass async callback to scriptengine **/ - unsigned int queueAsync(const std::string &serialized_fct, - const std::string &serialized_params); - private: /** find and run the main menu script */ diff --git a/src/script/cpp_api/s_async.cpp b/src/script/cpp_api/s_async.cpp index 0619b32c0..dacdcd75a 100644 --- a/src/script/cpp_api/s_async.cpp +++ b/src/script/cpp_api/s_async.cpp @@ -32,20 +32,19 @@ extern "C" { #include "filesys.h" #include "porting.h" #include "common/c_internal.h" +#include "lua_api/l_base.h" /******************************************************************************/ AsyncEngine::~AsyncEngine() { - // Request all threads to stop for (AsyncWorkerThread *workerThread : workerThreads) { workerThread->stop(); } - // Wake up all threads - for (std::vector::iterator it = workerThreads.begin(); - it != workerThreads.end(); ++it) { + for (auto it : workerThreads) { + (void)it; jobQueueCounter.post(); } @@ -68,6 +67,7 @@ AsyncEngine::~AsyncEngine() /******************************************************************************/ void AsyncEngine::registerStateInitializer(StateInitializer func) { + FATAL_ERROR_IF(initDone, "Initializer may not be registered after init"); stateInitializers.push_back(func); } @@ -85,36 +85,36 @@ void AsyncEngine::initialize(unsigned int numEngines) } /******************************************************************************/ -unsigned int AsyncEngine::queueAsyncJob(const std::string &func, - const std::string ¶ms) +u32 AsyncEngine::queueAsyncJob(std::string &&func, std::string &¶ms, + const std::string &mod_origin) { jobQueueMutex.lock(); - LuaJobInfo toAdd; - toAdd.id = jobIdCounter++; - toAdd.serializedFunction = func; - toAdd.serializedParams = params; + u32 jobId = jobIdCounter++; - jobQueue.push_back(toAdd); + jobQueue.emplace_back(); + auto &to_add = jobQueue.back(); + to_add.id = jobId; + to_add.function = std::move(func); + to_add.params = std::move(params); + to_add.mod_origin = mod_origin; jobQueueCounter.post(); - jobQueueMutex.unlock(); - - return toAdd.id; + return jobId; } /******************************************************************************/ -LuaJobInfo AsyncEngine::getJob() +bool AsyncEngine::getJob(LuaJobInfo *job) { jobQueueCounter.wait(); jobQueueMutex.lock(); - LuaJobInfo retval; + bool retval = false; if (!jobQueue.empty()) { - retval = jobQueue.front(); + *job = std::move(jobQueue.front()); jobQueue.pop_front(); - retval.valid = true; + retval = true; } jobQueueMutex.unlock(); @@ -122,10 +122,10 @@ LuaJobInfo AsyncEngine::getJob() } /******************************************************************************/ -void AsyncEngine::putJobResult(const LuaJobInfo &result) +void AsyncEngine::putJobResult(LuaJobInfo &&result) { resultQueueMutex.lock(); - resultQueue.push_back(result); + resultQueue.emplace_back(std::move(result)); resultQueueMutex.unlock(); } @@ -134,26 +134,30 @@ void AsyncEngine::step(lua_State *L) { int error_handler = PUSH_ERROR_HANDLER(L); lua_getglobal(L, "core"); - resultQueueMutex.lock(); + + ScriptApiBase *script = ModApiBase::getScriptApiBase(L); + + MutexAutoLock autolock(resultQueueMutex); while (!resultQueue.empty()) { - LuaJobInfo jobDone = resultQueue.front(); + LuaJobInfo j = std::move(resultQueue.front()); resultQueue.pop_front(); lua_getfield(L, -1, "async_event_handler"); - - if (lua_isnil(L, -1)) { + if (lua_isnil(L, -1)) FATAL_ERROR("Async event handler does not exist!"); - } - luaL_checktype(L, -1, LUA_TFUNCTION); - lua_pushinteger(L, jobDone.id); - lua_pushlstring(L, jobDone.serializedResult.data(), - jobDone.serializedResult.size()); + lua_pushinteger(L, j.id); + lua_pushlstring(L, j.result.data(), j.result.size()); - PCALL_RESL(L, lua_pcall(L, 2, 0, error_handler)); + // Call handler + const char *origin = j.mod_origin.empty() ? nullptr : j.mod_origin.c_str(); + script->setOriginDirect(origin); + int result = lua_pcall(L, 2, 0, error_handler); + if (result) + script_error(L, result, origin, ""); } - resultQueueMutex.unlock(); + lua_pop(L, 2); // Pop core and error handler } @@ -168,8 +172,8 @@ void AsyncEngine::prepareEnvironment(lua_State* L, int top) /******************************************************************************/ AsyncWorkerThread::AsyncWorkerThread(AsyncEngine* jobDispatcher, const std::string &name) : - Thread(name), ScriptApiBase(ScriptingType::Async), + Thread(name), jobDispatcher(jobDispatcher) { lua_State *L = getStack(); @@ -196,9 +200,9 @@ void* AsyncWorkerThread::run() { lua_State *L = getStack(); - std::string script = getServer()->getBuiltinLuaPath() + DIR_DELIM + "init.lua"; try { - loadScript(script); + loadMod(getServer()->getBuiltinLuaPath() + DIR_DELIM + "init.lua", + BUILTIN_MOD_NAME); } catch (const ModError &e) { errorstream << "Execution of async base environment failed: " << e.what() << std::endl; @@ -213,44 +217,44 @@ void* AsyncWorkerThread::run() } // Main loop + LuaJobInfo j; while (!stopRequested()) { // Wait for job - LuaJobInfo toProcess = jobDispatcher->getJob(); - - if (!toProcess.valid || stopRequested()) { + if (!jobDispatcher->getJob(&j) || stopRequested()) continue; - } lua_getfield(L, -1, "job_processor"); - if (lua_isnil(L, -1)) { + if (lua_isnil(L, -1)) FATAL_ERROR("Unable to get async job processor!"); - } - luaL_checktype(L, -1, LUA_TFUNCTION); - // Call it - lua_pushlstring(L, - toProcess.serializedFunction.data(), - toProcess.serializedFunction.size()); - lua_pushlstring(L, - toProcess.serializedParams.data(), - toProcess.serializedParams.size()); + if (luaL_loadbuffer(L, j.function.data(), j.function.size(), "=(async)")) { + errorstream << "ASYNC WORKER: Unable to deserialize function" << std::endl; + lua_pushnil(L); + } + lua_pushlstring(L, j.params.data(), j.params.size()); + // Call it + setOriginDirect(j.mod_origin.empty() ? nullptr : j.mod_origin.c_str()); int result = lua_pcall(L, 2, 1, error_handler); if (result) { - PCALL_RES(result); - toProcess.serializedResult = ""; + try { + scriptError(result, ""); + } catch (const ModError &e) { + errorstream << e.what() << std::endl; + } } else { // Fetch result size_t length; const char *retval = lua_tolstring(L, -1, &length); - toProcess.serializedResult = std::string(retval, length); + j.result.assign(retval, length); } lua_pop(L, 1); // Pop retval // Put job result - jobDispatcher->putJobResult(toProcess); + if (!j.result.empty()) + jobDispatcher->putJobResult(std::move(j)); } lua_pop(L, 2); // Pop core and error handler diff --git a/src/script/cpp_api/s_async.h b/src/script/cpp_api/s_async.h index 99a4f891c..697cb0221 100644 --- a/src/script/cpp_api/s_async.h +++ b/src/script/cpp_api/s_async.h @@ -21,7 +21,6 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include -#include #include "threading/semaphore.h" #include "threading/thread.h" @@ -39,26 +38,29 @@ struct LuaJobInfo { LuaJobInfo() = default; - // Function to be called in async environment - std::string serializedFunction = ""; - // Parameter to be passed to function - std::string serializedParams = ""; - // Result of function call - std::string serializedResult = ""; + // Function to be called in async environment (from string.dump) + std::string function; + // Parameter to be passed to function (serialized) + std::string params; + // Result of function call (serialized) + std::string result; + // Name of the mod who invoked this call + std::string mod_origin; // JobID used to identify a job and match it to callback - unsigned int id = 0; - - bool valid = false; + u32 id; }; // Asynchronous working environment -class AsyncWorkerThread : public Thread, public ScriptApiBase { +class AsyncWorkerThread : public Thread, virtual public ScriptApiBase { + friend class AsyncEngine; public: - AsyncWorkerThread(AsyncEngine* jobDispatcher, const std::string &name); virtual ~AsyncWorkerThread(); void *run(); +protected: + AsyncWorkerThread(AsyncEngine* jobDispatcher, const std::string &name); + private: AsyncEngine *jobDispatcher = nullptr; }; @@ -89,7 +91,8 @@ public: * @param params Serialized parameters * @return jobid The job is queued */ - unsigned int queueAsyncJob(const std::string &func, const std::string ¶ms); + u32 queueAsyncJob(std::string &&func, std::string &¶ms, + const std::string &mod_origin = ""); /** * Engine step to process finished jobs @@ -102,15 +105,16 @@ protected: /** * Get a Job from queue to be processed * this function blocks until a job is ready - * @return a job to be processed + * @param job a job to be processed + * @return whether a job was available */ - LuaJobInfo getJob(); + bool getJob(LuaJobInfo *job); /** * Put a Job result back to result queue * @param result result of completed job */ - void putJobResult(const LuaJobInfo &result); + void putJobResult(LuaJobInfo &&result); /** * Initialize environment with current registred functions @@ -129,11 +133,10 @@ private: std::vector stateInitializers; // Internal counter to create job IDs - unsigned int jobIdCounter = 0; + u32 jobIdCounter = 0; // Mutex to protect job queue std::mutex jobQueueMutex; - // Job queue std::deque jobQueue; diff --git a/src/script/cpp_api/s_base.cpp b/src/script/cpp_api/s_base.cpp index f965975a3..921f713c0 100644 --- a/src/script/cpp_api/s_base.cpp +++ b/src/script/cpp_api/s_base.cpp @@ -331,13 +331,9 @@ void ScriptApiBase::setOriginDirect(const char *origin) void ScriptApiBase::setOriginFromTableRaw(int index, const char *fxn) { -#ifdef SCRIPTAPI_DEBUG lua_State *L = getStack(); - m_last_run_mod = lua_istable(L, index) ? getstringfield_default(L, index, "mod_origin", "") : ""; - //printf(">>>> running %s for mod: %s\n", fxn, m_last_run_mod.c_str()); -#endif } /* diff --git a/src/script/cpp_api/s_base.h b/src/script/cpp_api/s_base.h index 86f7f7bac..7a8ebc85a 100644 --- a/src/script/cpp_api/s_base.h +++ b/src/script/cpp_api/s_base.h @@ -39,7 +39,6 @@ extern "C" { #include "config.h" #define SCRIPTAPI_LOCK_DEBUG -#define SCRIPTAPI_DEBUG // MUST be an invalid mod name so that mods can't // use that name to bypass security! @@ -108,7 +107,9 @@ public: Client* getClient(); #endif - std::string getOrigin() { return m_last_run_mod; } + // IMPORTANT: these cannot be used for any security-related uses, they exist + // only to enrich error messages + const std::string &getOrigin() { return m_last_run_mod; } void setOriginDirect(const char *origin); void setOriginFromTableRaw(int index, const char *fxn); diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp index add7b1658..580042ec2 100644 --- a/src/script/cpp_api/s_security.cpp +++ b/src/script/cpp_api/s_security.cpp @@ -18,7 +18,7 @@ with this program; if not, write to the Free Software Foundation, Inc., */ #include "cpp_api/s_security.h" - +#include "lua_api/l_base.h" #include "filesys.h" #include "porting.h" #include "server.h" @@ -538,15 +538,8 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, if (!removed.empty()) abs_path += DIR_DELIM + removed; - // Get server from registry - lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_SCRIPTAPI); - ScriptApiBase *script; -#if INDIRECT_SCRIPTAPI_RIDX - script = (ScriptApiBase *) *(void**)(lua_touserdata(L, -1)); -#else - script = (ScriptApiBase *) lua_touserdata(L, -1); -#endif - lua_pop(L, 1); + // Get gamedef from registry + ScriptApiBase *script = ModApiBase::getScriptApiBase(L); const IGameDef *gamedef = script->getGameDef(); if (!gamedef) return false; @@ -669,13 +662,7 @@ int ScriptApiSecurity::sl_g_load(lua_State *L) int ScriptApiSecurity::sl_g_loadfile(lua_State *L) { #ifndef SERVER - lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_SCRIPTAPI); -#if INDIRECT_SCRIPTAPI_RIDX - ScriptApiBase *script = (ScriptApiBase *) *(void**)(lua_touserdata(L, -1)); -#else - ScriptApiBase *script = (ScriptApiBase *) lua_touserdata(L, -1); -#endif - lua_pop(L, 1); + ScriptApiBase *script = ModApiBase::getScriptApiBase(L); // Client implementation if (script->getType() == ScriptingType::Client) { diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp index ad00de1c4..6e9a5c34f 100644 --- a/src/script/lua_api/l_mainmenu.cpp +++ b/src/script/lua_api/l_mainmenu.cpp @@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "lua_api/l_internal.h" #include "common/c_content.h" #include "cpp_api/s_async.h" +#include "scripting_mainmenu.h" #include "gui/guiEngine.h" #include "gui/guiMainMenu.h" #include "gui/guiKeyChangeMenu.h" @@ -816,20 +817,20 @@ int ModApiMainMenu::l_open_dir(lua_State *L) /******************************************************************************/ int ModApiMainMenu::l_do_async_callback(lua_State *L) { - GUIEngine* engine = getGuiEngine(L); + MainMenuScripting *script = getScriptApi(L); size_t func_length, param_length; const char* serialized_func_raw = luaL_checklstring(L, 1, &func_length); - const char* serialized_param_raw = luaL_checklstring(L, 2, ¶m_length); sanity_check(serialized_func_raw != NULL); sanity_check(serialized_param_raw != NULL); - std::string serialized_func = std::string(serialized_func_raw, func_length); - std::string serialized_param = std::string(serialized_param_raw, param_length); + u32 jobId = script->queueAsync( + std::string(serialized_func_raw, func_length), + std::string(serialized_param_raw, param_length)); - lua_pushinteger(L, engine->queueAsync(serialized_func, serialized_param)); + lua_pushinteger(L, jobId); return 1; } diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp index bf5292521..9866e0bc8 100644 --- a/src/script/lua_api/l_server.cpp +++ b/src/script/lua_api/l_server.cpp @@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "common/c_content.h" #include "cpp_api/s_base.h" #include "cpp_api/s_security.h" +#include "scripting_server.h" #include "server.h" #include "environment.h" #include "remoteplayer.h" @@ -498,31 +499,6 @@ int ModApiServer::l_notify_authentication_modified(lua_State *L) return 0; } -// get_last_run_mod() -int ModApiServer::l_get_last_run_mod(lua_State *L) -{ - NO_MAP_LOCK_REQUIRED; - lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME); - std::string current_mod = readParam(L, -1, ""); - if (current_mod.empty()) { - lua_pop(L, 1); - lua_pushstring(L, getScriptApiBase(L)->getOrigin().c_str()); - } - return 1; -} - -// set_last_run_mod(modname) -int ModApiServer::l_set_last_run_mod(lua_State *L) -{ - NO_MAP_LOCK_REQUIRED; -#ifdef SCRIPTAPI_DEBUG - const char *mod = lua_tostring(L, 1); - getScriptApiBase(L)->setOriginDirect(mod); - //printf(">>>> last mod set from Lua: %s\n", mod); -#endif - return 0; -} - void ModApiServer::Initialize(lua_State *L, int top) { API_FCT(request_shutdown); @@ -555,7 +531,4 @@ void ModApiServer::Initialize(lua_State *L, int top) API_FCT(remove_player); API_FCT(unban_player_or_ip); API_FCT(notify_authentication_modified); - - API_FCT(get_last_run_mod); - API_FCT(set_last_run_mod); } diff --git a/src/script/lua_api/l_server.h b/src/script/lua_api/l_server.h index 2df180b17..fb7a851f4 100644 --- a/src/script/lua_api/l_server.h +++ b/src/script/lua_api/l_server.h @@ -103,12 +103,6 @@ private: // notify_authentication_modified(name) static int l_notify_authentication_modified(lua_State *L); - // get_last_run_mod() - static int l_get_last_run_mod(lua_State *L); - - // set_last_run_mod(modname) - static int l_set_last_run_mod(lua_State *L); - public: static void Initialize(lua_State *L, int top); }; diff --git a/src/script/lua_api/l_util.cpp b/src/script/lua_api/l_util.cpp index 87436fce0..9152b5f7f 100644 --- a/src/script/lua_api/l_util.cpp +++ b/src/script/lua_api/l_util.cpp @@ -535,6 +535,30 @@ int ModApiUtil::l_encode_png(lua_State *L) return 1; } +// get_last_run_mod() +int ModApiUtil::l_get_last_run_mod(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + + lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME); + std::string current_mod = readParam(L, -1, ""); + if (current_mod.empty()) { + lua_pop(L, 1); + lua_pushstring(L, getScriptApiBase(L)->getOrigin().c_str()); + } + return 1; +} + +// set_last_run_mod(modname) +int ModApiUtil::l_set_last_run_mod(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + + const char *mod = luaL_checkstring(L, 1); + getScriptApiBase(L)->setOriginDirect(mod); + return 0; +} + void ModApiUtil::Initialize(lua_State *L, int top) { API_FCT(log); @@ -574,6 +598,9 @@ void ModApiUtil::Initialize(lua_State *L, int top) API_FCT(encode_png); + API_FCT(get_last_run_mod); + API_FCT(set_last_run_mod); + LuaSettings::create(L, g_settings, g_settings_path); lua_setfield(L, top, "settings"); } @@ -629,6 +656,9 @@ void ModApiUtil::InitializeAsync(lua_State *L, int top) API_FCT(colorspec_to_colorstring); API_FCT(colorspec_to_bytes); + API_FCT(get_last_run_mod); + API_FCT(set_last_run_mod); + LuaSettings::create(L, g_settings, g_settings_path); lua_setfield(L, top, "settings"); } diff --git a/src/script/lua_api/l_util.h b/src/script/lua_api/l_util.h index 54d2be619..cc91e8d39 100644 --- a/src/script/lua_api/l_util.h +++ b/src/script/lua_api/l_util.h @@ -110,6 +110,12 @@ private: // encode_png(w, h, data, level) static int l_encode_png(lua_State *L); + // get_last_run_mod() + static int l_get_last_run_mod(lua_State *L); + + // set_last_run_mod(modname) + static int l_set_last_run_mod(lua_State *L); + public: static void Initialize(lua_State *L, int top); static void InitializeAsync(lua_State *L, int top); diff --git a/src/script/scripting_mainmenu.cpp b/src/script/scripting_mainmenu.cpp index b102a66a1..2a0cadb23 100644 --- a/src/script/scripting_mainmenu.cpp +++ b/src/script/scripting_mainmenu.cpp @@ -92,9 +92,9 @@ void MainMenuScripting::step() } /******************************************************************************/ -unsigned int MainMenuScripting::queueAsync(const std::string &serialized_func, - const std::string &serialized_param) +u32 MainMenuScripting::queueAsync(std::string &&serialized_func, + std::string &&serialized_param) { - return asyncEngine.queueAsyncJob(serialized_func, serialized_param); + return asyncEngine.queueAsyncJob(std::move(serialized_func), std::move(serialized_param)); } diff --git a/src/script/scripting_mainmenu.h b/src/script/scripting_mainmenu.h index 9e23bdc1b..3c329654a 100644 --- a/src/script/scripting_mainmenu.h +++ b/src/script/scripting_mainmenu.h @@ -38,8 +38,9 @@ public: void step(); // Pass async events from engine to async threads - unsigned int queueAsync(const std::string &serialized_func, - const std::string &serialized_params); + u32 queueAsync(std::string &&serialized_func, + std::string &&serialized_param); + private: void initializeModApi(lua_State *L, int top); static void registerLuaClasses(lua_State *L, int top);