1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QFactoryLoader: fix mem-leak on setExtraSearchPath()

Browse Source 
When, like in tst_QFactoryLoader::extraSearchPath(), where asan caught
it, or, presumably, on re-creation of a QGuiApplication with a
different QT_QPA_PLATFORM_PLUGIN_PATH, setExtraSearchPath() is called
with a different path than before, then it would leak QLibaryPrivate
objects in the call to libraryList.clear().

Fix by adding QLibraryPrivate::Deleter and holding the objects in
unique_ptr<QLibraryPrivate, Deleter> instead of as raw pointers. This
statically guarantees we're not leaking these objects anywhere else in
QFactoryLoader.

Change the name of the container from libraryList to libraries to catch
any unported users, incl. in older branches.

Since libraryList is now a std::vector (QList cannot hold move-only
types), statically assert that it was never attempted to be copied or
moved, even in older branches, with Q_DISABLE_COPY_MOVE().

Amends ddba24535fb5732c3cb757414cf1a393bd98f693.

Not picking to 6.4 and 6.3, as they are closed at this point.

Fixes: QTBUG-115286
Change-Id: I6d1272622b12c505975cc72f9aba0d126d2817e2
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit e60aed5ed000b635d8424f9120249725d9e68c78)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2023-07-18 07:02:58 +02:00 committed by Qt Cherry-pick Bot
parent b528f9b1c6
commit 30ffd6639f
2 changed files with 21 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/corelib/plugin/qfactoryloader.cpp
View File

@ -32,6 +32,8 @@
#include <qtcore_tracepoints_p.h>
#include <vector>
QT_BEGIN_NAMESPACE
using namespace Qt::StringLiterals;
@ -98,6 +100,7 @@ QJsonObject QPluginParsedMetaData::toJson() const
class QFactoryLoaderPrivate : public QObjectPrivate
{
Q_DECLARE_PUBLIC(QFactoryLoader)
Q_DISABLE_COPY_MOVE(QFactoryLoaderPrivate)
public:
QFactoryLoaderPrivate() { }
QByteArray iid;
@ -105,7 +108,7 @@ public:
~QFactoryLoaderPrivate();
mutable QMutex mutex;
QDuplicateTracker<QString> loadedPaths;
QList<QLibraryPrivate*> libraryList;
std::vector<QLibraryPrivate::UniquePtr> libraries;
QMap<QString,QLibraryPrivate*> keyMap;
QString suffix;
QString extraSearchPath;
@ -133,10 +136,7 @@ struct QFactoryLoaderGlobals
Q_GLOBAL_STATIC(QFactoryLoaderGlobals, qt_factoryloader_global)
QFactoryLoaderPrivate::~QFactoryLoaderPrivate()
{
for (QLibraryPrivate *library : std::as_const(libraryList))
library->release();
}
= default;
inline void QFactoryLoaderPrivate::updateSinglePath(const QString &path)
{
@ -184,7 +184,7 @@ inline void QFactoryLoaderPrivate::updateSinglePath(const QString &path)
Q_TRACE(QFactoryLoader_update, fileName);
std::unique_ptr<QLibraryPrivate, LibraryReleaser> library;
QLibraryPrivate::UniquePtr library;
library.reset(QLibraryPrivate::findOrCreate(QFileInfo(fileName).canonicalFilePath()));
if (!library->isPlugin()) {
qCDebug(lcFactoryLoader) << library->errorString << Qt::endl
@ -229,7 +229,7 @@ inline void QFactoryLoaderPrivate::updateSinglePath(const QString &path)
if (keyUsageCount || keys.isEmpty()) {
library->setLoadHints(QLibrary::PreventUnloadHint); // once loaded, don't unload
QMutexLocker locker(&mutex);
libraryList += library.release();
libraries.push_back(std::move(library));
}
};
}
@ -328,7 +328,7 @@ void QFactoryLoader::setExtraSearchPath(const QString &path)
} else {
// must re-scan everything
d->loadedPaths.clear();
d->libraryList.clear();
d->libraries.clear();
d->keyMap.clear();
update();
}
@ -343,7 +343,7 @@ QFactoryLoader::MetaDataList QFactoryLoader::metaData() const
QList<QPluginParsedMetaData> metaData;
#if QT_CONFIG(library)
QMutexLocker locker(&d->mutex);
for (QLibraryPrivate *library : std::as_const(d->libraryList))
for (const auto &library : d->libraries)
metaData.append(library->metaData);
#endif
@ -369,8 +369,8 @@ QObject *QFactoryLoader::instance(int index) const
#if QT_CONFIG(library)
QMutexLocker lock(&d->mutex);
if (index < d->libraryList.size()) {
QLibraryPrivate *library = d->libraryList.at(index);
if (size_t(index) < d->libraries.size()) {
QLibraryPrivate *library = d->libraries[index].get();
if (QObject *obj = library->pluginInstance()) {
if (!obj->parent())
obj->moveToThread(QCoreApplicationPrivate::mainThread());
@ -378,7 +378,8 @@ QObject *QFactoryLoader::instance(int index) const
}
return nullptr;
}
index -= d->libraryList.size();
// we know d->libraries.size() <= index <= numeric_limits<decltype(index)>::max() → no overflow
index -= static_cast<int>(d->libraries.size());
lock.unlock();
#endif

8
src/corelib/plugin/qlibrary_p.h
View File

@ -28,6 +28,8 @@
# include "QtCore/qt_windows.h"
#endif
#include <memory>
QT_REQUIRE_CONFIG(library);
QT_BEGIN_NAMESPACE
@ -54,6 +56,12 @@ public:
#endif
enum UnloadFlag { UnloadSys, NoUnloadSys };
struct Deleter {
// QLibraryPrivate::release() is not, yet, and cannot easily be made, noexcept:
void operator()(QLibraryPrivate *p) const { p->release(); }
};
using UniquePtr = std::unique_ptr<QLibraryPrivate, Deleter>;
const QString fileName;
const QString fullVersion;