Merge branch 'lua_callback_crash' into 'master'

Lua Fix crash when passing a non-callback table to a callback argument

Closes #7827

See merge request OpenMW/openmw!3861

apps/openmw_test_suite/lua/test_storage.cpp

@@ -15,7 +15,7 @@ namespace
         return lua.safe_script("return " + luaCode).get<T>();
     }
 
-    TEST(LuaUtilStorageTest, Basic)
+    TEST(LuaUtilStorageTest, Subscribe)
     {
         // Note: LuaUtil::Callback can be used only if Lua is initialized via LuaUtil::LuaState
         LuaUtil::LuaState luaState{ nullptr, nullptr };
@@ -24,21 +24,21 @@ namespace
         LuaUtil::LuaStorage storage(mLua);
         storage.setActive(true);
 
-        std::vector<std::string> callbackCalls;
         sol::table callbackHiddenData(mLua, sol::create);
         callbackHiddenData[LuaUtil::ScriptsContainer::sScriptIdKey] = LuaUtil::ScriptId{};
-        sol::table callback(mLua, sol::create);
-        callback[1] = [&](const std::string& section, const sol::optional<std::string>& key) {
-            if (key)
-                callbackCalls.push_back(section + "_" + *key);
-            else
-                callbackCalls.push_back(section + "_*");
-        };
-        callback[2] = LuaUtil::AsyncPackageId{ nullptr, 0, callbackHiddenData };
+        LuaUtil::getAsyncPackageInitializer(
+            mLua.lua_state(), []() { return 0.0; }, []() { return 0.0; })(callbackHiddenData);
+        mLua["async"] = LuaUtil::AsyncPackageId{ nullptr, 0, callbackHiddenData };
 
         mLua["mutable"] = storage.getMutableSection("test");
         mLua["ro"] = storage.getReadOnlySection("test");
-        mLua["ro"]["subscribe"](mLua["ro"], callback);
+
+        mLua.safe_script(R"(
+            callbackCalls = {}
+            ro:subscribe(async:callback(function(section, key)
+                table.insert(callbackCalls, section .. '_' .. (key or '*'))
+            end))
+        )");
 
         mLua.safe_script("mutable:set('x', 5)");
         EXPECT_EQ(get<int>(mLua, "mutable:get('x')"), 5);
@@ -58,7 +58,7 @@ namespace
         EXPECT_EQ(get<int>(mLua, "ro:get('x')"), 4);
         EXPECT_EQ(get<int>(mLua, "ro:get('y')"), 7);
 
-        EXPECT_THAT(callbackCalls, ::testing::ElementsAre("test_x", "test_*", "test_*"));
+        EXPECT_THAT(get<std::string>(mLua, "table.concat(callbackCalls, ', ')"), "test_x, test_*, test_*");
     }
 
     TEST(LuaUtilStorageTest, Table)

components/lua/asyncpackage.cpp

@@ -24,7 +24,30 @@ namespace LuaUtil
 
     Callback Callback::fromLua(const sol::table& t)
     {
-        return Callback{ t.raw_get<sol::main_protected_function>(1), t.raw_get<AsyncPackageId>(2).mHiddenData };
+        const sol::object& function = t.raw_get<sol::object>(1);
+        const sol::object& asyncPackageId = t.raw_get<sol::object>(2);
+        if (!function.is<sol::main_protected_function>() || !asyncPackageId.is<AsyncPackageId>())
+            throw std::domain_error("Expected an async:callback, received a table");
+        return Callback{ function.as<sol::main_protected_function>(), asyncPackageId.as<AsyncPackageId>().mHiddenData };
+    }
+
+    sol::table Callback::makeMetatable(lua_State* L)
+    {
+        sol::table callbackMeta = sol::table::create(L);
+        callbackMeta[sol::meta_function::call] = [](const sol::table& callback, sol::variadic_args va) {
+            return Callback::fromLua(callback).call(sol::as_args(va));
+        };
+        callbackMeta[sol::meta_function::to_string] = [] { return "Callback"; };
+        callbackMeta[sol::meta_function::metatable] = false;
+        callbackMeta["isCallback"] = true;
+        return callbackMeta;
+    }
+    sol::table Callback::make(const AsyncPackageId& asyncId, sol::main_protected_function fn, sol::table metatable)
+    {
+        sol::table c = sol::table::create(fn.lua_state(), 2);
+        c.raw_set(1, std::move(fn), 2, asyncId);
+        c[sol::metatable_key] = metatable;
+        return c;
     }
 
     bool Callback::isLuaCallback(const sol::object& t)
@@ -69,18 +92,9 @@ namespace LuaUtil
                       TimerType::GAME_TIME, gameTimeFn() + delay, asyncId.mScriptId, std::move(callback));
               };
 
-        sol::table callbackMeta = sol::table::create(L);
-        callbackMeta[sol::meta_function::call] = [](const sol::table& callback, sol::variadic_args va) {
-            return Callback::fromLua(callback).call(sol::as_args(va));
-        };
-        callbackMeta[sol::meta_function::to_string] = [] { return "Callback"; };
-        callbackMeta[sol::meta_function::metatable] = false;
-        callbackMeta["isCallback"] = true;
+        sol::table callbackMeta = Callback::makeMetatable(L);
         api["callback"] = [callbackMeta](const AsyncPackageId& asyncId, sol::main_protected_function fn) -> sol::table {
-            sol::table c = sol::table::create(fn.lua_state(), 2);
-            c.raw_set(1, std::move(fn), 2, asyncId);
-            c[sol::metatable_key] = callbackMeta;
-            return c;
+            return Callback::make(asyncId, fn, callbackMeta);
         };
 
         auto initializer = [](sol::table hiddenData) {

components/lua/asyncpackage.hpp

@@ -24,6 +24,8 @@ namespace LuaUtil
 
         static bool isLuaCallback(const sol::object&);
         static Callback fromLua(const sol::table&);
+        static sol::table makeMetatable(lua_State* L);
+        static sol::table make(const AsyncPackageId& asyncId, sol::main_protected_function fn, sol::table metatable);
 
         bool isValid() const { return mHiddenData[ScriptsContainer::sScriptIdKey] != sol::nil; }
 

components/lua/luastate.cpp

@@ -377,7 +377,7 @@ namespace LuaUtil
     sol::protected_function_result LuaState::throwIfError(sol::protected_function_result&& res)
     {
         if (!res.valid() && static_cast<int>(res.get_type()) == LUA_TSTRING)
-            throw std::runtime_error("Lua error: " + res.get<std::string>());
+            throw std::runtime_error(std::string("Lua error: ") += res.get<sol::error>().what());
         else
             return std::move(res);
     }

files/data/openmw_aux/ui.lua

@@ -24,7 +24,7 @@ function aux_ui.deepLayoutCopy(layout)
     for k, v in pairs(layout) do
         if k == 'content' then
             result[k] = deepContentCopy(v)
-        elseif type(v) == 'table' then
+        elseif type(v) == 'table' and getmetatable(v) == nil then
             result[k] = aux_ui.deepLayoutCopy(v)
         else
             result[k] = v