From bb0071e8ca1bac82d029d26fb109f9f9f62137ed Mon Sep 17 00:00:00 2001 From: GriffinR Date: Sun, 11 Aug 2024 03:11:46 -0400 Subject: [PATCH 1/2] Fix script engine memory leak --- include/scriptutility.h | 4 +++- src/scriptapi/apiutility.cpp | 28 +++++++++++++++++++--------- src/scriptapi/scripting.cpp | 14 +++++++++++--- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/include/scriptutility.h b/include/scriptutility.h index 639e71f6..0d6fca75 100644 --- a/include/scriptutility.h +++ b/include/scriptutility.h @@ -10,7 +10,8 @@ class ScriptUtility : public QObject public: ScriptUtility(MainWindow *mainWindow); - void clearActions(); + ~ScriptUtility(); + QString getActionFunctionName(int actionIndex); Q_INVOKABLE bool registerAction(QString functionName, QString actionName, QString shortcut = ""); Q_INVOKABLE bool registerToggleAction(QString functionName, QString actionName, QString shortcut = "", bool checked = false); @@ -59,6 +60,7 @@ private: MainWindow *window; QList registeredActions; + QSet activeTimers; QHash actionMap; }; diff --git a/src/scriptapi/apiutility.cpp b/src/scriptapi/apiutility.cpp index 38ff3e39..d2b8ebf3 100644 --- a/src/scriptapi/apiutility.cpp +++ b/src/scriptapi/apiutility.cpp @@ -7,6 +7,18 @@ ScriptUtility::ScriptUtility(MainWindow *mainWindow) { this->window = mainWindow; } +ScriptUtility::~ScriptUtility() { + if (window && window->ui && window->ui->menuTools) { + for (auto action : this->registeredActions) { + window->ui->menuTools->removeAction(action); + } + } + for (auto timer : this->activeTimers) { + timer->stop(); + delete timer; + } +} + bool ScriptUtility::registerAction(QString functionName, QString actionName, QString shortcut) { if (!window || !window->ui || !window->ui->menuTools) return false; @@ -44,12 +56,6 @@ bool ScriptUtility::registerToggleAction(QString functionName, QString actionNam return true; } -void ScriptUtility::clearActions() { - for (auto action : this->registeredActions) { - window->ui->menuTools->removeAction(action); - } -} - QString ScriptUtility::getActionFunctionName(int actionIndex) { return this->actionMap.value(actionIndex); } @@ -58,11 +64,15 @@ void ScriptUtility::setTimeout(QJSValue callback, int milliseconds) { if (!callback.isCallable() || milliseconds < 0) return; - QTimer *timer = new QTimer(0); + QTimer *timer = new QTimer(); connect(timer, &QTimer::timeout, [=](){ - this->callTimeoutFunction(callback); + if (this->activeTimers.remove(timer)) { + this->callTimeoutFunction(callback); + timer->deleteLater(); + } }); - connect(timer, &QTimer::timeout, timer, &QTimer::deleteLater); + + this->activeTimers.insert(timer); timer->setSingleShot(true); timer->start(milliseconds); } diff --git a/src/scriptapi/scripting.cpp b/src/scriptapi/scripting.cpp index 82944be9..95a4325f 100644 --- a/src/scriptapi/scripting.cpp +++ b/src/scriptapi/scripting.cpp @@ -1,8 +1,10 @@ +#include + #include "scripting.h" #include "log.h" #include "config.h" -QMap callbackFunctions = { +const QMap callbackFunctions = { {OnProjectOpened, "onProjectOpened"}, {OnProjectClosed, "onProjectClosed"}, {OnBlockChanged, "onBlockChanged"}, @@ -24,8 +26,9 @@ Scripting *instance = nullptr; void Scripting::stop() { if (!instance) return; instance->engine->setInterrupted(true); - instance->scriptUtility->clearActions(); qDeleteAll(instance->imageCache); + delete instance->engine; + delete instance->scriptUtility; delete instance; instance = nullptr; } @@ -39,7 +42,7 @@ void Scripting::init(MainWindow *mainWindow) { Scripting::Scripting(MainWindow *mainWindow) { this->mainWindow = mainWindow; - this->engine = new QJSEngine(mainWindow); + this->engine = new QJSEngine(); this->engine->installExtensions(QJSEngine::ConsoleExtension); const QStringList paths = userConfig.getCustomScriptPaths(); const QList enabled = userConfig.getCustomScriptsEnabled(); @@ -79,6 +82,11 @@ void Scripting::populateGlobalObject(MainWindow *mainWindow) { instance->engine->globalObject().setProperty("overlay", instance->engine->newQObject(mainWindow->ui->graphicsView_Map)); instance->engine->globalObject().setProperty("utility", instance->engine->newQObject(instance->scriptUtility)); + // Note: QJSEngine also has these functions, but not in Qt 5.15. + QQmlEngine::setObjectOwnership(mainWindow, QQmlEngine::CppOwnership); + QQmlEngine::setObjectOwnership(mainWindow->ui->graphicsView_Map, QQmlEngine::CppOwnership); + QQmlEngine::setObjectOwnership(instance->scriptUtility, QQmlEngine::CppOwnership); + QJSValue constants = instance->engine->newObject(); // Get version numbers From 724b35be951d6306ed7e2e7bc1e4cc7a00b4f332 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Tue, 13 Aug 2024 15:12:02 -0400 Subject: [PATCH 2/2] Define destructor for Scripting --- include/scripting.h | 3 ++- src/scriptapi/scripting.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/scripting.h b/include/scripting.h index e2bfb1ff..c08bbd74 100644 --- a/include/scripting.h +++ b/include/scripting.h @@ -30,8 +30,9 @@ class Scripting { public: Scripting(MainWindow *mainWindow); - static void stop(); + ~Scripting(); static void init(MainWindow *mainWindow); + static void stop(); static void populateGlobalObject(MainWindow *mainWindow); static QJSEngine *getEngine(); static void invokeAction(int actionIndex); diff --git a/src/scriptapi/scripting.cpp b/src/scriptapi/scripting.cpp index 95a4325f..71fbabca 100644 --- a/src/scriptapi/scripting.cpp +++ b/src/scriptapi/scripting.cpp @@ -24,11 +24,6 @@ const QMap callbackFunctions = { Scripting *instance = nullptr; void Scripting::stop() { - if (!instance) return; - instance->engine->setInterrupted(true); - qDeleteAll(instance->imageCache); - delete instance->engine; - delete instance->scriptUtility; delete instance; instance = nullptr; } @@ -54,6 +49,13 @@ Scripting::Scripting(MainWindow *mainWindow) { this->scriptUtility = new ScriptUtility(mainWindow); } +Scripting::~Scripting() { + this->engine->setInterrupted(true); + qDeleteAll(this->imageCache); + delete this->engine; + delete this->scriptUtility; +} + void Scripting::loadModules(QStringList moduleFiles) { for (QString filepath : moduleFiles) { QString validPath = Project::getExistingFilepath(filepath);