feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

Extensionsystem: Fix memory leak

Browse Source 
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 <eike.ziller@qt.io>
This commit is contained in:
Marcus Tillmanns
2024-09-23 09:29:29 +02:00
parent 1daa7d34a2
commit ba54e05e58
4 changed files with 23 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/libs/extensionsystem/pluginmanager.cpp
View File

@@ -1877,21 +1877,21 @@ void PluginManagerPrivate::readPluginPaths()
// from the file system // from the file system
for (const FilePath &pluginFile : pluginFiles(pluginPaths)) { for (const FilePath &pluginFile : pluginFiles(pluginPaths)) {
expected_str<PluginSpec *> spec = readCppPluginSpec(pluginFile); expected_str<std::unique_ptr<PluginSpec>> spec = readCppPluginSpec(pluginFile);
if (!spec) { if (!spec) {
qCInfo(pluginLog).noquote() << QString("Ignoring plugin \"%1\" because: %2") qCInfo(pluginLog).noquote() << QString("Ignoring plugin \"%1\" because: %2")
.arg(pluginFile.toUserOutput()) .arg(pluginFile.toUserOutput())
.arg(spec.error()); .arg(spec.error());
continue; continue;
} }
newSpecs.append(*spec); newSpecs.append(spec->release());
} }
// static // static
for (const QStaticPlugin &plugin : QPluginLoader::staticPlugins()) { for (const QStaticPlugin &plugin : QPluginLoader::staticPlugins()) {
expected_str<PluginSpec *> spec = readCppPluginSpec(plugin); expected_str<std::unique_ptr<PluginSpec>> spec = readCppPluginSpec(plugin);
QTC_ASSERT_EXPECTED(spec, continue); QTC_ASSERT_EXPECTED(spec, continue);
newSpecs.append(*spec); newSpecs.append(spec->release());
} }
addPlugins(newSpecs); addPlugins(newSpecs);

13
src/libs/extensionsystem/pluginspec.cpp
View File

@@ -747,9 +747,9 @@ namespace {
\internal \internal
Returns false if the file does not represent a Qt Creator plugin. Returns false if the file does not represent a Qt Creator plugin.
*/ */
expected_str<PluginSpec *> readCppPluginSpec(const FilePath &fileName) expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(const FilePath &fileName)
{ {
auto spec = new CppPluginSpec; auto spec = std::unique_ptr<CppPluginSpec>(new CppPluginSpec());
const FilePath absPath = fileName.absoluteFilePath(); const FilePath absPath = fileName.absoluteFilePath();
@@ -771,9 +771,9 @@ expected_str<PluginSpec *> readCppPluginSpec(const FilePath &fileName)
return spec; return spec;
} }
expected_str<PluginSpec *> readCppPluginSpec(const QStaticPlugin &plugin) expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(const QStaticPlugin &plugin)
{ {
auto spec = new CppPluginSpec; auto spec = std::unique_ptr<CppPluginSpec>(new CppPluginSpec());
qCDebug(pluginLog) << "\nReading meta data of static plugin"; qCDebug(pluginLog) << "\nReading meta data of static plugin";
spec->d->staticPlugin = plugin; spec->d->staticPlugin = plugin;
@@ -1398,9 +1398,10 @@ static QList<PluginSpec *> createCppPluginsFromArchive(const FilePath &path)
while (it.hasNext()) { while (it.hasNext()) {
it.next(); it.next();
expected_str<PluginSpec *> spec = readCppPluginSpec(FilePath::fromUserInput(it.filePath())); expected_str<std::unique_ptr<PluginSpec>> spec = readCppPluginSpec(
FilePath::fromUserInput(it.filePath()));
if (spec) if (spec)
results.push_back(*spec); results.push_back(spec->release());
} }
return results; return results;
} }

8
src/libs/extensionsystem/pluginspec.h
View File

@@ -191,16 +191,16 @@ using PluginFromArchiveFactory = std::function<QList<PluginSpec *>(const Utils::
EXTENSIONSYSTEM_EXPORT QList<PluginFromArchiveFactory> &pluginSpecsFromArchiveFactories(); EXTENSIONSYSTEM_EXPORT QList<PluginFromArchiveFactory> &pluginSpecsFromArchiveFactories();
EXTENSIONSYSTEM_EXPORT QList<PluginSpec *> pluginSpecsFromArchive(const Utils::FilePath &path); EXTENSIONSYSTEM_EXPORT QList<PluginSpec *> pluginSpecsFromArchive(const Utils::FilePath &path);
EXTENSIONSYSTEM_EXPORT Utils::expected_str<PluginSpec *> readCppPluginSpec( EXTENSIONSYSTEM_EXPORT Utils::expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(
const Utils::FilePath &filePath); const Utils::FilePath &filePath);
EXTENSIONSYSTEM_EXPORT Utils::expected_str<PluginSpec *> readCppPluginSpec( EXTENSIONSYSTEM_EXPORT Utils::expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(
const QStaticPlugin &plugin); const QStaticPlugin &plugin);
class EXTENSIONSYSTEM_TEST_EXPORT CppPluginSpec : public PluginSpec class EXTENSIONSYSTEM_TEST_EXPORT CppPluginSpec : public PluginSpec
{ {
friend EXTENSIONSYSTEM_EXPORT Utils::expected_str<PluginSpec *> readCppPluginSpec( friend EXTENSIONSYSTEM_EXPORT Utils::expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(
const Utils::FilePath &filePath); const Utils::FilePath &filePath);
friend EXTENSIONSYSTEM_EXPORT Utils::expected_str<PluginSpec *> readCppPluginSpec( friend EXTENSIONSYSTEM_EXPORT Utils::expected_str<std::unique_ptr<PluginSpec>> readCppPluginSpec(
const QStaticPlugin &plugin); const QStaticPlugin &plugin);
public: public:

18
tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp
View File

@@ -234,10 +234,10 @@ void tst_PluginSpec::experimental()
void tst_PluginSpec::locationAndPath() void tst_PluginSpec::locationAndPath()
{ {
Utils::expected_str<PluginSpec *> ps = readCppPluginSpec( Utils::expected_str<std::unique_ptr<PluginSpec>> ps = readCppPluginSpec(
PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test")));
QVERIFY(ps); QVERIFY(ps);
CppPluginSpec *spec = static_cast<CppPluginSpec *>(*ps); CppPluginSpec *spec = static_cast<CppPluginSpec *>(ps->get());
QCOMPARE(spec->location(), PLUGIN_DIR_PATH / "testplugin"); QCOMPARE(spec->location(), PLUGIN_DIR_PATH / "testplugin");
QCOMPARE(spec->filePath(), PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); QCOMPARE(spec->filePath(), PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test")));
} }
@@ -288,11 +288,11 @@ void tst_PluginSpec::resolveDependencies()
void tst_PluginSpec::loadLibrary() void tst_PluginSpec::loadLibrary()
{ {
Utils::expected_str<PluginSpec *> ps = readCppPluginSpec( Utils::expected_str<std::unique_ptr<PluginSpec>> ps = readCppPluginSpec(
PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test")));
QVERIFY(ps); QVERIFY(ps);
CppPluginSpec *spec = static_cast<CppPluginSpec *>(*ps); CppPluginSpec *spec = static_cast<CppPluginSpec *>(ps->get());
QCOMPARE(spec->errorString(), QString()); QCOMPARE(spec->errorString(), QString());
QVERIFY(spec->resolveDependencies({})); QVERIFY(spec->resolveDependencies({}));
@@ -302,16 +302,14 @@ void tst_PluginSpec::loadLibrary()
== QLatin1String("MyPlugin::MyPluginImpl")); == QLatin1String("MyPlugin::MyPluginImpl"));
QCOMPARE(spec->state(), PluginSpec::Loaded); QCOMPARE(spec->state(), PluginSpec::Loaded);
QVERIFY(!spec->hasError()); QVERIFY(!spec->hasError());
delete *ps;
} }
void tst_PluginSpec::initializePlugin() void tst_PluginSpec::initializePlugin()
{ {
Utils::expected_str<PluginSpec *> ps = readCppPluginSpec( Utils::expected_str<std::unique_ptr<PluginSpec>> ps = readCppPluginSpec(
PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test")));
QVERIFY(ps); QVERIFY(ps);
CppPluginSpec *spec = static_cast<CppPluginSpec *>(*ps); CppPluginSpec *spec = static_cast<CppPluginSpec *>(ps->get());
QVERIFY(spec->resolveDependencies({})); QVERIFY(spec->resolveDependencies({}));
QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString())); QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString()));
bool isInitialized; bool isInitialized;
@@ -332,10 +330,10 @@ void tst_PluginSpec::initializePlugin()
void tst_PluginSpec::initializeExtensions() void tst_PluginSpec::initializeExtensions()
{ {
Utils::expected_str<PluginSpec *> ps = readCppPluginSpec( Utils::expected_str<std::unique_ptr<PluginSpec>> ps = readCppPluginSpec(
PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test"))); PLUGIN_DIR_PATH / "testplugin" / libraryName(QLatin1String("test")));
QVERIFY(ps); QVERIFY(ps);
CppPluginSpec *spec = static_cast<CppPluginSpec *>(*ps); CppPluginSpec *spec = static_cast<CppPluginSpec *>(ps->get());
QVERIFY(spec->resolveDependencies({})); QVERIFY(spec->resolveDependencies({}));
QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString())); QVERIFY2(spec->loadLibrary(), qPrintable(spec->errorString()));
bool isExtensionsInitialized; bool isExtensionsInitialized;