QML TypeLoader: Sort composite singletons before recursing into them

When recursingly loading further types for composite singletons before
sorting them, the order in which the recursively referenced types are
loaded is random because the composite singletons are kept in an
(unordered) hash. Any sorting after loading the child components doesn't
help as the recursive references may depend on the types already loaded
at that point.

Sorting the composite singletons before starting the recursion does help
because it eliminates the source of randomness in the system.

Fixes: QTBUG-66976
Change-Id: I0fa1f50b36eba8c73eb8d56b4d5118485ab05f35
Reviewed-by: Michael Brasser <michael.brasser@live.com>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

11 files changed, 119 insertions, 4 deletions

diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp
index bc53b98b5b..ab63489dbf 100644
--- a/src/qml/qml/qqmlimport.cpp
+++ b/src/qml/qml/qqmlimport.cpp
@@ -511,6 +511,20 @@ QList<QQmlImports::CompositeSingletonReference> QQmlImports::resolvedCompositeSi
         findCompositeSingletons(set, compositeSingletons, baseUrl());
     }
 
+    std::stable_sort(compositeSingletons.begin(), compositeSingletons.end(),
+                     [](const QQmlImports::CompositeSingletonReference &lhs,
+                        const QQmlImports::CompositeSingletonReference &rhs) {
+        if (lhs.prefix != rhs.prefix)
+            return lhs.prefix < rhs.prefix;
+
+        if (lhs.typeName != rhs.typeName)
+            return lhs.typeName < rhs.typeName;
+
+        return lhs.majorVersion != rhs.majorVersion
+            ? lhs.majorVersion < rhs.majorVersion
+            : lhs.minorVersion < rhs.minorVersion;
+    });
+
     return compositeSingletons;
 }
 
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 7480475ca7..cb90af4cf0 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -2723,10 +2723,6 @@ void QQmlTypeData::resolveTypes()
         }
     }
 
-    std::stable_sort(m_compositeSingletons.begin(), m_compositeSingletons.end(), [](const TypeReference &lhs, const TypeReference &rhs){
-        return lhs.qualifiedName() < rhs.qualifiedName();
-    });
-
     for (QV4::CompiledData::TypeReferenceMap::ConstIterator unresolvedRef = m_typeReferences.constBegin(), end = m_typeReferences.constEnd();
          unresolvedRef != end; ++unresolvedRef) {
 
diff --git a/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton1.qml b/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton1.qml
new file mode 100644
index 0000000000..f4ad5e5f7a
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton1.qml
@@ -0,0 +1,6 @@
+pragma Singleton
+import QtQuick 2.0
+
+Item {
+    property bool ok: true
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton2.qml b/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton2.qml
new file mode 100644
index 0000000000..55dd57517f
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/CppRegisteredSingleton2.qml
@@ -0,0 +1,7 @@
+pragma Singleton
+import QtQuick 2.0
+import cppsingletonmodule 1.0
+
+Item {
+    property bool ok: CppRegisteredSingleton1.ok
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/Singleton.qml b/tests/auto/qml/qqmltypeloader/data/Singleton.qml
new file mode 100644
index 0000000000..3a1b1c1493
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/Singleton.qml
@@ -0,0 +1,7 @@
+pragma Singleton
+import QtQml 2.0
+import modulewithsingleton 1.0
+
+QtObject {
+    property bool ok: true
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton1.qml b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton1.qml
new file mode 100644
index 0000000000..34eca59f86
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton1.qml
@@ -0,0 +1,7 @@
+pragma Singleton
+import QtQuick 2.0
+import "."
+
+Item {
+    property bool ok: true
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton2.qml b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton2.qml
new file mode 100644
index 0000000000..607d85d7fb
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/Singleton2.qml
@@ -0,0 +1,7 @@
+pragma Singleton
+import QtQuick 2.0
+import "."
+
+Item {
+    property bool ok: Singleton1.ok
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/qmldir b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/qmldir
new file mode 100644
index 0000000000..71b889a12d
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/imports/multisingletonmodule/qmldir
@@ -0,0 +1,2 @@
+singleton Singleton1 1.0 Singleton1.qml
+singleton Singleton2 1.0 Singleton2.qml
diff --git a/tests/auto/qml/qqmltypeloader/data/multisingletonuser.qml b/tests/auto/qml/qqmltypeloader/data/multisingletonuser.qml
new file mode 100644
index 0000000000..b80e2c5223
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/multisingletonuser.qml
@@ -0,0 +1,7 @@
+import QtQml 2.0
+import multisingletonmodule 1.0
+import cppsingletonmodule 1.0
+
+QtObject {
+    property bool ok: Singleton2.ok && CppRegisteredSingleton2.ok
+}
diff --git a/tests/auto/qml/qqmltypeloader/data/singletonuser.qml b/tests/auto/qml/qqmltypeloader/data/singletonuser.qml
new file mode 100644
index 0000000000..79ca47e12f
--- /dev/null
+++ b/tests/auto/qml/qqmltypeloader/data/singletonuser.qml
@@ -0,0 +1,6 @@
+import QtQml 2.0
+import modulewithsingleton 1.0
+
+QtObject {
+    property bool ok: Singleton.ok
+}
diff --git a/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp b/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp
index cf3bc8b050..3745fad470 100644
--- a/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp
+++ b/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp
@@ -31,6 +31,9 @@
 #include <QtQml/qqmlnetworkaccessmanagerfactory.h>
 #include <QtQuick/qquickview.h>
 #include <QtQuick/qquickitem.h>
+#if QT_CONFIG(process)
+#include <QtCore/qprocess.h>
+#endif
 #include <QtQml/private/qqmlengine_p.h>
 #include <QtQml/private/qqmltypeloader_p.h>
 #include "../../shared/testhttpserver.h"
@@ -50,6 +53,8 @@ private slots:
     void keepRegistrations();
     void intercept();
     void redirect();
+    void qmlSingletonWithinModule();
+    void multiSingletonModule();
 };
 
 void tst_QQMLTypeLoader::testLoadComplete()
@@ -428,6 +433,57 @@ void tst_QQMLTypeLoader::redirect()
     QTRY_COMPARE(object->property("xy").toInt(), 323232);
 }
 
+void tst_QQMLTypeLoader::qmlSingletonWithinModule()
+{
+    qmlClearTypeRegistrations();
+    QQmlEngine engine;
+    qmlRegisterSingletonType(testFileUrl("Singleton.qml"), "modulewithsingleton", 1, 0, "Singleton");
+
+    QQmlComponent component(&engine, testFileUrl("singletonuser.qml"));
+    QCOMPARE(component.status(), QQmlComponent::Ready);
+    QScopedPointer<QObject> obj(component.create());
+    QVERIFY(!obj.isNull());
+    QVERIFY(obj->property("ok").toBool());
+}
+
+void tst_QQMLTypeLoader::multiSingletonModule()
+{
+    qmlClearTypeRegistrations();
+    QQmlEngine engine;
+    engine.addImportPath(testFile("imports"));
+
+    qmlRegisterSingletonType(testFileUrl("CppRegisteredSingleton1.qml"), "cppsingletonmodule",
+                             1, 0, "CppRegisteredSingleton1");
+    qmlRegisterSingletonType(testFileUrl("CppRegisteredSingleton2.qml"), "cppsingletonmodule",
+                             1, 0, "CppRegisteredSingleton2");
+
+    QQmlComponent component(&engine, testFileUrl("multisingletonuser.qml"));
+    QCOMPARE(component.status(), QQmlComponent::Ready);
+    QScopedPointer<QObject> obj(component.create());
+    QVERIFY(!obj.isNull());
+    QVERIFY(obj->property("ok").toBool());
+
+#if QT_CONFIG(process)
+    const char *skipKey = "QT_TST_QQMLTYPELOADER_SKIP_MISMATCH";
+    if (qEnvironmentVariableIsSet(skipKey))
+        return;
+    for (int i = 0; i < 5; ++i) {
+        QProcess child;
+        child.setProgram(QCoreApplication::applicationFilePath());
+        child.setArguments(QStringList(QLatin1String("multiSingletonModule")));
+        QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+        env.insert(QLatin1String("QT_LOGGING_RULES"), QLatin1String("qt.qml.diskcache.debug=true"));
+        env.insert(QLatin1String(skipKey), QLatin1String("1"));
+        child.setProcessEnvironment(env);
+        child.start();
+        QVERIFY(child.waitForFinished());
+        QCOMPARE(child.exitCode(), 0);
+        QVERIFY(!child.readAllStandardOutput().contains("Checksum mismatch for cached version"));
+        QVERIFY(!child.readAllStandardError().contains("Checksum mismatch for cached version"));
+    }
+#endif
+}
+
 QTEST_MAIN(tst_QQMLTypeLoader)
 
 #include "tst_qqmltypeloader.moc"