Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Mar 6, 2024
13 changes: 13 additions & 0 deletions builtin/common/metatable.lua
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions builtin/game/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions builtin/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
15 changes: 14 additions & 1 deletion doc/lua_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
y5nw marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -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`
Expand Down
43 changes: 42 additions & 1 deletion games/devtest/mods/unittests/async_env.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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})
3 changes: 3 additions & 0 deletions games/devtest/mods/unittests/inside_async_env.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 47 additions & 1 deletion src/script/common/c_packer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
y5nw marked this conversation as resolved.
Show resolved Hide resolved
r->set_into = vi_table;
}
lua_pop(L, 2);
}

// exactly the table should be left on stack
assert(vidx == vi_table + 1);
return rtable;
Expand Down Expand Up @@ -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:
y5nw marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions src/script/common/c_packer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down