From 94aac9f4468a15da59f7d3187c91923fd3c6bcf8 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Mon, 4 Nov 2024 18:38:19 +0100 Subject: [PATCH] Lua: Make sure stored objects live on the main thread sol comes with "main_" variants of all reference types that automatically make sure that they are linked to the main thread and therefore don't die when a thread ends. Change-Id: I22554a706d9b4881372f9aa25d4f0c78f8c079cc Reviewed-by: hjk --- src/plugins/lua/bindings/action.cpp | 2 +- src/plugins/lua/bindings/gui.cpp | 5 +- src/plugins/lua/bindings/project.cpp | 10 +-- src/plugins/lua/bindings/settings.cpp | 96 ++++++++++--------------- src/plugins/lua/bindings/texteditor.cpp | 6 +- 5 files changed, 51 insertions(+), 68 deletions(-) diff --git a/src/plugins/lua/bindings/action.cpp b/src/plugins/lua/bindings/action.cpp index a132207704f..1a08b6ce486 100644 --- a/src/plugins/lua/bindings/action.cpp +++ b/src/plugins/lua/bindings/action.cpp @@ -69,7 +69,7 @@ void setupActionModule() if (key == "context") b.setContext(Id::fromString(v.as())); else if (key == "onTrigger") - b.addOnTriggered([f = v.as()]() { + b.addOnTriggered([f = v.as()]() { auto res = void_safe_call(f); QTC_CHECK_EXPECTED(res); }); diff --git a/src/plugins/lua/bindings/gui.cpp b/src/plugins/lua/bindings/gui.cpp index 74711c20599..2116ecd8deb 100644 --- a/src/plugins/lua/bindings/gui.cpp +++ b/src/plugins/lua/bindings/gui.cpp @@ -177,14 +177,15 @@ void setProperties(std::unique_ptr &item, const sol::table &children, QObject } if constexpr (has_onReturnPressed) { - const auto callback = children.get>("onReturnPressed"); + const auto callback = children.get>("onReturnPressed"); if (callback) { item->onReturnPressed([func = *callback]() { void_safe_call(func); }, guard); } } if constexpr (has_onRightSideIconClicked) { - const auto callback = children.get>("onRightSideIconClicked"); + const auto callback = children.get>( + "onRightSideIconClicked"); if (callback) item->onRightSideIconClicked([func = *callback]() { void_safe_call(func); }, guard); } diff --git a/src/plugins/lua/bindings/project.cpp b/src/plugins/lua/bindings/project.cpp index e140ca92790..3d256ce3391 100644 --- a/src/plugins/lua/bindings/project.cpp +++ b/src/plugins/lua/bindings/project.cpp @@ -143,7 +143,7 @@ void setupProjectModule() }); // startupProjectChanged - registerHook("projects.startupProjectChanged", [](sol::function func, QObject *guard) { + registerHook("projects.startupProjectChanged", [](sol::main_function func, QObject *guard) { QObject::connect( ProjectManager::instance(), &ProjectManager::startupProjectChanged, @@ -155,7 +155,7 @@ void setupProjectModule() }); // projectAdded - registerHook("projects.projectAdded", [](sol::function func, QObject *guard) { + registerHook("projects.projectAdded", [](sol::main_function func, QObject *guard) { QObject::connect( ProjectManager::instance(), &ProjectManager::projectAdded, @@ -167,7 +167,7 @@ void setupProjectModule() }); // projectRemoved - registerHook("projects.projectRemoved", [](sol::function func, QObject *guard) { + registerHook("projects.projectRemoved", [](sol::main_function func, QObject *guard) { QObject::connect( ProjectManager::instance(), &ProjectManager::projectRemoved, @@ -179,7 +179,7 @@ void setupProjectModule() }); // aboutToRemoveProject - registerHook("projects.aboutToRemoveProject", [](sol::function func, QObject *guard) { + registerHook("projects.aboutToRemoveProject", [](sol::main_function func, QObject *guard) { QObject::connect( ProjectManager::instance(), &ProjectManager::aboutToRemoveProject, @@ -191,7 +191,7 @@ void setupProjectModule() }); // runActionsUpdated - registerHook("projects.runActionsUpdated", [](sol::function func, QObject *guard) { + registerHook("projects.runActionsUpdated", [](sol::main_function func, QObject *guard) { QObject::connect( ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::runActionsUpdated, diff --git a/src/plugins/lua/bindings/settings.cpp b/src/plugins/lua/bindings/settings.cpp index cd04f074ff9..8bd9b2a4ec8 100644 --- a/src/plugins/lua/bindings/settings.cpp +++ b/src/plugins/lua/bindings/settings.cpp @@ -20,9 +20,7 @@ namespace Lua::Internal { class LuaAspectContainer : public AspectContainer { public: - LuaAspectContainer(sol::state_view lua) - : m_mainThreadState(lua) - {} + LuaAspectContainer() {} sol::object dynamic_get(const std::string &key) { @@ -33,17 +31,8 @@ public: return it->second; } - void dynamic_set(const std::string &key, sol::stack_object value) + void dynamic_set(const std::string &key, sol::main_object value) { - if (value.lua_state() != m_mainThreadState.lua_state()) { - // This pushes the reference onto the other Lua State and returns a stack reference - // to it. We need to do this so a stack_object from some coroutine does not die - // while we store it. - sol::stack_reference newRef = sol::stack_reference(m_mainThreadState, value); - dynamic_set(key, newRef); - return; - } - if (!value.is()) throw std::runtime_error("AspectContainer can only contain BaseAspect instances"); @@ -63,14 +52,11 @@ public: public: std::unordered_map m_entries; - // This is the Lua state of the main thread. - sol::state_view m_mainThreadState; }; -std::unique_ptr aspectContainerCreate( - sol::state_view mainThreadState, const sol::table &options) +std::unique_ptr aspectContainerCreate(const sol::main_table &options) { - auto container = std::make_unique(mainThreadState); + auto container = std::make_unique(); for (const auto &[k, v] : options) { if (k.is()) { @@ -80,7 +66,7 @@ std::unique_ptr aspectContainerCreate( } else if (key == "layouter") { if (v.is()) container->setLayouter( - [func = v.as()]() -> Layouting::Layout { + [func = v.as()]() -> Layouting::Layout { auto res = safe_call(func); QTC_ASSERT_EXPECTED(res, return {}); return *res; @@ -90,19 +76,12 @@ std::unique_ptr aspectContainerCreate( container.get(), &AspectContainer::applied, container.get(), - [func = v.as()] { void_safe_call(func); }); + [func = v.as()] { void_safe_call(func); }); } else if (key == "settingsGroup") { container->setSettingsGroup(v.as()); } else { if (v.is()) { - // Dynamic_set needs a stack reference, so we push the object back on the stack ... - v.push(); - // Take a reference to the head of the stack ... - sol::stack_reference so(v.lua_state(), -1); - // call dynamic_set with the reference ... - container->dynamic_set(key, so); - // and lastly pop the object from the stack again. - v.pop(); + container->dynamic_set(key, v); } else { qWarning() << "Unknown key:" << key.c_str(); } @@ -126,14 +105,16 @@ void baseAspectCreate(BaseAspect *aspect, const std::string &key, const sol::obj else if (key == "toolTip") aspect->setToolTip(value.as()); else if (key == "onValueChanged") { - QObject::connect(aspect, &BaseAspect::changed, aspect, [func = value.as()]() { - void_safe_call(func); - }); + QObject::connect( + aspect, &BaseAspect::changed, aspect, [func = value.as()]() { + void_safe_call(func); + }); } else if (key == "onVolatileValueChanged") { - QObject::connect(aspect, - &BaseAspect::volatileValueChanged, - aspect, - [func = value.as()] { void_safe_call(func); }); + QObject::connect( + aspect, + &BaseAspect::volatileValueChanged, + aspect, + [func = value.as()] { void_safe_call(func); }); } else if (key == "enabler") aspect->setEnabler(value.as()); else if (key == "macroExpander") { @@ -164,17 +145,17 @@ void typedAspectCreate(StringAspect *aspect, const std::string &key, const sol:: else if (key == "historyId") aspect->setHistoryCompleter(value.as().toLocal8Bit()); else if (key == "valueAcceptor") - aspect->setValueAcceptor([func = value.as()](const QString &oldValue, - const QString &newValue) - -> std::optional { - auto res = safe_call>(func, oldValue, newValue); - QTC_ASSERT_EXPECTED(res, return std::nullopt); - return *res; - }); + aspect->setValueAcceptor( + [func = value.as()](const QString &oldValue, const QString &newValue) + -> std::optional { + auto res = safe_call>(func, oldValue, newValue); + QTC_ASSERT_EXPECTED(res, return std::nullopt); + return *res; + }); else if (key == "showToolTipOnLabel") aspect->setShowToolTipOnLabel(value.as()); else if (key == "displayFilter") - aspect->setDisplayFilter([func = value.as()](const QString &value) { + aspect->setDisplayFilter([func = value.as()](const QString &value) { auto res = safe_call(func, value); QTC_ASSERT_EXPECTED(res, return value); return *res; @@ -194,7 +175,7 @@ void typedAspectCreate(StringAspect *aspect, const std::string &key, const sol:: else if (key == "completer") aspect->setCompleter(value.as()); else if (key == "addOnRightSideIconClicked") { - aspect->addOnRightSideIconClicked(aspect, [func = value.as()]() { + aspect->addOnRightSideIconClicked(aspect, [func = value.as()]() { void_safe_call(func); }); } @@ -220,7 +201,7 @@ void typedAspectCreate(FilePathAspect *aspect, const std::string &key, const sol else if (key == "validatePlaceHolder") aspect->setValidatePlaceHolder(value.as()); else if (key == "openTerminalHandler") - aspect->setOpenTerminalHandler([func = value.as()]() { + aspect->setOpenTerminalHandler([func = value.as()]() { auto res = void_safe_call(func); QTC_CHECK_EXPECTED(res); }); @@ -231,13 +212,13 @@ void typedAspectCreate(FilePathAspect *aspect, const std::string &key, const sol else if (key == "baseFileName") aspect->setBaseFileName(value.as()); else if (key == "valueAcceptor") - aspect->setValueAcceptor([func = value.as()](const QString &oldValue, - const QString &newValue) - -> std::optional { - auto res = safe_call>(func, oldValue, newValue); - QTC_ASSERT_EXPECTED(res, return std::nullopt); - return *res; - }); + aspect->setValueAcceptor( + [func = value.as()](const QString &oldValue, const QString &newValue) + -> std::optional { + auto res = safe_call>(func, oldValue, newValue); + QTC_ASSERT_EXPECTED(res, return std::nullopt); + return *res; + }); else if (key == "showToolTipOnLabel") aspect->setShowToolTipOnLabel(value.as()); else if (key == "autoApplyOnEditingFinished") @@ -249,7 +230,7 @@ void typedAspectCreate(FilePathAspect *aspect, const std::string &key, const sol }); */ else if (key == "displayFilter") - aspect->setDisplayFilter([func = value.as()](const QString &path) { + aspect->setDisplayFilter([func = value.as()](const QString &path) { auto res = safe_call(func, path); QTC_ASSERT_EXPECTED(res, return path); return *res; @@ -355,7 +336,7 @@ void setupSettingsModule() settings.new_usertype( "AspectContainer", "create", - [lua](const sol::table &options) { return aspectContainerCreate(lua, options); }, + &aspectContainerCreate, "apply", &LuaAspectContainer::apply, sol::meta_function::index, @@ -612,19 +593,20 @@ void setupSettingsModule() [](AspectList *aspect, const std::string &key, const sol::object &value) { if (key == "createItemFunction") { aspect->setCreateItemFunction( - [func = value.as()]() -> std::shared_ptr { + [func = value.as()]() + -> std::shared_ptr { auto res = safe_call>(func); QTC_ASSERT_EXPECTED(res, return nullptr); return *res; }); } else if (key == "onItemAdded") { - aspect->setItemAddedCallback([func = value.as()]( + aspect->setItemAddedCallback([func = value.as()]( std::shared_ptr item) { auto res = void_safe_call(func, item); QTC_CHECK_EXPECTED(res); }); } else if (key == "onItemRemoved") { - aspect->setItemRemovedCallback([func = value.as()]( + aspect->setItemRemovedCallback([func = value.as()]( std::shared_ptr item) { auto res = void_safe_call(func, item); QTC_CHECK_EXPECTED(res); diff --git a/src/plugins/lua/bindings/texteditor.cpp b/src/plugins/lua/bindings/texteditor.cpp index a774f1d6cbf..c4f5977d6b1 100644 --- a/src/plugins/lua/bindings/texteditor.cpp +++ b/src/plugins/lua/bindings/texteditor.cpp @@ -392,7 +392,7 @@ void setupTextEditorModule() return result; }); - registerHook("editors.text.currentChanged", [](sol::function func, QObject *guard) { + registerHook("editors.text.currentChanged", [](sol::main_function func, QObject *guard) { QObject::connect( TextEditorRegistry::instance(), &TextEditorRegistry::currentEditorChanged, @@ -403,7 +403,7 @@ void setupTextEditorModule() }); }); - registerHook("editors.text.contentsChanged", [](sol::function func, QObject *guard) { + registerHook("editors.text.contentsChanged", [](sol::main_function func, QObject *guard) { QObject::connect( TextEditorRegistry::instance(), &TextEditorRegistry::documentContentsChanged, @@ -415,7 +415,7 @@ void setupTextEditorModule() }); }); - registerHook("editors.text.cursorChanged", [](sol::function func, QObject *guard) { + registerHook("editors.text.cursorChanged", [](sol::main_function func, QObject *guard) { QObject::connect( TextEditorRegistry::instance(), &TextEditorRegistry::currentCursorChanged,