From 65d1ac702fccf91d496d0eb36fb33378e4035a51 Mon Sep 17 00:00:00 2001
From: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Date: Tue, 13 Jun 2023 12:52:28 +0200
Subject: [PATCH 134/147] 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>
(cherry picked from commit 666ce51d4eb6b5dd312f98e2d7a18c54b59945e4)
---
 src/corelib/plugin/qlibrary.cpp               | 13 ++++++++-
 src/corelib/plugin/qpluginloader.cpp          |  5 ++--
 .../plugin/qfactoryloader/plugin1/plugin1.h   |  2 +-
 .../qfactoryloader/plugin1/plugin1.json       |  5 ++++
 .../qfactoryloader/tst_qfactoryloader.cpp     | 27 +++++++++++++++++++
 .../qpluginloader/tst_qpluginloader.cpp       | 26 +++++++++++++++---
 6 files changed, 71 insertions(+), 7 deletions(-)
 create mode 100644 tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.json

diff --git a/src/corelib/plugin/qlibrary.cpp b/src/corelib/plugin/qlibrary.cpp
index 5d2f024267..45b5a3fe27 100644
--- a/src/corelib/plugin/qlibrary.cpp
+++ b/src/corelib/plugin/qlibrary.cpp
@@ -526,7 +526,7 @@ void QLibraryPrivate::mergeLoadHints(QLibrary::LoadHints lh)
     if (pHnd.loadRelaxed())
         return;
 
-    loadHintsInt.storeRelaxed(lh);
+    loadHintsInt.fetchAndOrRelaxed(lh);
 }
 
 QFunctionPointer QLibraryPrivate::resolve(const char *symbol)
@@ -538,6 +538,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);
+        return;
+    }
+
     // this locks a global mutex
     QMutexLocker lock(&qt_library_mutex);
     mergeLoadHints(lh);
@@ -1166,6 +1173,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.
 
diff --git a/src/corelib/plugin/qpluginloader.cpp b/src/corelib/plugin/qpluginloader.cpp
index de429d5b47..02b8c588be 100644
--- a/src/corelib/plugin/qpluginloader.cpp
+++ b/src/corelib/plugin/qpluginloader.cpp
@@ -416,10 +416,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
diff --git a/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.h b/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.h
index ca2ceed7a9..624316dfdb 100644
--- a/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.h
+++ b/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.h
@@ -35,7 +35,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:
diff --git a/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.json b/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.json
new file mode 100644
index 0000000000..ce67846d48
--- /dev/null
+++ b/tests/auto/corelib/plugin/qfactoryloader/plugin1/plugin1.json
@@ -0,0 +1,5 @@
+{
+    "Keys": [
+        "plugin1"
+    ]
+}
diff --git a/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp b/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp
index 9fa61804b3..88ada1b806 100644
--- a/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp
+++ b/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp
@@ -31,6 +31,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"
 
@@ -52,6 +53,7 @@ public slots:
 
 private slots:
     void usingTwoFactoriesFromSameDir();
+    void multiplePaths();
 };
 
 static const char binFolderC[] = "bin";
@@ -92,5 +94,30 @@ void tst_QFactoryLoader::usingTwoFactoriesFromSameDir()
     QCOMPARE(plugin2->pluginName(), QLatin1String("Plugin2 ok"));
 }
 
+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
+    const QString binFolder = QFINDTESTDATA(binFolderC);
+    
+    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"
diff --git a/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp b/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
index 3b91fc7d0d..833f68b1de 100644
--- a/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
+++ b/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
@@ -193,7 +193,9 @@ void tst_QPluginLoader::errorString()
     QVERIFY(!unloaded);
     }
 
-#if !defined(Q_OS_WIN) && !defined(Q_OS_MAC) && !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_MAC) && !defined(Q_OS_HPUX) && !defined(Q_OS_QNX)
     {
     QPluginLoader loader( sys_qualifiedLibraryName("almostplugin"));     //a plugin with unresolved symbols
     loader.setLoadHints(QLibrary::ResolveAllSymbolsHint);
@@ -246,16 +248,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()
-- 
2.40.1