From cfaf851e268dac95cb140fbffc1233981ce48d4c Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 21 Jul 2014 16:36:24 -0700 Subject: Fix a few more race conditions with QLibrary::LoadHints This commit makes replaces the loadHints member with a setter, a getter and an atomic variable. The setter will not set anything if the library has already been loaded. Task-number: QTBUG-39642 Change-Id: Ibb7692f16d80211b52aaf4dc88db1a989738a24d Reviewed-by: David Faure --- src/corelib/plugin/qlibrary.cpp | 35 ++++++++++++++++++++++------------- src/corelib/plugin/qlibrary_p.h | 7 ++++++- src/corelib/plugin/qlibrary_unix.cpp | 1 + src/corelib/plugin/qpluginloader.cpp | 11 +++-------- 4 files changed, 32 insertions(+), 22 deletions(-) (limited to 'src/corelib/plugin') diff --git a/src/corelib/plugin/qlibrary.cpp b/src/corelib/plugin/qlibrary.cpp index 50281b632a..ae53dcdc32 100644 --- a/src/corelib/plugin/qlibrary.cpp +++ b/src/corelib/plugin/qlibrary.cpp @@ -485,9 +485,9 @@ inline void QLibraryStore::releaseLibrary(QLibraryPrivate *lib) QLibraryPrivate::QLibraryPrivate(const QString &canonicalFileName, const QString &version, QLibrary::LoadHints loadHints) : pHnd(0), fileName(canonicalFileName), fullVersion(version), instance(0), - loadHints(loadHints), libraryRefCount(0), libraryUnloadCount(0), pluginState(MightBeAPlugin) { + loadHintsInt.store(loadHints); if (canonicalFileName.isEmpty()) errorString = QLibrary::tr("The shared library was not found."); } @@ -508,7 +508,7 @@ void QLibraryPrivate::mergeLoadHints(QLibrary::LoadHints lh) if (pHnd) return; - loadHints = lh; + loadHintsInt.store(lh); } QFunctionPointer QLibraryPrivate::resolve(const char *symbol) @@ -518,6 +518,12 @@ QFunctionPointer QLibraryPrivate::resolve(const char *symbol) return resolve_sys(symbol); } +void QLibraryPrivate::setLoadHints(QLibrary::LoadHints lh) +{ + // this locks a global mutex + QMutexLocker lock(&qt_library_mutex); + mergeLoadHints(lh); +} bool QLibraryPrivate::load() { @@ -914,13 +920,12 @@ void QLibrary::setFileName(const QString &fileName) { QLibrary::LoadHints lh; if (d) { - lh = d->loadHints; + lh = d->loadHints(); d->release(); d = 0; did_load = false; } - d = QLibraryPrivate::findOrCreate(fileName); - d->loadHints = lh; + d = QLibraryPrivate::findOrCreate(fileName, QString(), lh); } QString QLibrary::fileName() const @@ -943,13 +948,12 @@ void QLibrary::setFileNameAndVersion(const QString &fileName, int verNum) { QLibrary::LoadHints lh; if (d) { - lh = d->loadHints; + lh = d->loadHints(); d->release(); d = 0; did_load = false; } - d = QLibraryPrivate::findOrCreate(fileName, verNum >= 0 ? QString::number(verNum) : QString()); - d->loadHints = lh; + d = QLibraryPrivate::findOrCreate(fileName, verNum >= 0 ? QString::number(verNum) : QString(), lh); } /*! @@ -965,13 +969,12 @@ void QLibrary::setFileNameAndVersion(const QString &fileName, const QString &ver { QLibrary::LoadHints lh; if (d) { - lh = d->loadHints; + lh = d->loadHints(); d->release(); d = 0; did_load = false; } - d = QLibraryPrivate::findOrCreate(fileName, version); - d->loadHints = lh; + d = QLibraryPrivate::findOrCreate(fileName, version, lh); } /*! @@ -1102,6 +1105,12 @@ QString QLibrary::errorString() const By default, none of these flags are set, so libraries will be loaded with lazy symbol resolution, and will not export external symbols for resolution in other dynamically-loaded libraries. + + \note Setting this property after the library has been loaded has no effect + and loadHints() will not reflect those changes. + + \note This property is shared among all QLibrary instances that refer to + the same library. */ void QLibrary::setLoadHints(LoadHints hints) { @@ -1109,12 +1118,12 @@ void QLibrary::setLoadHints(LoadHints hints) d = QLibraryPrivate::findOrCreate(QString()); // ugly, but we need a d-ptr d->errorString.clear(); } - d->loadHints = hints; + d->setLoadHints(hints); } QLibrary::LoadHints QLibrary::loadHints() const { - return d ? d->loadHints : (QLibrary::LoadHints)0; + return d ? d->loadHints() : QLibrary::LoadHints(); } /* Internal, for debugging */ diff --git a/src/corelib/plugin/qlibrary_p.h b/src/corelib/plugin/qlibrary_p.h index e58d18e87b..df95d1342d 100644 --- a/src/corelib/plugin/qlibrary_p.h +++ b/src/corelib/plugin/qlibrary_p.h @@ -94,6 +94,10 @@ public: void release(); QFunctionPointer resolve(const char *); + QLibrary::LoadHints loadHints() const + { return QLibrary::LoadHints(loadHintsInt.load()); } + void setLoadHints(QLibrary::LoadHints lh); + static QLibraryPrivate *findOrCreate(const QString &fileName, const QString &version = QString(), QLibrary::LoadHints loadHints = 0); static QStringList suffixes_sys(const QString &fullVersion); @@ -104,7 +108,6 @@ public: QJsonObject metaData; QString errorString; - QLibrary::LoadHints loadHints; void updatePluginState(); bool isPlugin(); @@ -126,6 +129,8 @@ private: bool unload_sys(); QFunctionPointer resolve_sys(const char *); + QAtomicInt loadHintsInt; + /// counts how many QLibrary or QPluginLoader are attached to us, plus 1 if it's loaded QAtomicInt libraryRefCount; /// counts how many times load() or loadPlugin() were called diff --git a/src/corelib/plugin/qlibrary_unix.cpp b/src/corelib/plugin/qlibrary_unix.cpp index 43e2b5c15b..b9af143f35 100644 --- a/src/corelib/plugin/qlibrary_unix.cpp +++ b/src/corelib/plugin/qlibrary_unix.cpp @@ -162,6 +162,7 @@ bool QLibraryPrivate::load_sys() dlFlags |= BIND_DEFERRED; } #else + int loadHints = this->loadHints(); if (loadHints & QLibrary::ResolveAllSymbolsHint) { dlFlags |= RTLD_NOW; } else { diff --git a/src/corelib/plugin/qpluginloader.cpp b/src/corelib/plugin/qpluginloader.cpp index 2c139669e6..f34c019823 100644 --- a/src/corelib/plugin/qpluginloader.cpp +++ b/src/corelib/plugin/qpluginloader.cpp @@ -339,7 +339,7 @@ void QPluginLoader::setFileName(const QString &fileName) #if defined(QT_SHARED) QLibrary::LoadHints lh; if (d) { - lh = d->loadHints; + lh = d->loadHints(); d->release(); d = 0; did_load = false; @@ -405,17 +405,12 @@ void QPluginLoader::setLoadHints(QLibrary::LoadHints loadHints) d = QLibraryPrivate::findOrCreate(QString()); // ugly, but we need a d-ptr d->errorString.clear(); } - d->loadHints = loadHints; + d->setLoadHints(loadHints); } QLibrary::LoadHints QPluginLoader::loadHints() const { - if (!d) { - QPluginLoader *that = const_cast(this); - that->d = QLibraryPrivate::findOrCreate(QString()); // ugly, but we need a d-ptr - that->d->errorString.clear(); - } - return d->loadHints; + return d ? d->loadHints() : QLibrary::LoadHints(); } /*! -- cgit v1.2.3