From ba54e05e58b4b9dc15217973a1d15befb7bdf235 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Mon, 23 Sep 2024 09:29:29 +0200 Subject: [PATCH] Extensionsystem: Fix memory leak Changed return value of readCppPluginSpec to unique_ptr to make ownership more obvious. Memory was leaked when an error was returned. The spec tests were also leaking. Change-Id: Icf4d561b57725b90583b5d7f4e72c2e3879d3b08 Reviewed-by: Eike Ziller --- src/libs/extensionsystem/pluginmanager.cpp | 8 ++++---- src/libs/extensionsystem/pluginspec.cpp | 13 +++++++------ src/libs/extensionsystem/pluginspec.h | 8 ++++---- .../pluginspec/tst_pluginspec.cpp | 18 ++++++++---------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/libs/extensionsystem/pluginmanager.cpp b/src/libs/extensionsystem/pluginmanager.cpp index e4a278ec9bc..7560c86a85d 100644 --- a/src/libs/extensionsystem/pluginmanager.cpp +++ b/src/libs/extensionsystem/pluginmanager.cpp @@ -1877,21 +1877,21 @@ void PluginManagerPrivate::readPluginPaths() // from the file system for (const FilePath &pluginFile : pluginFiles(pluginPaths)) { - expected_str spec = readCppPluginSpec(pluginFile); + expected_str> spec = readCppPluginSpec(pluginFile); if (!spec) { qCInfo(pluginLog).noquote() << QString("Ignoring plugin \"%1\" because: %2") .arg(pluginFile.toUserOutput()) .arg(spec.error()); continue; } - newSpecs.append(*spec); + newSpecs.append(spec->release()); } // static for (const QStaticPlugin &plugin : QPluginLoader::staticPlugins()) { - expected_str spec = readCppPluginSpec(plugin); + expected_str> spec = readCppPluginSpec(plugin); QTC_ASSERT_EXPECTED(spec, continue); - newSpecs.append(*spec); + newSpecs.append(spec->release()); } addPlugins(newSpecs); diff --git a/src/libs/extensionsystem/pluginspec.cpp b/src/libs/extensionsystem/pluginspec.cpp index c5622786cc2..302690da8f2 100644 --- a/src/libs/extensionsystem/pluginspec.cpp +++ b/src/libs/extensionsystem/pluginspec.cpp @@ -747,9 +747,9 @@ namespace { \internal Returns false if the file does not represent a Qt Creator plugin. */ -expected_str readCppPluginSpec(const FilePath &fileName) +expected_str> readCppPluginSpec(const FilePath &fileName) { - auto spec = new CppPluginSpec; + auto spec = std::unique_ptr(new CppPluginSpec()); const FilePath absPath = fileName.absoluteFilePath(); @@ -771,9 +771,9 @@ expected_str readCppPluginSpec(const FilePath &fileName) return spec; } -expected_str readCppPluginSpec(const QStaticPlugin &plugin) +expected_str> readCppPluginSpec(const QStaticPlugin &plugin) { - auto spec = new CppPluginSpec; + auto spec = std::unique_ptr(new CppPluginSpec()); qCDebug(pluginLog) << "\nReading meta data of static plugin"; spec->d->staticPlugin = plugin; @@ -1398,9 +1398,10 @@ static QList createCppPluginsFromArchive(const FilePath &path) while (it.hasNext()) { it.next(); - expected_str spec = readCppPluginSpec(FilePath::fromUserInput(it.filePath())); + expected_str> spec = readCppPluginSpec( + FilePath::fromUserInput(it.filePath())); if (spec) - results.push_back(*spec); + results.push_back(spec->release()); } return results; } diff --git a/src/libs/extensionsystem/pluginspec.h b/src/libs/extensionsystem/pluginspec.h index b8d972f9ce0..2f5a5a37209 100644 --- a/src/libs/extensionsystem/pluginspec.h +++ b/src/libs/extensionsystem/pluginspec.h @@ -191,16 +191,16 @@ using PluginFromArchiveFactory = std::function(const Utils:: EXTENSIONSYSTEM_EXPORT QList &pluginSpecsFromArchiveFactories(); EXTENSIONSYSTEM_EXPORT QList pluginSpecsFromArchive(const Utils::FilePath &path); -EXTENSIONSYSTEM_EXPORT Utils::expected_str readCppPluginSpec( +EXTENSIONSYSTEM_EXPORT Utils::expected_str> readCppPluginSpec( const Utils::FilePath &filePath); -EXTENSIONSYSTEM_EXPORT Utils::expected_str readCppPluginSpec( +EXTENSIONSYSTEM_EXPORT Utils::expected_str> readCppPluginSpec( const QStaticPlugin &plugin); class EXTENSIONSYSTEM_TEST_EXPORT CppPluginSpec : public PluginSpec { - friend EXTENSIONSYSTEM_EXPORT Utils::expected_str readCppPluginSpec( + friend EXTENSIONSYSTEM_EXPORT Utils::expected_str> readCppPluginSpec( const Utils::FilePath &filePath); - friend EXTENSIONSYSTEM_EXPORT Utils::expected_str readCppPluginSpec( + friend EXTENSIONSYSTEM_EXPORT Utils::expected_str> readCppPluginSpec( const QStaticPlugin &plugin); public: diff --git a/tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp b/tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp index b8735bca5e7..3a4a9444400 100644 --- a/tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp +++ b/tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp @@ -234,10 +234,10 @@ void tst_PluginSpec::experimental() void tst_PluginSpec::locationAndPath() { - Utils::expected_str ps = readCppPluginSpec( + Utils::expected_str> ps = readCppPluginSpec( PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); QVERIFY(ps); - CppPluginSpec *spec = static_cast(*ps); + CppPluginSpec *spec = static_cast(ps->get()); QCOMPARE(spec->location(), PLUGIN_DIR_PATH / "testplugin"); QCOMPARE(spec->filePath(), PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); } @@ -288,11 +288,11 @@ void tst_PluginSpec::resolveDependencies() void tst_PluginSpec::loadLibrary() { - Utils::expected_str ps = readCppPluginSpec( + Utils::expected_str> ps = readCppPluginSpec( PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); QVERIFY(ps); - CppPluginSpec *spec = static_cast(*ps); + CppPluginSpec *spec = static_cast(ps->get()); QCOMPARE(spec->errorString(), QString()); QVERIFY(spec->resolveDependencies({})); @@ -302,16 +302,14 @@ void tst_PluginSpec::loadLibrary() == QLatin1String("MyPlugin::MyPluginImpl")); QCOMPARE(spec->state(), PluginSpec::Loaded); QVERIFY(!spec->hasError()); - - delete *ps; } void tst_PluginSpec::initializePlugin() { - Utils::expected_str ps = readCppPluginSpec( + Utils::expected_str> ps = readCppPluginSpec( PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); QVERIFY(ps); - CppPluginSpec *spec = static_cast(*ps); + CppPluginSpec *spec = static_cast(ps->get()); QVERIFY(spec->resolveDependencies({})); QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString())); bool isInitialized; @@ -332,10 +330,10 @@ void tst_PluginSpec::initializePlugin() void tst_PluginSpec::initializeExtensions() { - Utils::expected_str ps = readCppPluginSpec( + Utils::expected_str> ps = readCppPluginSpec( PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); QVERIFY(ps); - CppPluginSpec *spec = static_cast(*ps); + CppPluginSpec *spec = static_cast(ps->get()); QVERIFY(spec->resolveDependencies({})); QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString())); bool isExtensionsInitialized;