QLibraryPrivate: Actually merge load hints

Or old and new load hints in mergeLoadHints() instead of just storing
new ones. Andjust QLibraryPrivate::setLoadHints() to handle objects
with no file name differently and just set load hints directly.

Mention that load hints are merged once the file name is set
in the documentation for QLibrary::setLoadHints().

Add a regression test into tst_qfactoryloader.

Update and extend tst_QPluginLoader::loadHints() to take into account
load hints merging.

Fixes: QTBUG-114480
Change-Id: I3b9afaec7acde1f5ff992d913f8d7217392c7e00
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/plugin/qlibrary.cpp

@@ -465,7 +465,7 @@ void QLibraryPrivate::mergeLoadHints(QLibrary::LoadHints lh)
     if (pHnd.loadRelaxed())
         return;
 
-    loadHintsInt.storeRelaxed(lh.toInt());
+    loadHintsInt.fetchAndOrRelaxed(lh.toInt());
 }
 
 QFunctionPointer QLibraryPrivate::resolve(const char *symbol)
@@ -477,6 +477,13 @@ QFunctionPointer QLibraryPrivate::resolve(const char *symbol)
 
 void QLibraryPrivate::setLoadHints(QLibrary::LoadHints lh)
 {
+    // Set the load hints directly for a dummy if this object is not associated
+    // with a file. Such object is not shared between multiple instances.
+    if (fileName.isEmpty()) {
+        loadHintsInt.storeRelaxed(lh.toInt());
+        return;
+    }
+
     // this locks a global mutex
     QMutexLocker lock(&qt_library_mutex);
     mergeLoadHints(lh);
@@ -1113,6 +1120,10 @@ QString QLibrary::errorString() const
     lazy symbol resolution, and will not export external symbols for resolution
     in other dynamically-loaded libraries.
 
+    \note Hints can only be cleared when this object is not associated with a
+    file. Hints can only be added once the file name is set (\a hints will
+    be or'ed with the old hints).
+
     \note Setting this property after the library has been loaded has no effect
     and loadHints() will not reflect those changes.
 

src/corelib/plugin/qpluginloader.cpp

@@ -349,10 +349,11 @@ QString QPluginLoader::errorString() const
 void QPluginLoader::setLoadHints(QLibrary::LoadHints loadHints)
 {
     if (!d) {
-        d = QLibraryPrivate::findOrCreate(QString());   // ugly, but we need a d-ptr
+        d = QLibraryPrivate::findOrCreate({}, {}, loadHints); // ugly, but we need a d-ptr
         d->errorString.clear();
+    } else {
+        d->setLoadHints(loadHints);
     }
-    d->setLoadHints(loadHints);
 }
 
 QLibrary::LoadHints QPluginLoader::loadHints() const

tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.h

@@ -10,7 +10,7 @@
 class Plugin1 : public QObject, public PluginInterface1
 {
     Q_OBJECT
-    Q_PLUGIN_METADATA(IID "org.qt-project.Qt.autotests.plugininterface1")
+    Q_PLUGIN_METADATA(IID "org.qt-project.Qt.autotests.plugininterface1" FILE "plugin1.json")
     Q_INTERFACES(PluginInterface1)
 
 public:

tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.json

@@ -0,0 +1,5 @@
+{
+    "Keys": [
+        "plugin1"
+    ]
+}

tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp

@@ -6,6 +6,7 @@
 #include <QtCore/qfileinfo.h>
 #include <QtCore/qplugin.h>
 #include <private/qfactoryloader_p.h>
+#include <private/qlibrary_p.h>
 #include "plugin1/plugininterface1.h"
 #include "plugin2/plugininterface2.h"
 
@@ -29,6 +30,7 @@ public slots:
 private slots:
     void usingTwoFactoriesFromSameDir();
     void extraSearchPath();
+    void multiplePaths();
 };
 
 static const char binFolderC[] = "bin";
@@ -109,5 +111,28 @@ void tst_QFactoryLoader::extraSearchPath()
 #endif
 }
 
+void tst_QFactoryLoader::multiplePaths()
+{
+#if !QT_CONFIG(library) || !(defined(Q_OS_UNIX) && !defined(Q_OS_DARWIN)) || defined(Q_OS_ANDROID)
+    QSKIP("Test not applicable in this configuration.");
+#else
+    QTemporaryDir dir;
+    QVERIFY(dir.isValid());
+
+    QString pluginsPath = QFileInfo(binFolder, binFolderC).absolutePath();
+    QString linkPath = dir.filePath(binFolderC);
+    QVERIFY(QFile::link(pluginsPath, linkPath));
+
+    QCoreApplication::setLibraryPaths({ QFileInfo(binFolder).absolutePath(), dir.path() });
+
+    const QString suffix = QLatin1Char('/') + QLatin1String(binFolderC);
+    QFactoryLoader loader1(PluginInterface1_iid, suffix);
+
+    QLibraryPrivate *library1 = loader1.library("plugin1");
+    QVERIFY(library1);
+    QCOMPARE(library1->loadHints(), QLibrary::PreventUnloadHint);
+#endif
+}
+
 QTEST_MAIN(tst_QFactoryLoader)
 #include "tst_qfactoryloader.moc"

tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp

@@ -278,7 +278,9 @@ void tst_QPluginLoader::errorString()
     QVERIFY(!unloaded);
     }
 
-#if !defined(Q_OS_WIN) && !defined(Q_OS_DARWIN) && !defined(Q_OS_HPUX)
+// A bug in QNX causes the test to crash on exit after attempting to load
+// a shared library with undefined symbols (tracked as QTBUG-114682).
+#if !defined(Q_OS_WIN) && !defined(Q_OS_DARWIN) && !defined(Q_OS_HPUX) && !defined(Q_OS_QNX)
     {
     QPluginLoader loader( sys_qualifiedLibraryName("almostplugin"));     //a plugin with unresolved symbols
     loader.setLoadHints(QLibrary::ResolveAllSymbolsHint);
@@ -338,16 +340,34 @@ void tst_QPluginLoader::loadHints()
     QCOMPARE(loader.loadHints(), QLibrary::PreventUnloadHint);   //Do not crash
     loader.setLoadHints(QLibrary::ResolveAllSymbolsHint);
     QCOMPARE(loader.loadHints(), QLibrary::ResolveAllSymbolsHint);
+    // We can clear load hints when file name is not set.
+    loader.setLoadHints(QLibrary::LoadHints{});
+    QCOMPARE(loader.loadHints(), QLibrary::LoadHints{});
+    // Set the hints again
+    loader.setLoadHints(QLibrary::ResolveAllSymbolsHint);
+    QCOMPARE(loader.loadHints(), QLibrary::ResolveAllSymbolsHint);
     loader.setFileName( sys_qualifiedLibraryName("theplugin"));     //a plugin
     QCOMPARE(loader.loadHints(), QLibrary::ResolveAllSymbolsHint);
 
+    QPluginLoader loader4;
+    QCOMPARE(loader4.loadHints(), QLibrary::PreventUnloadHint);
+    loader4.setLoadHints(QLibrary::LoadHints{});
+    QCOMPARE(loader4.loadHints(), QLibrary::LoadHints{});
+    loader4.setFileName(sys_qualifiedLibraryName("theplugin"));
+    // Hints are merged with hints from the previous loader.
+    QCOMPARE(loader4.loadHints(), QLibrary::ResolveAllSymbolsHint);
+    // We cannot clear load hints after associating the loader with a file.
+    loader.setLoadHints(QLibrary::LoadHints{});
+    QCOMPARE(loader.loadHints(), QLibrary::ResolveAllSymbolsHint);
+
     QPluginLoader loader2;
     QCOMPARE(loader2.loadHints(), QLibrary::PreventUnloadHint);
     loader2.setFileName(sys_qualifiedLibraryName("theplugin"));
-    QCOMPARE(loader2.loadHints(), QLibrary::PreventUnloadHint);
+    // Hints are merged with hints from previous loaders.
+    QCOMPARE(loader2.loadHints(), QLibrary::PreventUnloadHint | QLibrary::ResolveAllSymbolsHint);
 
     QPluginLoader loader3(sys_qualifiedLibraryName("theplugin"));
-    QCOMPARE(loader3.loadHints(), QLibrary::PreventUnloadHint);
+    QCOMPARE(loader3.loadHints(), QLibrary::PreventUnloadHint | QLibrary::ResolveAllSymbolsHint);
 }
 
 void tst_QPluginLoader::deleteinstanceOnUnload()