Prevent Lua callbacks from being called from secondary SaveRenderers

SaveRenderers populate their own Simulation with Simulation::Load, which may call various callbacks now that these SaveRenderers know about custom elements. Make sure these callbacks don't try to call into the main thread's Lua state.

Seeing as these callbacks now need to be protected from races too, the scopes of some of the exclusive locks of the graphics property mutex needed to be extended.
This commit is contained in:
Tamás Bálint Misius 2023-12-09 14:36:07 +01:00
parent ca6c67c16c
commit 2d7b40b9e5
No known key found for this signature in database
GPG Key ID: 5B472A12F6ECA9F2

View File

@ -3287,6 +3287,8 @@ void LuaScriptInterface::LuaSetParticleProperty(lua_State* l, int particleID, St
int LuaScriptInterface::elements_loadDefault(lua_State * l) int LuaScriptInterface::elements_loadDefault(lua_State * l)
{ {
auto &sd = SimulationData::Ref(); auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements;
auto &builtinElements = GetElements(); auto &builtinElements = GetElements();
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
{ {
@ -3304,14 +3306,10 @@ int LuaScriptInterface::elements_loadDefault(lua_State * l)
lua_pushnil(l); lua_pushnil(l);
lua_settable(l, -3); lua_settable(l, -3);
{
std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements;
if (id < (int)builtinElements.size()) if (id < (int)builtinElements.size())
elements[id] = builtinElements[id]; elements[id] = builtinElements[id];
else else
elements[id] = Element(); elements[id] = Element();
}
tpt_lua_pushByteString(l, identifier); tpt_lua_pushByteString(l, identifier);
lua_pushinteger(l, id); lua_pushinteger(l, id);
@ -3320,9 +3318,6 @@ int LuaScriptInterface::elements_loadDefault(lua_State * l)
} }
else else
{ {
{
std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements;
for (int i = 0; i < PT_NUM; i++) for (int i = 0; i < PT_NUM; i++)
{ {
if (i < (int)builtinElements.size()) if (i < (int)builtinElements.size())
@ -3330,7 +3325,7 @@ int LuaScriptInterface::elements_loadDefault(lua_State * l)
else else
elements[i] = Element(); elements[i] = Element();
} }
}
lua_pushnil(l); lua_pushnil(l);
lua_setglobal(l, "elements"); lua_setglobal(l, "elements");
lua_pushnil(l); lua_pushnil(l);
@ -3446,6 +3441,10 @@ int LuaScriptInterface::elements_allocate(lua_State * l)
static int luaUpdateWrapper(UPDATE_FUNC_ARGS) static int luaUpdateWrapper(UPDATE_FUNC_ARGS)
{ {
if (!sim->useLuaCallbacks)
{
return 0;
}
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
auto &builtinElements = GetElements(); auto &builtinElements = GetElements();
auto *builtinUpdate = builtinElements[parts[i].type].Update; auto *builtinUpdate = builtinElements[parts[i].type].Update;
@ -3552,6 +3551,10 @@ static int luaGraphicsWrapper(GRAPHICS_FUNC_ARGS)
static void luaCreateWrapper(ELEMENT_CREATE_FUNC_ARGS) static void luaCreateWrapper(ELEMENT_CREATE_FUNC_ARGS)
{ {
if (!sim->useLuaCallbacks)
{
return;
}
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
if (luaCreateHandlers[sim->parts[i].type]) if (luaCreateHandlers[sim->parts[i].type])
{ {
@ -3571,6 +3574,13 @@ static void luaCreateWrapper(ELEMENT_CREATE_FUNC_ARGS)
static bool luaCreateAllowedWrapper(ELEMENT_CREATE_ALLOWED_FUNC_ARGS) static bool luaCreateAllowedWrapper(ELEMENT_CREATE_ALLOWED_FUNC_ARGS)
{ {
if (!sim->useLuaCallbacks)
{
// Nothing really bad can happen, no callbacks are allowed anyway. The worst thing that can happen
// is that a well-crafted save looks odd in previews because it has multiple Element::defaultGraphics-rendered
// instances of something that should be limited to one instance.
return 1;
}
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
bool ret = false; bool ret = false;
if (luaCreateAllowedHandlers[t]) if (luaCreateAllowedHandlers[t])
@ -3597,6 +3607,10 @@ static bool luaCreateAllowedWrapper(ELEMENT_CREATE_ALLOWED_FUNC_ARGS)
static void luaChangeTypeWrapper(ELEMENT_CHANGETYPE_FUNC_ARGS) static void luaChangeTypeWrapper(ELEMENT_CHANGETYPE_FUNC_ARGS)
{ {
if (!sim->useLuaCallbacks)
{
return;
}
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
if (luaChangeTypeHandlers[sim->parts[i].type]) if (luaChangeTypeHandlers[sim->parts[i].type])
{ {
@ -3616,6 +3630,10 @@ static void luaChangeTypeWrapper(ELEMENT_CHANGETYPE_FUNC_ARGS)
static bool luaCtypeDrawWrapper(CTYPEDRAW_FUNC_ARGS) static bool luaCtypeDrawWrapper(CTYPEDRAW_FUNC_ARGS)
{ {
if (!sim->useLuaCallbacks)
{
return false;
}
auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface); auto *luacon_ci = static_cast<LuaScriptInterface *>(commandInterface);
bool ret = false; bool ret = false;
if (luaCtypeDrawHandlers[sim->parts[i].type]) if (luaCtypeDrawHandlers[sim->parts[i].type])
@ -3651,8 +3669,8 @@ int LuaScriptInterface::elements_element(lua_State * l)
if (lua_gettop(l) > 1) if (lua_gettop(l) > 1)
{ {
auto &sd = SimulationData::Ref();
{ {
auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx); std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements; auto &elements = sd.elements;
luaL_checktype(l, 2, LUA_TTABLE); luaL_checktype(l, 2, LUA_TTABLE);
@ -3748,15 +3766,15 @@ int LuaScriptInterface::elements_element(lua_State * l)
elements[id].CtypeDraw = builtinElements[id].CtypeDraw; elements[id].CtypeDraw = builtinElements[id].CtypeDraw;
} }
lua_pop(l, 1); lua_pop(l, 1);
}
lua_getfield(l, -1, "DefaultProperties"); lua_getfield(l, -1, "DefaultProperties");
SetDefaultProperties(l, id, lua_gettop(l)); SetDefaultProperties(l, id, lua_gettop(l));
lua_pop(l, 1); lua_pop(l, 1);
sd.graphicscache[id].isready = 0;
}
luacon_model->BuildMenus(); luacon_model->BuildMenus();
luacon_ci->custom_init_can_move(); luacon_ci->custom_init_can_move();
sd.graphicscache[id].isready = 0;
return 0; return 0;
} }
@ -3855,9 +3873,11 @@ int LuaScriptInterface::elements_property(lua_State * l)
if (lua_gettop(l) > 2) if (lua_gettop(l) > 2)
{ {
auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements;
if (prop != properties.end()) if (prop != properties.end())
{ {
auto &sd = SimulationData::Ref();
if (lua_type(l, 3) != LUA_TNIL) if (lua_type(l, 3) != LUA_TNIL)
{ {
if (prop->Type == StructProperty::TransitionType) if (prop->Type == StructProperty::TransitionType)
@ -3868,22 +3888,15 @@ int LuaScriptInterface::elements_property(lua_State * l)
return luaL_error(l, "Invalid element"); return luaL_error(l, "Invalid element");
} }
} }
{
std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements;
intptr_t propertyAddress = (intptr_t)(((unsigned char*)&elements[id]) + prop->Offset); intptr_t propertyAddress = (intptr_t)(((unsigned char*)&elements[id]) + prop->Offset);
LuaSetProperty(l, *prop, propertyAddress, 3); LuaSetProperty(l, *prop, propertyAddress, 3);
}
}
luacon_model->BuildMenus(); luacon_model->BuildMenus();
luacon_ci->custom_init_can_move(); luacon_ci->custom_init_can_move();
sd.graphicscache[id].isready = 0; sd.graphicscache[id].isready = 0;
} }
}
else if (propertyName == "Update") else if (propertyName == "Update")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
switch (luaL_optint(l, 4, 0)) switch (luaL_optint(l, 4, 0))
@ -3912,8 +3925,6 @@ int LuaScriptInterface::elements_property(lua_State * l)
} }
else if (propertyName == "Graphics") else if (propertyName == "Graphics")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
lua_gr_func[id].Assign(l, 3); lua_gr_func[id].Assign(l, 3);
@ -3928,8 +3939,6 @@ int LuaScriptInterface::elements_property(lua_State * l)
} }
else if (propertyName == "Create") else if (propertyName == "Create")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
luaCreateHandlers[id].Assign(l, 3); luaCreateHandlers[id].Assign(l, 3);
@ -3943,8 +3952,6 @@ int LuaScriptInterface::elements_property(lua_State * l)
} }
else if (propertyName == "CreateAllowed") else if (propertyName == "CreateAllowed")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
luaCreateAllowedHandlers[id].Assign(l, 3); luaCreateAllowedHandlers[id].Assign(l, 3);
@ -3958,8 +3965,6 @@ int LuaScriptInterface::elements_property(lua_State * l)
} }
else if (propertyName == "ChangeType") else if (propertyName == "ChangeType")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
luaChangeTypeHandlers[id].Assign(l, 3); luaChangeTypeHandlers[id].Assign(l, 3);
@ -3973,8 +3978,6 @@ int LuaScriptInterface::elements_property(lua_State * l)
} }
else if (propertyName == "CtypeDraw") else if (propertyName == "CtypeDraw")
{ {
auto &sd = SimulationData::Ref();
auto &elements = sd.elements;
if (lua_type(l, 3) == LUA_TFUNCTION) if (lua_type(l, 3) == LUA_TFUNCTION)
{ {
luaCtypeDrawHandlers[id].Assign(l, 3); luaCtypeDrawHandlers[id].Assign(l, 3);