Fix qmlClearTypeRegistrations() not dropping all registrations

In commit 48c09a85ce397979c7e706e3694c879ffe456e09 we added the
undeletableTypes container to hold a reference on C++ registered types
to keep the indices returned by the public qmlRegisterType() API stable.

Since qmlClearTypeRegistrations() is API that also resets those indices,
we must also clear the undeletableTypes container to avoid leaking
memory.

Change-Id: I2038c00913f894d58aca3714d64d497493585326
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 61 insertions, 9 deletions

diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp
index 8f5d11a96f..ac670bdabb 100644
--- a/src/qml/qml/qqmlmetatype.cpp
+++ b/src/qml/qml/qqmlmetatype.cpp
@@ -1144,6 +1144,13 @@ void QQmlType::derefHandle(QQmlTypePrivate *priv)
         delete priv;
 }
 
+int QQmlType::refCount(QQmlTypePrivate *priv)
+{
+    if (priv)
+        return priv->refCount;
+    return -1;
+}
+
 namespace {
 template <typename QQmlTypeContainer>
 void removeQQmlTypePrivate(QQmlTypeContainer &container, const QQmlTypePrivate *reference)
@@ -1334,6 +1341,7 @@ void qmlClearTypeRegistrations() // Declared in qqml.h
     data->urlToNonFileImportType.clear();
     data->metaObjectToType.clear();
     data->uriToModule.clear();
+    data->undeletableTypes.clear();
 
     QQmlEnginePrivate::baseModulesUninitialized = true; //So the engine re-registers its types
 #if QT_CONFIG(library)
diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h
index 9a7736ffcd..74b1cf0e06 100644
--- a/src/qml/qml/qqmlmetatype_p.h
+++ b/src/qml/qml/qqmlmetatype_p.h
@@ -245,6 +245,7 @@ public:
     QQmlTypePrivate *priv() const { return d; }
     static void refHandle(QQmlTypePrivate *priv);
     static void derefHandle(QQmlTypePrivate *priv);
+    static int refCount(QQmlTypePrivate *priv);
 
     enum RegistrationType {
         CppType = 0,
diff --git a/tests/auto/qml/qqmlenginecleanup/qqmlenginecleanup.pro b/tests/auto/qml/qqmlenginecleanup/qqmlenginecleanup.pro
index 5bcec9f5b4..90508609a8 100644
--- a/tests/auto/qml/qqmlenginecleanup/qqmlenginecleanup.pro
+++ b/tests/auto/qml/qqmlenginecleanup/qqmlenginecleanup.pro
@@ -6,4 +6,4 @@ include (../../shared/util.pri)
 
 SOURCES += tst_qqmlenginecleanup.cpp
 
-QT += testlib qml
+QT += testlib qml qml-private
diff --git a/tests/auto/qml/qqmlenginecleanup/tst_qqmlenginecleanup.cpp b/tests/auto/qml/qqmlenginecleanup/tst_qqmlenginecleanup.cpp
index d0a8b6401f..7e9a1524b0 100644
--- a/tests/auto/qml/qqmlenginecleanup/tst_qqmlenginecleanup.cpp
+++ b/tests/auto/qml/qqmlenginecleanup/tst_qqmlenginecleanup.cpp
@@ -31,6 +31,8 @@
 #include <QtQml/qqml.h>
 #include <QtQml/QQmlEngine>
 #include <QtQml/QQmlComponent>
+#include <private/qhashedstring_p.h>
+#include <private/qqmlmetatype_p.h>
 
 //Separate test, because if engine cleanup attempts fail they can easily break unrelated tests
 class tst_qqmlenginecleanup : public QQmlDataTest
@@ -44,41 +46,82 @@ private slots:
     void test_valueTypeProviderModule(); // QTBUG-43004
 };
 
+// A wrapper around QQmlComponent to ensure the temporary reference counts
+// on the type data as a result of the main thread <> loader thread communication
+// are dropped. Regular Synchronous loading will leave us with an event posted
+// to the gui thread and an extra refcount that will only be dropped after the
+// event delivery. A plain sendPostedEvents() however is insufficient because
+// we can't be sure that the event is posted after the constructor finished.
+class CleanlyLoadingComponent : public QQmlComponent
+{
+public:
+    CleanlyLoadingComponent(QQmlEngine *engine, const QUrl &url)
+        : QQmlComponent(engine, url, QQmlComponent::Asynchronous)
+    { waitForLoad(); }
+    CleanlyLoadingComponent(QQmlEngine *engine, const QString &fileName)
+        : QQmlComponent(engine, fileName, QQmlComponent::Asynchronous)
+    { waitForLoad(); }
+
+    void waitForLoad()
+    {
+        QTRY_VERIFY(status() == QQmlComponent::Ready || status() == QQmlComponent::Error);
+    }
+};
+
 void tst_qqmlenginecleanup::test_qmlClearTypeRegistrations()
 {
     //Test for preventing memory leaks is in tests/manual/qmltypememory
     QQmlEngine* engine;
-    QQmlComponent* component;
+    CleanlyLoadingComponent* component;
     QUrl testFile = testFileUrl("types.qml");
 
+    const auto qmlTypeForTestType = []() {
+        return QQmlMetaType::qmlType(QStringLiteral("TestTypeCpp"), QStringLiteral("Test"), 2, 0);
+    };
+
+    QVERIFY(!qmlTypeForTestType().isValid());
     qmlRegisterType<QObject>("Test", 2, 0, "TestTypeCpp");
+    QVERIFY(qmlTypeForTestType().isValid());
+
     engine = new QQmlEngine;
-    component = new QQmlComponent(engine, testFile);
+    component = new CleanlyLoadingComponent(engine, testFile);
     QVERIFY(component->isReady());
 
-    delete engine;
     delete component;
-    qmlClearTypeRegistrations();
+    delete engine;
+
+    {
+        auto cppType = qmlTypeForTestType();
+
+        qmlClearTypeRegistrations();
+        QVERIFY(!qmlTypeForTestType().isValid());
+
+        // cppType should hold the last ref, qmlClearTypeRegistration should have wiped
+        // all internal references.
+        QCOMPARE(QQmlType::refCount(cppType.priv()), 1);
+    }
 
     //2nd run verifies that types can reload after a qmlClearTypeRegistrations
     qmlRegisterType<QObject>("Test", 2, 0, "TestTypeCpp");
+    QVERIFY(qmlTypeForTestType().isValid());
     engine = new QQmlEngine;
-    component = new QQmlComponent(engine, testFile);
+    component = new CleanlyLoadingComponent(engine, testFile);
     QVERIFY(component->isReady());
 
-    delete engine;
     delete component;
+    delete engine;
     qmlClearTypeRegistrations();
+    QVERIFY(!qmlTypeForTestType().isValid());
 
     //3nd run verifies that TestTypeCpp is no longer registered
     engine = new QQmlEngine;
-    component = new QQmlComponent(engine, testFile);
+    component = new CleanlyLoadingComponent(engine, testFile);
     QVERIFY(component->isError());
     QCOMPARE(component->errorString(),
             testFile.toString() +":33 module \"Test\" is not installed\n");
 
-    delete engine;
     delete component;
+    delete engine;
 }
 
 static void cleanState(QQmlEngine **e)