Fix QML context leak with visual data model and list property models

When using the VDM or QML list properties as models, the delegate model
injects an intermediate QQmlContext that provides access to the
properties of the exposed QObject as context properties. Before commit
e22b624d9ab1f36021adb9cdbfa9b37054282bb8, that context was marked to be
owned by the parent QQmlContext.

When the reference counting was introduced, that parent became
referenced from the cacheItem (DelegateModelItem), but that intermediate
QQmlContext became floating and was leaked.

This can be observed by running the objectListModel test of
tst_qquickvisualdatamodel with detect_leaks=1 in ASAN_OPTIONS.

The leak is fixed by re-introducing the exceptional case of a parent
holding a strong reference to the child, in just this one case.

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

3 files changed, 14 insertions, 6 deletions

diff --git a/src/qml/qml/qqmlcontext.cpp b/src/qml/qml/qqmlcontext.cpp
index 5dd3278b4c..3dcfa92416 100644
--- a/src/qml/qml/qqmlcontext.cpp
+++ b/src/qml/qml/qqmlcontext.cpp
@@ -536,7 +536,7 @@ QQmlContextData::QQmlContextData()
 QQmlContextData::QQmlContextData(QQmlContext *ctxt)
     : engine(nullptr), isInternal(false), isJSContext(false),
       isPragmaLibraryContext(false), unresolvedNames(false), hasEmittedDestruction(false), isRootObjectInCreation(false),
-      publicContext(ctxt), incubator(nullptr), componentObjectIndex(-1),
+      stronglyReferencedByParent(false), publicContext(ctxt), incubator(nullptr), componentObjectIndex(-1),
       contextObject(nullptr), nextChild(nullptr), prevChild(nullptr),
       expressions(nullptr), contextObjects(nullptr), idValues(nullptr), idValueCount(0),
       componentAttached(nullptr)
@@ -577,7 +577,10 @@ void QQmlContextData::invalidate()
 
     while (childContexts) {
         Q_ASSERT(childContexts != this);
-        childContexts->invalidate();
+        if (childContexts->stronglyReferencedByParent && !--childContexts->refCount)
+            childContexts->destroy();
+        else
+            childContexts->invalidate();
     }
 
     if (prevChild) {
@@ -672,12 +675,16 @@ void QQmlContextData::destroy()
     delete this;
 }
 
-void QQmlContextData::setParent(QQmlContextData *p)
+void QQmlContextData::setParent(QQmlContextData *p, bool stronglyReferencedByParent)
 {
     if (p == parent)
         return;
     if (p) {
+        Q_ASSERT(!parent);
         parent = p;
+        this->stronglyReferencedByParent = stronglyReferencedByParent;
+        if (stronglyReferencedByParent)
+            ++refCount; // balanced in QQmlContextData::invalidate()
         engine = p->engine;
         nextChild = p->childContexts;
         if (nextChild) nextChild->prevChild = &nextChild;
diff --git a/src/qml/qml/qqmlcontext_p.h b/src/qml/qml/qqmlcontext_p.h
index 5dfee48848..290b7fc7ee 100644
--- a/src/qml/qml/qqmlcontext_p.h
+++ b/src/qml/qml/qqmlcontext_p.h
@@ -126,7 +126,7 @@ public:
     QQmlContextData *parent = nullptr;
     QQmlEngine *engine;
 
-    void setParent(QQmlContextData *);
+    void setParent(QQmlContextData *, bool stronglyReferencedByParent = false);
     void refreshExpressions();
 
     void addObject(QObject *);
@@ -144,7 +144,8 @@ public:
     quint32 unresolvedNames:1; // True if expressions in this context failed to resolve a toplevel name
     quint32 hasEmittedDestruction:1;
     quint32 isRootObjectInCreation:1;
-    quint32 dummy:26;
+    quint32 stronglyReferencedByParent:1;
+    quint32 dummy:25;
     QQmlContext *publicContext;
 
     // The incubator that is constructing this context if any
diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index 26c20d043f..eb02e8045a 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -989,7 +989,7 @@ QObject *QQmlDelegateModelPrivate::object(Compositor::Group group, int index, QQ
             if (QQmlAdaptorModelProxyInterface *proxy
                     = qobject_cast<QQmlAdaptorModelProxyInterface *>(cacheItem)) {
                 ctxt = new QQmlContextData;
-                ctxt->setParent(cacheItem->contextData);
+                ctxt->setParent(cacheItem->contextData, /*stronglyReferencedByParent*/true);
                 ctxt->contextObject = proxy->proxiedObject();
             }
         }