QML: Avoid cyclic references between ResolvedTypeRefence and CU

The compilation unit keeps a list of references to used types. Usually
those are references to types in other compilation units. However, in
the case of inline components they can be references to the same
compilation unit. In that case we should not addref() the compilation
unit, as that forms a cyclic reference.

Fixes: QTBUG-82768
Change-Id: I29d28f3a4780aad4fabd4aa2ed02ca0d0108987e
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

7 files changed, 105 insertions, 51 deletions

diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp
index fa4a1f1ce4..29e1c6dbf7 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit.cpp
+++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp
@@ -406,9 +406,9 @@ void ExecutableCompilationUnit::finalizeCompositeType(QQmlEnginePrivate *qmlEngi
         const QV4::CompiledData::Object *obj = objectAt(/*root object*/0);
         auto *typeRef = resolvedTypes.value(obj->inheritedTypeNameIndex);
         Q_ASSERT(typeRef);
-        if (typeRef->compilationUnit) {
-            metaTypeId = typeRef->compilationUnit->metaTypeId;
-            listMetaTypeId = typeRef->compilationUnit->listMetaTypeId;
+        if (const auto compilationUnit = typeRef->compilationUnit()) {
+            metaTypeId = compilationUnit->metaTypeId;
+            listMetaTypeId = compilationUnit->listMetaTypeId;
         } else {
             metaTypeId = typeRef->type.typeId();
             listMetaTypeId = typeRef->type.qListTypeId();
@@ -453,17 +453,17 @@ void ExecutableCompilationUnit::finalizeCompositeType(QQmlEnginePrivate *qmlEngi
                     ++inlineComponentData[lastICRoot].totalParserStatusCount;
 
                 ++inlineComponentData[lastICRoot].totalObjectCount;
-                if (typeRef->compilationUnit) {
+                if (const auto compilationUnit = typeRef->compilationUnit()) {
                     // if the type is an inline component type, we have to extract the information from it
                     // This requires that inline components are visited in the correct order
-                    auto icRoot = typeRef->compilationUnit->icRoot;
+                    auto icRoot = compilationUnit->icRoot;
                     if (typeRef->type.isInlineComponentType()) {
                         icRoot = typeRef->type.inlineComponendId();
                     }
-                    QScopedValueRollback<int> rollback {typeRef->compilationUnit->icRoot, icRoot};
-                    inlineComponentData[lastICRoot].totalBindingCount += typeRef->compilationUnit->totalBindingsCount();
-                    inlineComponentData[lastICRoot].totalParserStatusCount += typeRef->compilationUnit->totalParserStatusCount();
-                    inlineComponentData[lastICRoot].totalObjectCount += typeRef->compilationUnit->totalObjectCount();
+                    QScopedValueRollback<int> rollback {compilationUnit->icRoot, icRoot};
+                    inlineComponentData[lastICRoot].totalBindingCount += compilationUnit->totalBindingsCount();
+                    inlineComponentData[lastICRoot].totalParserStatusCount += compilationUnit->totalParserStatusCount();
+                    inlineComponentData[lastICRoot].totalObjectCount += compilationUnit->totalObjectCount();
                 }
             }
         }
@@ -481,15 +481,15 @@ void ExecutableCompilationUnit::finalizeCompositeType(QQmlEnginePrivate *qmlEngi
             if (typeRef->type.isValid() && typeRef->type.parserStatusCast() != -1)
                 ++parserStatusCount;
             ++objectCount;
-            if (typeRef->compilationUnit) {
-                auto icRoot = typeRef->compilationUnit->icRoot;
+            if (const auto compilationUnit = typeRef->compilationUnit()) {
+                auto icRoot = compilationUnit->icRoot;
                 if (typeRef->type.isInlineComponentType()) {
                     icRoot = typeRef->type.inlineComponendId();
                 }
-                QScopedValueRollback<int> rollback {typeRef->compilationUnit->icRoot, icRoot};
-                bindingCount += typeRef->compilationUnit->totalBindingsCount();
-                parserStatusCount += typeRef->compilationUnit->totalParserStatusCount();
-                objectCount += typeRef->compilationUnit->totalObjectCount();
+                QScopedValueRollback<int> rollback {compilationUnit->icRoot, icRoot};
+                bindingCount += compilationUnit->totalBindingsCount();
+                parserStatusCount += compilationUnit->totalParserStatusCount();
+                objectCount += compilationUnit->totalObjectCount();
             }
         }
     }
@@ -806,7 +806,7 @@ QQmlRefPointer<QQmlPropertyCache> ResolvedTypeReference::propertyCache() const
     if (type.isValid())
         return typePropertyCache;
     else
-        return compilationUnit->rootPropertyCache();
+        return m_compilationUnit->rootPropertyCache();
 }
 
 /*!
@@ -820,8 +820,8 @@ QQmlRefPointer<QQmlPropertyCache> ResolvedTypeReference::createPropertyCache(QQm
         typePropertyCache = QQmlEnginePrivate::get(engine)->cache(type.metaObject(), minorVersion);
         return typePropertyCache;
     } else {
-        Q_ASSERT(compilationUnit);
-        return compilationUnit->rootPropertyCache();
+        Q_ASSERT(m_compilationUnit);
+        return m_compilationUnit->rootPropertyCache();
     }
 }
 
@@ -832,10 +832,10 @@ bool ResolvedTypeReference::addToHash(QCryptographicHash *hash, QQmlEngine *engi
         hash->addData(createPropertyCache(engine)->checksum(&ok));
         return ok;
     }
-    if (!compilationUnit)
+    if (!m_compilationUnit)
         return false;
-    hash->addData(compilationUnit->data->md5Checksum,
-                  sizeof(compilationUnit->data->md5Checksum));
+    hash->addData(m_compilationUnit->data->md5Checksum,
+                  sizeof(m_compilationUnit->data->md5Checksum));
     return true;
 }
 
@@ -856,8 +856,8 @@ void ResolvedTypeReference::doDynamicTypeCheck()
         mo = typePropertyCache->firstCppMetaObject();
     else if (type.isValid())
         mo = type.metaObject();
-    else if (compilationUnit)
-        mo = compilationUnit->rootPropertyCache()->firstCppMetaObject();
+    else if (m_compilationUnit)
+        mo = m_compilationUnit->rootPropertyCache()->firstCppMetaObject();
     isFullyDynamicType = qtTypeInherits<QQmlPropertyMap>(mo);
 }
 
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 8cad18a3dc..7bfb68da9d 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -325,27 +325,73 @@ private:
 
 struct ResolvedTypeReference
 {
+public:
     ResolvedTypeReference()
-        : majorVersion(0)
-          , minorVersion(0)
-          , isFullyDynamicType(false)
+        : m_compilationUnit(nullptr)
+        , m_stronglyReferencesCompilationUnit(true)
+        , majorVersion(0)
+        , minorVersion(0)
+        , isFullyDynamicType(false)
     {}
 
+    ~ResolvedTypeReference()
+    {
+        if (m_stronglyReferencesCompilationUnit && m_compilationUnit)
+            m_compilationUnit->release();
+    }
+
+    QQmlRefPointer<QV4::ExecutableCompilationUnit> compilationUnit() { return m_compilationUnit; }
+    void setCompilationUnit(QQmlRefPointer<QV4::ExecutableCompilationUnit> unit)
+    {
+        if (m_compilationUnit == unit.data())
+            return;
+        if (m_stronglyReferencesCompilationUnit) {
+            if (m_compilationUnit)
+                m_compilationUnit->release();
+            m_compilationUnit = unit.take();
+        } else {
+            m_compilationUnit = unit.data();
+        }
+    }
+
+    bool referencesCompilationUnit() const { return m_stronglyReferencesCompilationUnit; }
+    void setReferencesCompilationUnit(bool doReference)
+    {
+        if (doReference == m_stronglyReferencesCompilationUnit)
+            return;
+        m_stronglyReferencesCompilationUnit = doReference;
+        if (!m_compilationUnit)
+            return;
+        if (doReference) {
+            m_compilationUnit->addref();
+        } else if (m_compilationUnit->count() == 1) {
+            m_compilationUnit->release();
+            m_compilationUnit = nullptr;
+        } else {
+            m_compilationUnit->release();
+        }
+    }
+
+    QQmlRefPointer<QQmlPropertyCache> propertyCache() const;
+    QQmlRefPointer<QQmlPropertyCache> createPropertyCache(QQmlEngine *);
+    bool addToHash(QCryptographicHash *hash, QQmlEngine *engine);
+
+    void doDynamicTypeCheck();
+
     QQmlType type;
     QQmlRefPointer<QQmlPropertyCache> typePropertyCache;
-    QQmlRefPointer<QV4::ExecutableCompilationUnit> compilationUnit;
+private:
+    Q_DISABLE_COPY_MOVE(ResolvedTypeReference)
+
+    QV4::ExecutableCompilationUnit *m_compilationUnit;
+    bool m_stronglyReferencesCompilationUnit;
 
+public:
     int majorVersion;
     int minorVersion;
     // Types such as QQmlPropertyMap can add properties dynamically at run-time and
     // therefore cannot have a property cache installed when instantiated.
     bool isFullyDynamicType;
-
-    QQmlRefPointer<QQmlPropertyCache> propertyCache() const;
-    QQmlRefPointer<QQmlPropertyCache> createPropertyCache(QQmlEngine *);
-    bool addToHash(QCryptographicHash *hash, QQmlEngine *engine);
-
-    void doDynamicTypeCheck();
 };
 
 IdentifierHash ExecutableCompilationUnit::namedObjectsPerComponent(int componentObjectIndex)
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 76f387bbdc..3ca89924f9 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -1214,11 +1214,12 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
 
             sharedState->allCreatedObjects.push(instance);
         } else {
-            Q_ASSERT(typeRef->compilationUnit);
-            typeName = typeRef->compilationUnit->fileName();
+            const auto compilationUnit = typeRef->compilationUnit();
+            Q_ASSERT(compilationUnit);
+            typeName = compilationUnit->fileName();
             // compilation unit is shared between root type and its inline component types
             // so isSingleton errorneously returns true for inline components
-            if (typeRef->compilationUnit->unitData()->isSingleton() && !type.isInlineComponentType())
+            if (compilationUnit->unitData()->isSingleton() && !type.isInlineComponentType())
             {
                 recordError(obj->location, tr("Composite Singleton Type %1 is not creatable").arg(stringAt(obj->inheritedTypeNameIndex)));
                 return nullptr;
@@ -1226,7 +1227,7 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
 
 
             if (!type.isInlineComponentType()) {
-                QQmlObjectCreator subCreator(context, typeRef->compilationUnit, sharedState.data());
+                QQmlObjectCreator subCreator(context, compilationUnit, sharedState.data());
                 instance = subCreator.create();
                 if (!instance) {
                     errors += subCreator.errors;
@@ -1234,8 +1235,8 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
                 }
             } else {
                 int subObjectId = type.inlineComponendId();
-                QScopedValueRollback<int> rollback {typeRef->compilationUnit->icRoot, subObjectId};
-                QQmlObjectCreator subCreator(context, typeRef->compilationUnit, sharedState.data());
+                QScopedValueRollback<int> rollback {compilationUnit->icRoot, subObjectId};
+                QQmlObjectCreator subCreator(context, compilationUnit, sharedState.data());
                 instance = subCreator.create(subObjectId, nullptr, nullptr, CreationFlags::InlineComponent);
                 if (!instance) {
                     errors += subCreator.errors;
diff --git a/src/qml/qml/qqmlpropertycachecreator_p.h b/src/qml/qml/qqmlpropertycachecreator_p.h
index 5669f9e13d..6b02d6fb98 100644
--- a/src/qml/qml/qqmlpropertycachecreator_p.h
+++ b/src/qml/qml/qqmlpropertycachecreator_p.h
@@ -840,7 +840,7 @@ inline QQmlError QQmlPropertyCacheAliasCreator<ObjectContainer>::propertyDataFor
         if (typeRef->type.isValid())
             *type = typeRef->type.typeId();
         else
-            *type = typeRef->compilationUnit->metaTypeId;
+            *type = typeRef->compilationUnit()->metaTypeId;
 
         *minorVersion = typeRef->minorVersion;
 
diff --git a/src/qml/qml/qqmltypecompiler.cpp b/src/qml/qml/qqmltypecompiler.cpp
index f4737ba99d..058bf8848c 100644
--- a/src/qml/qml/qqmltypecompiler.cpp
+++ b/src/qml/qml/qqmltypecompiler.cpp
@@ -813,8 +813,8 @@ void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(const QmlI
         const QMetaObject *firstMetaObject = nullptr;
         if (tr->type.isValid())
             firstMetaObject = tr->type.metaObject();
-        else if (tr->compilationUnit)
-            firstMetaObject = tr->compilationUnit->rootPropertyCache()->firstCppMetaObject();
+        else if (const auto compilationUnit = tr->compilationUnit())
+            firstMetaObject = compilationUnit->rootPropertyCache()->firstCppMetaObject();
         if (isUsableComponent(firstMetaObject))
             continue;
         // if here, not a QQmlComponent, so needs wrapping
diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp
index cd4e0acf39..fc1d0cfbcf 100644
--- a/src/qml/qml/qqmltypedata.cpp
+++ b/src/qml/qml/qqmltypedata.cpp
@@ -300,7 +300,14 @@ void QQmlTypeData::setCompileUnit(const Container &container)
         auto const root = container->objectAt(i);
         for (auto it = root->inlineComponentsBegin(); it != root->inlineComponentsEnd(); ++it) {
             auto *typeRef = m_compiledData->resolvedType(it->nameIndex);
-            typeRef->compilationUnit = m_compiledData; // share compilation unit
+
+            // We don't want the type reference to keep a strong reference to the compilation unit
+            // here. The compilation unit owns the type reference, and having a strong reference
+            // would prevent the compilation unit from ever getting deleted. We can still be sure
+            // that the compilation unit outlives the type reference, due to ownership.
+            typeRef->setReferencesCompilationUnit(false);
+
+            typeRef->setCompilationUnit(m_compiledData); // share compilation unit
         }
     }
 }
@@ -919,7 +926,7 @@ QQmlError QQmlTypeData::buildTypeResolutionCaches(
             if (resolvedType->needsCreation && qmlType.isCompositeSingleton()) {
                 return qQmlCompileError(resolvedType->location, tr("Composite Singleton Type %1 is not creatable.").arg(qmlType.qmlTypeName()));
             }
-            ref->compilationUnit = resolvedType->typeData->compilationUnit();
+            ref->setCompilationUnit(resolvedType->typeData->compilationUnit());
             if (resolvedType->type.isInlineComponentType()) {
                 // Inline component which is part of an already resolved type
                 int objectId = -1;
@@ -943,12 +950,12 @@ QQmlError QQmlTypeData::buildTypeResolutionCaches(
                     auto typeID = qmlType.lookupInlineComponentById(qmlType.inlineComponendId()).typeId();
                     auto exUnit = engine->obtainExecutableCompilationUnit(typeID);
                     if (exUnit) {
-                        ref->compilationUnit = exUnit;
+                        ref->setCompilationUnit(exUnit);
                         ref->typePropertyCache = engine->propertyCacheForType(typeID);
                     }
                 }
             } else {
-                ref->compilationUnit = m_inlineComponentToCompiledData[resolvedType.key()];
+                ref->setCompilationUnit(m_inlineComponentToCompiledData[resolvedType.key()]);
                 ref->typePropertyCache = m_inlineComponentToCompiledData[resolvedType.key()]->rootPropertyCache();
             }
 
diff --git a/src/qmltest/quicktest.cpp b/src/qmltest/quicktest.cpp
index 470b3c0f7a..443496c9bc 100644
--- a/src/qmltest/quicktest.cpp
+++ b/src/qmltest/quicktest.cpp
@@ -332,8 +332,9 @@ private:
         }
     };
 
-    TestCaseEnumerationResult enumerateTestCases(QV4::ExecutableCompilationUnit *compilationUnit,
-                                                 const Object *object = nullptr)
+    TestCaseEnumerationResult enumerateTestCases(
+            const QQmlRefPointer<QV4::ExecutableCompilationUnit> &compilationUnit,
+            const Object *object = nullptr)
     {
         QQmlType testCaseType;
         for (quint32 i = 0, count = compilationUnit->importCount(); i < count; ++i) {
@@ -356,9 +357,8 @@ private:
         if (!object) // Start at root of compilation unit if not enumerating a specific child
             object = compilationUnit->objectAt(0);
 
-        if (QV4::ExecutableCompilationUnit *superTypeUnit
-            = compilationUnit->resolvedTypes.value(object->inheritedTypeNameIndex)
-                      ->compilationUnit.data()) {
+        if (const auto superTypeUnit = compilationUnit->resolvedTypes.value(
+                    object->inheritedTypeNameIndex)->compilationUnit()) {
             // We have a non-C++ super type, which could indicate we're a subtype of a TestCase
             if (testCaseType.isValid() && superTypeUnit->url() == testCaseType.sourceUrl())
                 result.isTestCase = true;