whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QFactoryLoader: fix mem-leak on setExtraSearchPath() 

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 ddba24535f.

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

Pick-to: 6.6 6.5
Fixes: QTBUG-115286
Change-Id: I6d1272622b12c505975cc72f9aba0d126d2817e2
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
bb10
Marc Mutz 2023-07-18 07:02:58 +02:00
parent 575b8a7fa2
commit e60aed5ed0
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;