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

Leak a little less memory with dynamic allocations in Lua #5508

Closed
wants to merge 13 commits into from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
- Bugfix: Fixed tab visibility being controllable in the emote popup. (#5530)
- Bugfix: Fixed account switch not being saved if no other settings were changed. (#5558)
- Bugfix: Fixed some tooltips not being readable. (#5578)
- Bugfix: Fixed Lua plugins leaking memory in error branches. (#5508)
- Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420)
- Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422)
- Dev: Unsingletonize `ISoundController`. (#5462)
Expand Down
53 changes: 41 additions & 12 deletions src/controllers/plugins/LuaAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include "controllers/plugins/PluginController.hpp"
# include "messages/MessageBuilder.hpp"
# include "providers/twitch/TwitchIrcServer.hpp"
# include "util/drop.hpp"

extern "C" {
# include <lauxlib.h>
Expand Down Expand Up @@ -73,15 +74,21 @@ int c2_register_command(lua_State *L)
}

QString name;
if (!lua::peek(L, &name, 1))
auto pres = lua::peek(L, &name, 1);
if (!pres)
{
luaL_error(L, "cannot get command name (1st arg of register_command, "
"expected a string)");
pres.errorReason.push_back(
QString("While getting command name (1st arg of register_command, "
"expected a string got %1)")
.arg(luaL_typename(L, 1)));
drop(name);
pres.throwAsLuaError(L);
return 0;
}
if (lua_isnoneornil(L, 2))
{
luaL_error(L, "missing argument for register_command: function "
drop(name);
luaL_error(L, "Missing argument for register_command: function "
"\"pointer\"");
return 0;
}
Expand All @@ -106,10 +113,15 @@ int c2_register_callback(lua_State *L)
return 0;
}
EventType evtType{};
if (!lua::peek(L, &evtType, 1))
auto pres = lua::peek(L, &evtType, 1);
if (!pres)
{
luaL_error(L, "cannot get event name (1st arg of register_callback, "
"expected a string)");
pres.errorReason.push_back(
QString("cannot get event name (1st arg of register_callback, "
"expected a string, got %1)")
.arg(lua_typename(L, 1)));
// no types to drop yet
pres.throwAsLuaError(L);
return 0;
}
if (lua_isnoneornil(L, 2))
Expand Down Expand Up @@ -139,14 +151,25 @@ int c2_log(lua_State *L)
luaL_error(L, "c2_log: internal error: no plugin?");
return 0;
}
if (lua_gettop(L) < 2)
{
// no arguments
return luaL_error(L,
"c2.log expects at least two arguments, a log level "
"(c2.LogLevel) and an object to print "
"(usually a string)");
}
auto logc = lua_gettop(L) - 1;
// This is almost the expansion of qCDebug() macro, actual thing is wrapped in a for loop
LogLevel lvl{};
if (!lua::pop(L, &lvl, 1))
auto pres = lua::pop(L, &lvl, 1);
if (!pres)
{
luaL_error(L, "Invalid log level, use one from c2.LogLevel.");
pres.errorReason.emplace_back(
"While getting log level (1st argument of c2.log)");
pres.throwAsLuaError(L);
return 0;
}
// This is almost the expansion of qCDebug() macro, actual thing is wrapped in a for loop
QDebug stream = qdebugStreamForLogLevel(lvl);
logHelper(L, pl, stream, logc);
return 0;
Expand Down Expand Up @@ -224,6 +247,8 @@ int g_load(lua_State *L)
utf8->toUnicode(data.constData(), data.size(), &state);
if (state.invalidChars != 0)
{
drop(data);
drop(state);
luaL_error(L, "invalid utf-8 in load() is not allowed");
return 0;
}
Expand Down Expand Up @@ -297,6 +322,10 @@ int loadfile(lua_State *L, const QString &str)
return 2;
}

drop(temp);
drop(info);
drop(datadir);
drop(dir);
return luaL_error(L, "error loading module '%s' from file '%s':\n\t%s",
lua_tostring(L, 1), filename, lua_tostring(L, -1));
}
Expand All @@ -306,10 +335,10 @@ int searcherAbsolute(lua_State *L)
auto name = QString::fromUtf8(luaL_checkstring(L, 1));
name = name.replace('.', QDir::separator());

QString filename;
auto *pl = getApp()->getPlugins()->getPluginByStatePtr(L);
if (pl == nullptr)
{
drop(name);
return luaL_error(L, "searcherAbsolute: internal error: no plugin?");
}

Expand All @@ -332,7 +361,7 @@ int searcherRelative(lua_State *L)
lua::push(
L,
QString(
"Unable to load relative to file:caller has no source file"));
"Unable to load relative to file: caller has no source file"));
return 1;
}

Expand Down
101 changes: 76 additions & 25 deletions src/controllers/plugins/LuaUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,108 +156,127 @@ StackIdx push(lua_State *L, const api::CompletionEvent &ev)
return idx;
}

bool peek(lua_State *L, int *out, StackIdx idx)
PeekResult peek(lua_State *L, int *out, StackIdx idx)
{
StackGuard guard(L);
if (lua_isnumber(L, idx) == 0)
{
return false;
return {
false,
{QString("Expected an integer got %1").arg(luaL_typename(L, idx))}};
}

*out = lua_tointeger(L, idx);
return true;
return {};
}

bool peek(lua_State *L, bool *out, StackIdx idx)
PeekResult peek(lua_State *L, bool *out, StackIdx idx)
{
StackGuard guard(L);
if (!lua_isboolean(L, idx))
{
return false;
return {
false,
{QString("Expected a boolean got %1").arg(luaL_typename(L, idx))}};
}

*out = bool(lua_toboolean(L, idx));
return true;
return {};
}

bool peek(lua_State *L, double *out, StackIdx idx)
PeekResult peek(lua_State *L, double *out, StackIdx idx)
{
StackGuard guard(L);
int ok{0};
auto v = lua_tonumberx(L, idx, &ok);
if (ok != 0)
{
*out = v;
return {};
}
return ok != 0;

return PeekResult::ofTypeError(L, idx, "integer or float");
}

bool peek(lua_State *L, QString *out, StackIdx idx)
PeekResult peek(lua_State *L, QString *out, StackIdx idx)
{
StackGuard guard(L);
size_t len{0};
const char *str = lua_tolstring(L, idx, &len);
if (str == nullptr)
{
return false;
return PeekResult::ofTypeError(L, idx, "string");
}
if (len >= INT_MAX)
{
assert(false && "string longer than INT_MAX, shit's fucked, yo");
return {false,
{QString("Strings bigger than 2.1 gigabytes are not allowed")}};
}
*out = QString::fromUtf8(str, int(len));
return true;
return {};
}

bool peek(lua_State *L, QByteArray *out, StackIdx idx)
PeekResult peek(lua_State *L, QByteArray *out, StackIdx idx)
{
StackGuard guard(L);
size_t len{0};
const char *str = lua_tolstring(L, idx, &len);
if (str == nullptr)
{
return false;
return PeekResult::ofTypeError(L, idx, "string");
}
if (len >= INT_MAX)
{
assert(false && "string longer than INT_MAX, shit's fucked, yo");
return {false,
{QString("Strings bigger than 2.1 gigabytes are not allowed")}};
}
*out = QByteArray(str, int(len));
return true;
return {};
}

bool peek(lua_State *L, std::string *out, StackIdx idx)
PeekResult peek(lua_State *L, std::string *out, StackIdx idx)
{
StackGuard guard(L);
size_t len{0};
const char *str = lua_tolstring(L, idx, &len);
if (str == nullptr)
{
return false;
return PeekResult::ofTypeError(L, idx, "string");
}
if (len >= INT_MAX)
{
assert(false && "string longer than INT_MAX, shit's fucked, yo");
return {false,
{QString("Strings bigger than 2.1 gigabytes are not allowed")}};
}
*out = std::string(str, len);
return true;
return {};
}

bool peek(lua_State *L, api::CompletionList *out, StackIdx idx)
PeekResult peek(lua_State *L, api::CompletionList *out, StackIdx idx)
{
StackGuard guard(L);
int typ = lua_getfield(L, idx, "values");
if (typ != LUA_TTABLE)
{
lua_pop(L, 1);
return false;
auto res = PeekResult::ofTypeError(L, idx, "string");
res.errorReason.emplace_back("While processing CompletionList.values");
return res;
}
if (!lua::pop(L, &out->values, -1))
auto pres = lua::pop(L, &out->values, -1);
if (!pres)
{
return false;
pres.errorReason.emplace_back("While processing CompletionList.values");
return pres;
}
lua_getfield(L, idx, "hide_others");
return lua::pop(L, &out->hideOthers);
pres = lua::pop(L, &out->hideOthers);
if (!pres)
{
pres.errorReason.emplace_back(
"While processing CompletionList.hide_others");
}
return pres;
}

QString toString(lua_State *L, StackIdx idx)
Expand All @@ -266,5 +285,37 @@ QString toString(lua_State *L, StackIdx idx)
const auto *ptr = luaL_tolstring(L, idx, &len);
return QString::fromUtf8(ptr, int(len));
}

void PeekResult::throwAsLuaError(lua_State *L)
{
// This uses lua buffers to ensure deallocation of the error string
luaL_Buffer buf;
luaL_buffinit(L, &buf);
bool first = true;
for (const auto &s : this->errorReason)
{
if (!first)
{
luaL_addchar(&buf, '\n');
}
first = false;
luaL_addstring(&buf, s.toStdString().c_str());
}
// Remove our copy
this->errorReason.clear();

luaL_pushresult(&buf);
lua_error(L); // This call never returns
assert(false && "unreachable");
}

PeekResult PeekResult::ofTypeError(lua_State *L, StackIdx idx,
const QString &expect)
{
return PeekResult{
.ok = false,
.errorReason = {
QString("Expected %1, got %2").arg(expect, luaL_typename(L, idx))}};
}
} // namespace chatterino::lua
#endif
Loading
Loading