Skip to content

Commit d6fc2cd

Browse files
committed
fix: destroy C++ locals before raising a Lua error
`luaL_error` and `lua_error` report a failure by longjmp-ing to the enclosing `lua_pcall`. The jump runs no C++ destructors on the frames it unwinds, so any local with a non-trivial destructor still alive at the point of the raise is leaked, and the unwind itself is undefined. Two of the Lua binding functions raised while such a local was live. The one invoked when a script assigns to a `dom::Object` field held the value being assigned and raised from inside a `catch`. The one invoked when a script calls a `dom::Function` held the argument `dom::Array`, the call result, and the function being invoked. Stage the outcome on the Lua stack first - push the result value, or build a location-prefixed message with `luaL_where` and `lua_concat` - then close the scope so every C++ local is destroyed, and only then raise. Lua copies a pushed string, so the message outlives the destructors.
1 parent 2094838 commit d6fc2cd

1 file changed

Lines changed: 43 additions & 22 deletions

File tree

src/lib/Support/Lua.cpp

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -598,18 +598,27 @@ domObject_push_metatable(
598598
[](lua_State* L)
599599
{
600600
Access A(L);
601-
auto& obj = domObject_get(A, 1);
602-
auto key = luaM_getstring(A, 2);
603-
dom::Value value = luaValueToDom(L, 3);
604-
try
605-
{
606-
obj.set(key, std::move(value));
607-
}
608-
catch (std::exception const& ex)
601+
// `lua_error` longjmps to the enclosing `pcall` and skips any
602+
// pending C++ destructor on this frame, so scope the locals and
603+
// stage the error message before raising.
604+
bool raised = false;
609605
{
610-
return luaL_error(L, "%s", ex.what());
606+
auto& obj = domObject_get(A, 1);
607+
auto key = luaM_getstring(A, 2);
608+
dom::Value value = luaValueToDom(L, 3);
609+
try
610+
{
611+
obj.set(key, std::move(value));
612+
}
613+
catch (std::exception const& ex)
614+
{
615+
luaL_where(A, 1);
616+
lua_pushstring(A, ex.what());
617+
lua_concat(A, 2);
618+
raised = true;
619+
}
611620
}
612-
return 0;
621+
return raised ? lua_error(A) : 0;
613622
});
614623
lua_settable(A, -3);
615624

@@ -740,20 +749,32 @@ domFunction_push_metatable(
740749
{
741750
Access A(L);
742751
int const top = lua_gettop(A);
743-
dom::Array args;
744-
for (int i = 2; i <= top; ++i)
752+
// `lua_error` longjmps to the enclosing `pcall` and skips any
753+
// pending C++ destructor on this frame, so stage the outcome -
754+
// the result value, or a location-prefixed error message, on the
755+
// Lua stack - and let every local here be destroyed before
756+
// raising.
757+
bool raised = false;
745758
{
746-
args.push_back(luaValueToDom(L, i));
747-
}
748-
dom::Function fn = domFunction_get(A, 1);
749-
Expected<dom::Value> result = fn.call(args);
750-
if (! result)
751-
{
752-
return luaL_error(L, "%s",
753-
result.error().reason().c_str());
759+
dom::Array args;
760+
for (int i = 2; i <= top; ++i)
761+
{
762+
args.push_back(luaValueToDom(L, i));
763+
}
764+
Expected<dom::Value> result = domFunction_get(A, 1).call(args);
765+
if (result)
766+
{
767+
domValue_push(A, *result);
768+
}
769+
else
770+
{
771+
luaL_where(A, 1);
772+
lua_pushstring(A, result.error().reason().c_str());
773+
lua_concat(A, 2);
774+
raised = true;
775+
}
754776
}
755-
domValue_push(A, *result);
756-
return 1;
777+
return raised ? lua_error(A) : 1;
757778
});
758779
lua_settable(A, -3);
759780

0 commit comments

Comments
 (0)