Try to preserve metatable when exchanging data with the async env (#14369)

Co-authored-by: sfan5 <sfan5@live.de>
Co-authored-by: Lars Mueller <appgurulars@gmx.de>

builtin/common/metatable.lua

@@ -0,0 +1,13 @@
+-- Registered metatables, used by the C++ packer
+local known_metatables = {}
+function core.register_async_metatable(name, mt)
+	assert(type(name) == "string", ("attempt to use %s value as metatable name"):format(type(name)))
+	assert(type(mt) == "table", ("attempt to register a %s value as metatable"):format(type(mt)))
+	assert(known_metatables[name] == nil or known_metatables[name] == mt,
+			("attempt to override metatable %s"):format(name))
+	known_metatables[name] = mt
+	known_metatables[mt] = name
+end
+core.known_metatables = known_metatables
+
+core.register_async_metatable("__builtin:vector", vector.metatable)

builtin/game/init.lua

@@ -18,6 +18,7 @@ if core.settings:get_bool("profiler.load") then
 end
 
 dofile(commonpath .. "after.lua")
+dofile(commonpath .. "metatable.lua")
 dofile(commonpath .. "mod_storage.lua")
 dofile(gamepath .. "item_entity.lua")
 dofile(gamepath .. "deprecated.lua")

builtin/init.lua

@@ -63,6 +63,7 @@ elseif INIT == "mainmenu" then
 elseif INIT == "async"  then
 	dofile(asyncpath .. "mainmenu.lua")
 elseif INIT == "async_game" then
+	dofile(commonpath .. "metatable.lua")
 	dofile(asyncpath .. "game.lua")
 elseif INIT == "client" then
 	dofile(scriptdir .. "client" .. DIR_DELIM .. "init.lua")

doc/lua_api.md

@@ -6012,7 +6012,7 @@ Environment access
 * `minetest.add_entity(pos, name, [staticdata])`: Spawn Lua-defined entity at
   position.
     * Returns `ObjectRef`, or `nil` if failed
-    * Entities with `static_save = true` can be added also 
+    * Entities with `static_save = true` can be added also
       to unloaded and non-generated blocks.
 * `minetest.add_item(pos, item)`: Spawn item
     * Returns `ObjectRef`, or `nil` if failed
@@ -6598,6 +6598,18 @@ This allows you easy interoperability for delegating work to jobs.
     * Register a path to a Lua file to be imported when an async environment
       is initialized. You can use this to preload code which you can then call
       later using `minetest.handle_async()`.
+* `minetest.register_async_metatable(name, mt)`:
+    * Register a metatable that should be preserved when data is transferred
+    between the main thread and the async environment.
+    * `name` is a string that identifies the metatable. It is recommended to
+      follow the `modname:name` convention for this identifier.
+    * `mt` is the metatable to register.
+    * Note that it is allowed to register the same metatable under multiple
+      names, but it is not allowed to register multiple metatables under the
+      same name.
+    * You must register the metatable in both the main environment
+      and the async environment for this mechanism to work.
+
 
 ### List of APIs available in an async environment
 
@@ -6623,6 +6635,7 @@ Class instances that can be transferred between environments:
 Functions:
 * Standalone helpers such as logging, filesystem, encoding,
   hashing or compression APIs
+* `minetest.register_async_metatable` (see above)
 
 Variables:
 * `minetest.settings`

games/devtest/mods/unittests/async_env.lua

@@ -53,7 +53,7 @@ local test_object = {
 	end,
 	sunlight_propagates = true,
 	is_ground_content = false,
-	light_source = 0,
+	pos = vector.new(-1, -2, -3),
 }
 
 local function test_object_passing()
@@ -166,3 +166,44 @@ local function test_userdata_passing2(cb, _, pos)
 	end, vm, pos)
 end
 unittests.register("test_userdata_passing2", test_userdata_passing2, {map=true, async=true})
+
+local function test_async_metatable_override()
+	assert(pcall(core.register_async_metatable, "__builtin:vector", vector.metatable),
+			"Metatable name aliasing throws an error when it should be allowed")
+
+	assert(not pcall(core.register_async_metatable, "__builtin:vector", {}),
+			"Illegal metatable overriding allowed")
+end
+unittests.register("test_async_metatable_override", test_async_metatable_override)
+
+local function test_async_metatable_registration(cb)
+	local custom_metatable = {}
+	core.register_async_metatable("unittests:custom_metatable", custom_metatable)
+
+	core.handle_async(function(x)
+		-- unittests.custom_metatable is registered in inside_async_env.lua
+		return getmetatable(x) == unittests.custom_metatable, x
+	end, function(metatable_preserved_async, table_after_roundtrip)
+		if not metatable_preserved_async then
+			return cb("Custom metatable not preserved (main -> async)")
+		end
+		if getmetatable(table_after_roundtrip) ~= custom_metatable then
+			return cb("Custom metable not preserved (after roundtrip)")
+		end
+		cb()
+	end, setmetatable({}, custom_metatable))
+end
+unittests.register("test_async_metatable_registration", test_async_metatable_registration, {async=true})
+
+local function test_vector_preserve(cb)
+	local vec = vector.new(1, 2, 3)
+	core.handle_async(function(x)
+		return x[1]
+	end, function(ret)
+		if ret ~= vec then -- fails if metatable was not preserved
+			return cb("Vector value mismatch")
+		end
+		cb()
+	end, {vec})
+end
+unittests.register("test_async_vector", test_vector_preserve, {async=true})

games/devtest/mods/unittests/inside_async_env.lua

@@ -2,6 +2,9 @@ unittests = {}
 
 core.log("info", "Hello World")
 
+unittests.custom_metatable = {}
+core.register_async_metatable("unittests:custom_metatable", unittests.custom_metatable)
+
 local function do_tests()
 	assert(core == minetest)
 	-- stuff that should not be here

src/script/common/c_packer.cpp

@@ -99,6 +99,27 @@ static inline bool suitable_key(lua_State *L, int idx)
 	}
 }
 
+/**
+ * Push core.known_metatables to the stack if it exists.
+ * @param L Lua state
+ * @return true if core.known_metatables exists, false otherwise.
+*/
+static inline bool get_known_lua_metatables(lua_State *L)
+{
+	lua_getglobal(L, "core");
+	if (!lua_istable(L, -1)) {
+		lua_pop(L, 1);
+		return false;
+	}
+	lua_getfield(L, -1, "known_metatables");
+	if (lua_istable(L, -1)) {
+		lua_remove(L, -2);
+		return true;
+	}
+	lua_pop(L, 2);
+	return false;
+}
+
 namespace {
 	// checks if you left any values on the stack, for debugging
 	class StackChecker {
@@ -450,6 +471,18 @@ static VectorRef<PackedInstr> pack_inner(lua_State *L, int idx, int vidx, Packed
 		lua_pop(L, 1);
 	}
 
+	// try to preserve metatable information
+	if (lua_getmetatable(L, idx) && get_known_lua_metatables(L)) {
+		lua_insert(L, -2);
+		lua_gettable(L, -2);
+		if (lua_isstring(L, -1)) {
+			auto r = emplace(pv, INSTR_SETMETATABLE);
+			r->sdata = std::string(lua_tostring(L, -1));
+			r->set_into = vi_table;
+		}
+		lua_pop(L, 2);
+	}
+
 	// exactly the table should be left on stack
 	assert(vidx == vi_table + 1);
 	return rtable;
@@ -514,6 +547,16 @@ void script_unpack(lua_State *L, PackedValue *pv)
 				lua_pushinteger(L, i.sidata1);
 				lua_rawget(L, top);
 				break;
+			case INSTR_SETMETATABLE:
+				if (get_known_lua_metatables(L)) {
+					lua_getfield(L, -1, i.sdata.c_str());
+					lua_remove(L, -2);
+					if (lua_istable(L, -1))
+						lua_setmetatable(L, top + i.set_into);
+					else
+						lua_pop(L, 1);
+				}
+				continue;
 
 			/* Lua types */
 			case LUA_TNIL:
@@ -614,6 +657,9 @@ void script_dump_packed(const PackedValue *val)
 			case INSTR_PUSHREF:
 				printf("PUSHREF(%d)", i.sidata1);
 				break;
+			case INSTR_SETMETATABLE:
+				printf("SETMETATABLE(%s)", i.sdata.c_str());
+				break;
 			case LUA_TNIL:
 				printf("nil");
 				break;
@@ -636,7 +682,7 @@ void script_dump_packed(const PackedValue *val)
 				printf("userdata %s %p", i.sdata.c_str(), i.ptrdata);
 				break;
 			default:
-				printf("!!UNKNOWN!!");
+				FATAL_ERROR("unknown type");
 				break;
 		}
 		if (i.set_into) {

src/script/common/c_packer.h

@@ -34,9 +34,10 @@ extern "C" {
 	states and cannot be used for persistence or network transfer.
 */
 
-#define INSTR_SETTABLE (-10)
-#define INSTR_POP      (-11)
-#define INSTR_PUSHREF  (-12)
+#define INSTR_SETTABLE     (-10)
+#define INSTR_POP          (-11)
+#define INSTR_PUSHREF      (-12)
+#define INSTR_SETMETATABLE (-13)
 
 /**
  * Represents a single instruction that pushes a new value or operates with existing ones.
@@ -70,6 +71,7 @@ struct PackedInstr
 		- function: buffer
 		- w/ set_into: string key (no null bytes!)
 		- userdata: name in registry
+		- INSTR_SETMETATABLE: name of the metatable
 	*/
 	std::string sdata;