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 <david.faure@kdab.com>

4 files changed, 32 insertions, 22 deletions

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<QPluginLoader *>(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();
 }
 
 /*!