From 5c04801162c72cfc4f2ff961876b842866292450 Mon Sep 17 00:00:00 2001 From: philmoz Date: Fri, 13 Jun 2025 11:08:54 +1000 Subject: [PATCH] fix(color): fix EM crashes when shutting down Lua states (#6337) --- radio/src/edgetx.cpp | 3 + radio/src/gui/colorlcd/standalone_lua.cpp | 15 +-- radio/src/lua/interface.cpp | 147 ++++++++++++---------- radio/src/lua/lua_api.h | 13 +- radio/src/lua/widgets.cpp | 2 - radio/src/mixer.cpp | 8 +- radio/src/tests/lua.cpp | 2 +- radio/src/thirdparty/Lua/src/lstate.c | 8 +- 8 files changed, 106 insertions(+), 92 deletions(-) diff --git a/radio/src/edgetx.cpp b/radio/src/edgetx.cpp index e345e2931..4f4d09b66 100644 --- a/radio/src/edgetx.cpp +++ b/radio/src/edgetx.cpp @@ -1119,6 +1119,8 @@ void edgeTxResume() TRACE("edgeTxResume"); if (!sdMounted()) sdInit(); + + luaInitMainState(); #if defined(COLORLCD) && defined(LUA) // reload widgets luaInitThemesAndWidgets(); @@ -1441,6 +1443,7 @@ void edgeTxInit() logsInit(); } + luaInitMainState(); #if defined(COLORLCD) && defined(LUA) if (!UNEXPECTED_SHUTDOWN()) { // lua widget state must be prepared before the call to storageReadAll() diff --git a/radio/src/gui/colorlcd/standalone_lua.cpp b/radio/src/gui/colorlcd/standalone_lua.cpp index 055fbf932..c9f68e776 100644 --- a/radio/src/gui/colorlcd/standalone_lua.cpp +++ b/radio/src/gui/colorlcd/standalone_lua.cpp @@ -49,8 +49,6 @@ static void luaStandaloneHook(lua_State * L, lua_Debug *ar) static void luaStandaloneInit() { - luaClose(&lsStandalone); - #if defined(USE_CUSTOM_ALLOCATOR) lsStandalone = lua_newstate(custom_l_alloc, nullptr); //we use our own allocator! #elif defined(LUA_ALLOCATOR_TRACER) @@ -66,18 +64,14 @@ static void luaStandaloneInit() lua_atpanic(lsStandalone, &custom_lua_atpanic); #if defined(LUA_ALLOCATOR_TRACER) - lua_sethook(L, luaStandaloneHook, LUA_MASKLINE); + lua_sethook(lsStandalone, luaStandaloneHook, LUA_MASKLINE); #endif // protect libs and constants registration PROTECT_LUA() { luaRegisterLibraries(lsStandalone); - } - else { - // if we got panic during registration - // we disable Lua for this session + } else { luaClose(&lsStandalone); - lsStandalone = 0; } UNPROTECT_LUA(); } @@ -85,7 +79,7 @@ static void luaStandaloneInit() void luaExecStandalone(const char * filename) { - if (lsStandalone == NULL) + if (lsStandalone == nullptr) luaStandaloneInit(); PROTECT_LUA() { @@ -212,9 +206,10 @@ void StandaloneLuaWindow::deleteLater(bool detach, bool trash) if (initFunction != LUA_REFNIL) luaL_unref(lsStandalone, LUA_REGISTRYINDEX, initFunction); if (runFunction != LUA_REFNIL) luaL_unref(lsStandalone, LUA_REGISTRYINDEX, runFunction); - lua_settop(lsStandalone, 0); luaLcdBuffer = nullptr; + luaClose(&lsStandalone); + if (lcdBuffer) delete lcdBuffer; lcdBuffer = nullptr; diff --git a/radio/src/lua/interface.cpp b/radio/src/lua/interface.cpp index 4ba95fa1f..27e87b683 100644 --- a/radio/src/lua/interface.cpp +++ b/radio/src/lua/interface.cpp @@ -63,7 +63,7 @@ extern "C" { // Since we may not run FG every time, keep the events in a buffer event_t events[EVENT_BUFFER_SIZE] = { 0 }; // The main thread - lsScripts is now a coroutine -lua_State * L = nullptr; +lua_State * mainState = nullptr; lua_State *lsScripts = nullptr; uint8_t luaState = 0; uint8_t luaScriptsCount = 0; @@ -171,12 +171,12 @@ void luaGetInputs(ScriptInputsOutputs & sid) { // To preserve the string value, truncate to 6 chars and move it to the main stack char str[7] = {0}; strncpy(str, lua_tostring(lsScripts, -1), 6); - lua_pushstring(L, &str[0]); + lua_pushstring(mainState, &str[0]); } lua_pop(lsScripts, 1); lua_pushnil(lsScripts); // Keep the stack balanced - lua_insert(L, -2); // Keep the coroutine at the top of the main stack - si->name = lua_tostring(L, -2); + lua_insert(mainState, -2); // Keep the coroutine at the top of the main stack + si->name = lua_tostring(mainState, -2); break; case 1: luaL_checktype(lsScripts, -2, LUA_TNUMBER); // key is number @@ -234,10 +234,10 @@ void luaGetOutputs(ScriptInputsOutputs & sid) // To preserve the string value, truncate to 6 chars and move it to the main stack char str[7] = {0}; strncpy(str, lua_tostring(lsScripts, -1), 6); - lua_pushstring(L, &str[0]); + lua_pushstring(mainState, &str[0]); // Keep the coroutine at the top of the main stack - lua_insert(L, -2); - sid.outputs[sid.outputsCount++].name = lua_tostring(L, -2); + lua_insert(mainState, -2); + sid.outputs[sid.outputsCount++].name = lua_tostring(mainState, -2); } lua_pop(lsScripts, 1); } @@ -278,10 +278,10 @@ void luaClose() { #if defined(COLORLCD) luaClose(&lsWidgets); - extern lua_State* lsStandalone; - luaClose(&lsStandalone); #endif - luaClose(&lsScripts); + // Close main state last + luaClose(&mainState); + lsScripts = nullptr; } void luaRegisterLibraries(lua_State * L) @@ -871,8 +871,12 @@ static int luaRegisterFunction(const char * key) } } +#if defined(COLORLCD) +static void luaLoadScripts(bool init) +#else // Load Lua scripts. If filename is given, then load a standalone script static void luaLoadScripts(bool init, const char * filename = nullptr) +#endif { // Static variables for keeping state when Lua is preempted static uint8_t ref; @@ -887,16 +891,14 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) luaEmptyEventBuffer(); // Initialize loop over references - if (filename) { - ref = SCRIPT_STANDALONE; - } - else { -#if defined(LUA_MODEL_SCRIPTS) - ref = SCRIPT_MIX_FIRST; +#if defined(COLORLCD) + ref = SCRIPT_REF_FIRST; #else - ref = SCRIPT_FUNC_FIRST; + if (filename) + ref = SCRIPT_STANDALONE; + else + ref = SCRIPT_REF_FIRST; #endif - } } // Continue loop with existing reference value @@ -928,6 +930,7 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) } } #endif +#if !defined(COLORLCD) else { // Standalone script ScriptInternalData & sid = scriptInternalData[luaScriptsCount++]; @@ -937,6 +940,7 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) break; } } +#endif // Skip the rest of the loop if we did not get a new script if (countBefore == luaScriptsCount) continue; } @@ -1000,9 +1004,11 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) // If init(), push it on the stack if (initFunction != LUA_NOREF) { lua_rawgeti(lsScripts, LUA_REGISTRYINDEX, initFunction); +#if !defined(COLORLCD) if (ref == SCRIPT_STANDALONE) { luaLcdAllowed = true; } +#endif } } } @@ -1017,12 +1023,12 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) luaError(lsScripts, sid.state); // Replace the dead coroutine with a new one - lua_pop(L, 1); // Pop the dead coroutine off the main stack - lsScripts = lua_newthread(L); // Push the new coroutine + lua_pop(mainState, 1); // Pop the dead coroutine off the main stack + lsScripts = lua_newthread(mainState); // Push the new coroutine luaDoGc(lsScripts, true); } - } while(++ref < SCRIPT_STANDALONE); + } while(++ref < SCRIPT_REF_LAST); // Loading has finished - start running scripts luaState = INTERPRETER_START_RUNNING; @@ -1031,11 +1037,13 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) void luaGetValueAndPush(lua_State *L, int src); +#if !defined(COLORLCD) void luaExec(const char * filename) { luaState = INTERPRETER_LOADING; luaLoadScripts(true, filename); } +#endif static bool resumeLua(bool init, bool allowLcdUsage) { @@ -1045,8 +1053,10 @@ static bool resumeLua(bool init, bool allowLcdUsage) bool scriptWasRun = false; bool fullGC = !allowLcdUsage; +#if !defined(COLORLCD) static uint8_t luaDisplayStatistics = false; - +#endif + // Run in the right interactive mode if (lua_status(lsScripts) == LUA_YIELD && allowLcdUsage != luaLcdAllowed) { #if defined(PCBTARANIS) @@ -1070,7 +1080,8 @@ static bool resumeLua(bool init, bool allowLcdUsage) if (sid.state != SCRIPT_OK) { displayLuaError(); scriptWasRun = true; - + +#if !defined(COLORLCD) if (ref == SCRIPT_STANDALONE) { // Pull a new event from the buffer luaNextEvent(&evt); @@ -1079,7 +1090,8 @@ static bool resumeLua(bool init, bool allowLcdUsage) killEvents(evt.event); } } - +#endif + continue; } @@ -1089,32 +1101,23 @@ static bool resumeLua(bool init, bool allowLcdUsage) if (luaStatus == LUA_OK) { // Not preempted - setup another function call lua_settop(lsScripts, 0); - + +#if !defined(COLORLCD) if (allowLcdUsage) { -#if defined(PCBTARANIS) if ((menuHandlers[menuLevel] == menuViewTelemetry && ref == SCRIPT_TELEMETRY_FIRST + selectedTelemView) || ref == SCRIPT_STANDALONE) { -#else - if (ref == SCRIPT_STANDALONE) { -#endif // Pull a new event from the buffer luaNextEvent(&evt); lua_rawgeti(lsScripts, LUA_REGISTRYINDEX, sid.run); lua_pushinteger(lsScripts, evt.event); inputsCount = 1; - -#if defined(HARDWARE_TOUCH) - if (IS_TOUCH_EVENT(evt.event)) { - luaPushTouchEventTable(lsScripts, &evt); - inputsCount = 2; - } -#endif } else continue; - } - else { + } else +#endif + { #if defined(LUA_MODEL_SCRIPTS) if (ref <= SCRIPT_MIX_LAST) { lua_rawgeti(lsScripts, LUA_REGISTRYINDEX, sid.run); @@ -1203,6 +1206,7 @@ static bool resumeLua(bool init, bool allowLcdUsage) } } else #endif +#if !defined(COLORLCD) if (ref == SCRIPT_STANDALONE) { lua_settop(lsScripts, 1); if (lua_isnumber(lsScripts, -1)) { @@ -1215,14 +1219,12 @@ static bool resumeLua(bool init, bool allowLcdUsage) luaState = INTERPRETER_RELOAD_PERMANENT_SCRIPTS; } else if (luaDisplayStatistics) { - #if !defined(COLORLCD) lcdDrawSolidHorizontalLine(0, 7*FH-1, lcdLastRightPos+6, ERASE); lcdDrawText(0, 7*FH, "GV Use: "); lcdDrawNumber(lcdLastRightPos, 7*FH, luaGetMemUsed(lsScripts), LEFT); lcdDrawChar(lcdLastRightPos, 7*FH, 'b'); lcdDrawSolidHorizontalLine(0, 7*FH-2, lcdLastRightPos+6, FORCE); lcdDrawVerticalLine(lcdLastRightPos+6, 7*FH-2, FH+2, SOLID, FORCE); - #endif } } else if (lua_isstring(lsScripts, -1)) { @@ -1253,6 +1255,9 @@ static bool resumeLua(bool init, bool allowLcdUsage) } #endif } +#else + {} +#endif } else { // Error @@ -1260,8 +1265,8 @@ static bool resumeLua(bool init, bool allowLcdUsage) luaError(lsScripts, sid.state); // Replace the dead coroutine with a new one - lua_pop(L, 1); // Pop the dead coroutine off the main stack - lsScripts = lua_newthread(L); // Push the new coroutine + lua_pop(mainState, 1); // Pop the dead coroutine off the main stack + lsScripts = lua_newthread(mainState); // Push the new coroutine luaFree(lsScripts, sid); luaDoGc(lsScripts, true); } @@ -1329,12 +1334,8 @@ void checkLuaMemoryUsage() if (totalMemUsed > LUA_MEM_MAX) { TRACE_ERROR("checkLuaMemoryUsage(): max limit reached (%u), killing Lua\n", totalMemUsed); // disable Lua scripts - luaClose(&lsScripts); + luaClose(); luaDisable(); -#if defined(COLORLCD) - // disable widgets - luaClose(&lsWidgets); -#endif } #endif } @@ -1344,35 +1345,43 @@ uint32_t luaGetMemUsed(lua_State * L) return L ? (lua_gc(L, LUA_GCCOUNT, 0) << 10) + lua_gc(L, LUA_GCCOUNTB, 0) : 0; } +void luaInitMainState() +{ + if (mainState != nullptr) return; + +#if defined(USE_CUSTOM_ALLOCATOR) + mainState = lua_newstate(custom_l_alloc, nullptr); //we use our own allocator! +#elif defined(LUA_ALLOCATOR_TRACER) + memclear(&lsScriptsTrace, sizeof(lsScriptsTrace)); + lsScriptsTrace.script = "lua_newstate(scripts)"; + mainState = lua_newstate(tracer_alloc, &lsScriptsTrace); //we use tracer allocator +#else + mainState = luaL_newstate(); //we use Lua default allocator +#endif + + if (mainState) { + // install our panic handler + lua_atpanic(mainState, &custom_lua_atpanic); + +#if defined(LUA_ALLOCATOR_TRACER) + lua_sethook(mainState, luaHook, LUA_MASKCOUNT|LUA_MASKLINE, PERMANENT_SCRIPTS_MAX_INSTRUCTIONS); +#else + lua_sethook(mainState, luaHook, LUA_MASKCOUNT, PERMANENT_SCRIPTS_MAX_INSTRUCTIONS); +#endif + } +} + void luaInit() { TRACE("luaInit"); - luaClose(&lsScripts); - L = nullptr; - if (luaState != INTERPRETER_PANIC) { -#if defined(USE_CUSTOM_ALLOCATOR) - L = lua_newstate(custom_l_alloc, nullptr); //we use our own allocator! -#elif defined(LUA_ALLOCATOR_TRACER) - memclear(&lsScriptsTrace, sizeof(lsScriptsTrace)); - lsScriptsTrace.script = "lua_newstate(scripts)"; - L = lua_newstate(tracer_alloc, &lsScriptsTrace); //we use tracer allocator -#else - L = luaL_newstate(); //we use Lua default allocator -#endif - if (L) { - // install our panic handler - lua_atpanic(L, &custom_lua_atpanic); - -#if defined(LUA_ALLOCATOR_TRACER) - lua_sethook(L, luaHook, LUA_MASKCOUNT|LUA_MASKLINE, PERMANENT_SCRIPTS_MAX_INSTRUCTIONS); -#else - lua_sethook(L, luaHook, LUA_MASKCOUNT, PERMANENT_SCRIPTS_MAX_INSTRUCTIONS); -#endif + if (mainState) { + lua_settop(mainState, 0); + lua_gc(mainState, LUA_GCCOLLECT, 0); // lsScripts is now a coroutine in lieu of the main thread to support preemption - lsScripts = lua_newthread(L); + lsScripts = lua_newthread(mainState); // Clear loaded scripts memclear(scriptInternalData, sizeof(scriptInternalData)); diff --git a/radio/src/lua/lua_api.h b/radio/src/lua/lua_api.h index f3c922c0f..e4134dfbd 100644 --- a/radio/src/lua/lua_api.h +++ b/radio/src/lua/lua_api.h @@ -65,6 +65,7 @@ extern uint32_t luaExtraMemoryUsage; void luaInitThemesAndWidgets(); #endif +void luaInitMainState(); void luaInit(); void luaClose(); @@ -121,11 +122,14 @@ enum ScriptState { }; enum ScriptReference { + SCRIPT_REF_FIRST, #if defined(LUA_MODEL_SCRIPTS) - SCRIPT_MIX_FIRST, + SCRIPT_MIX_FIRST = SCRIPT_REF_FIRST, SCRIPT_MIX_LAST=SCRIPT_MIX_FIRST+MAX_SCRIPTS-1, -#endif SCRIPT_FUNC_FIRST, +#else + SCRIPT_FUNC_FIRST = SCRIPT_REF_FIRST, +#endif SCRIPT_FUNC_LAST=SCRIPT_FUNC_FIRST+MAX_SPECIAL_FUNCTIONS-1, // model functions SCRIPT_GFUNC_FIRST, SCRIPT_GFUNC_LAST=SCRIPT_GFUNC_FIRST+MAX_SPECIAL_FUNCTIONS-1, // global functions @@ -133,7 +137,10 @@ enum ScriptReference { SCRIPT_TELEMETRY_FIRST, SCRIPT_TELEMETRY_LAST=SCRIPT_TELEMETRY_FIRST+MAX_SCRIPTS, // telem0 and telem1 .. telem7 #endif - SCRIPT_STANDALONE // Standalone script + SCRIPT_REF_LAST, +#if !defined(COLORLCD) + SCRIPT_STANDALONE = SCRIPT_REF_LAST // Standalone script +#endif }; struct ScriptInternalData { diff --git a/radio/src/lua/widgets.cpp b/radio/src/lua/widgets.cpp index 2dd509af8..6ecac3a88 100644 --- a/radio/src/lua/widgets.cpp +++ b/radio/src/lua/widgets.cpp @@ -248,9 +248,7 @@ void luaInitThemesAndWidgets() else { // if we got panic during registration // we disable Lua for this session - // luaDisable(); luaClose(&lsWidgets); - lsWidgets = 0; } UNPROTECT_LUA(); TRACE("lsWidgets %p", lsWidgets); diff --git a/radio/src/mixer.cpp b/radio/src/mixer.cpp index 1a48d7a07..de51312ba 100644 --- a/radio/src/mixer.cpp +++ b/radio/src/mixer.cpp @@ -870,9 +870,11 @@ void evalFlightModeMixes(uint8_t mode, uint8_t tick10ms) // disable mixer if Lua script is used as source and script was killed if (srcRawAbs >= MIXSRC_FIRST_LUA && srcRawAbs <= MIXSRC_LAST_LUA) { div_t qr = div(int(srcRawAbs - MIXSRC_FIRST_LUA), MAX_SCRIPT_OUTPUTS); - if (scriptInternalData[qr.quot].state != SCRIPT_OK) { - mixCondition = true; - mixEnabled = 0; + for (int n = 0; n < MAX_SCRIPTS; n += 1) { + if ((scriptInternalData[n].reference == qr.quot) && (scriptInternalData[n].state != SCRIPT_OK)) { + mixCondition = true; + mixEnabled = 0; + } } } #endif diff --git a/radio/src/tests/lua.cpp b/radio/src/tests/lua.cpp index 2d75180c1..2b7accb09 100644 --- a/radio/src/tests/lua.cpp +++ b/radio/src/tests/lua.cpp @@ -34,7 +34,7 @@ ::testing::AssertionResult __luaExecStr(const char * str) { extern lua_State * lsScripts; - if (!lsScripts) luaInit(); + if (!lsScripts) { luaInitMainState(); luaInit(); } if (!lsScripts) return ::testing::AssertionFailure() << "No Lua state!"; if (luaL_dostring(lsScripts, str)) { return ::testing::AssertionFailure() << "lua error: " << lua_tostring(lsScripts, -1); diff --git a/radio/src/thirdparty/Lua/src/lstate.c b/radio/src/thirdparty/Lua/src/lstate.c index 2ea4d62a6..6b3a9da60 100644 --- a/radio/src/thirdparty/Lua/src/lstate.c +++ b/radio/src/thirdparty/Lua/src/lstate.c @@ -340,18 +340,18 @@ LUA_API lua_State *lua_newstate (lua_Alloc f, void *ud) { g->ROstrt.size = 0; g->ROstrt.nuse = 0; g->ROstrt.hash = NULL; -#ifdef LUA_ENABLE_TEST +// #ifdef LUA_ENABLE_TEST // EdgeTX uses multiple states - ensure only 1st one is set to L0 if (L0) { /* This is a second state */ g->cache=G(L0)->cache; } else { -#endif +// #endif L0 = L; g->cache = cast(KeyCacheLine *, (*f)(ud, NULL, 0, KEYCACHE_N * sizeof(KeyCacheLine))); memset(g->cache, 0, KEYCACHE_N * sizeof(KeyCacheLine)); -#ifdef LUA_ENABLE_TEST +// #ifdef LUA_ENABLE_TEST } -#endif +// #endif for (i=0; i < LUA_NUMTAGS; i++) g->mt[i] = NULL; if (luaD_rawrunprotected(L, f_luaopen, NULL) != LUA_OK) { /* memory allocation error: free partial state */