QtQml: Clear context objects more thoroughly on destruction

The same object can be the context object of a hierarchy of contexts. So
far we would only clear one of them, leaving dangling pointers in the
others. Clear all the contexts.

Pick-to: 6.7 6.6 6.5 6.2 5.15
Fixes: QTBUG-119326
Change-Id: I509f257672813866e3736b51f430f1243a8577f0
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

8 files changed, 77 insertions, 18 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 5463133801..e233b65f4e 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -1514,9 +1514,7 @@ void QObjectWrapper::destroyObject(bool lastCall)
             if (!o->parent() && !ddata->indestructible) {
                 if (ddata && ddata->ownContext) {
                     Q_ASSERT(ddata->ownContext.data() == ddata->context);
-                    ddata->ownContext->emitDestruction();
-                    if (ddata->ownContext->contextObject() == o)
-                        ddata->ownContext->setContextObject(nullptr);
+                    ddata->ownContext->deepClearContextObject(o);
                     ddata->ownContext.reset();
                     ddata->context = nullptr;
                 }
diff --git a/src/qml/qml/qqmlcontextdata_p.h b/src/qml/qml/qqmlcontextdata_p.h
index 71dda623b9..c8e362ec8d 100644
--- a/src/qml/qml/qqmlcontextdata_p.h
+++ b/src/qml/qml/qqmlcontextdata_p.h
@@ -128,6 +128,28 @@ public:
     QObject *contextObject() const { return m_contextObject; }
     void setContextObject(QObject *contextObject) { m_contextObject = contextObject; }
 
+    template<typename HandleSelf, typename HandleLinked>
+    void deepClearContextObject(
+            QObject *contextObject, HandleSelf &&handleSelf, HandleLinked &&handleLinked) {
+        for (QQmlContextData *lc = m_linkedContext.data(); lc; lc = lc->m_linkedContext.data()) {
+            handleLinked(lc);
+            if (lc->m_contextObject == contextObject)
+                lc->m_contextObject = nullptr;
+        }
+
+        handleSelf(this);
+        if (m_contextObject == contextObject)
+            m_contextObject = nullptr;
+    }
+
+    void deepClearContextObject(QObject *contextObject)
+    {
+        deepClearContextObject(
+                contextObject,
+                [](QQmlContextData *self) { self->emitDestruction(); },
+                [](QQmlContextData *){});
+    }
+
     QQmlEngine *engine() const { return m_engine; }
     void setEngine(QQmlEngine *engine) { m_engine = engine; }
 
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 196312314c..c7812059a1 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -217,23 +217,16 @@ void QQmlPrivate::qdeclarativeelement_destructor(QObject *o)
 {
     QObjectPrivate *p = QObjectPrivate::get(o);
     if (QQmlData *d = QQmlData::get(p)) {
+        const auto invalidate = [](QQmlContextData *c) {c->invalidate();};
         if (d->ownContext) {
-            for (QQmlRefPointer<QQmlContextData> lc = d->ownContext->linkedContext(); lc;
-                 lc = lc->linkedContext()) {
-                lc->invalidate();
-                if (lc->contextObject() == o)
-                    lc->setContextObject(nullptr);
-            }
-            d->ownContext->invalidate();
-            if (d->ownContext->contextObject() == o)
-                d->ownContext->setContextObject(nullptr);
+            d->ownContext->deepClearContextObject(o, invalidate, invalidate);
             d->ownContext.reset();
             d->context = nullptr;
+            Q_ASSERT(!d->outerContext || d->outerContext->contextObject() != o);
+        } else if (d->outerContext && d->outerContext->contextObject() == o) {
+            d->outerContext->deepClearContextObject(o, invalidate, invalidate);
         }
 
-        if (d->outerContext && d->outerContext->contextObject() == o)
-            d->outerContext->setContextObject(nullptr);
-
         if (d->hasVMEMetaObject || d->hasInterceptorMetaObject) {
             // This is somewhat dangerous because another thread might concurrently
             // try to resolve the dynamic metaobject. In practice this will then
@@ -406,9 +399,7 @@ void QQmlData::setQueuedForDeletion(QObject *object)
         if (QQmlData *ddata = QQmlData::get(object)) {
             if (ddata->ownContext) {
                 Q_ASSERT(ddata->ownContext.data() == ddata->context);
-                ddata->context->emitDestruction();
-                if (ddata->ownContext->contextObject() == object)
-                    ddata->ownContext->setContextObject(nullptr);
+                ddata->ownContext->deepClearContextObject(object);
                 ddata->ownContext.reset();
                 ddata->context = nullptr;
             }
diff --git a/tests/auto/qml/qqmlcontext/data/A.qml b/tests/auto/qml/qqmlcontext/data/A.qml
new file mode 100644
index 0000000000..1a44f005af
--- /dev/null
+++ b/tests/auto/qml/qqmlcontext/data/A.qml
@@ -0,0 +1,6 @@
+import QtQml
+
+B {
+    id: b
+    property int y: 2
+}
diff --git a/tests/auto/qml/qqmlcontext/data/B.qml b/tests/auto/qml/qqmlcontext/data/B.qml
new file mode 100644
index 0000000000..7754728304
--- /dev/null
+++ b/tests/auto/qml/qqmlcontext/data/B.qml
@@ -0,0 +1,6 @@
+import QtQml
+
+C {
+    id: z
+    property int z: 3
+}
diff --git a/tests/auto/qml/qqmlcontext/data/C.qml b/tests/auto/qml/qqmlcontext/data/C.qml
new file mode 100644
index 0000000000..6afd23aa6c
--- /dev/null
+++ b/tests/auto/qml/qqmlcontext/data/C.qml
@@ -0,0 +1,6 @@
+import QtQml
+
+QtObject {
+    id: outer
+    objectName: "the" + "C"
+}
diff --git a/tests/auto/qml/qqmlcontext/data/destroyContextObject.qml b/tests/auto/qml/qqmlcontext/data/destroyContextObject.qml
new file mode 100644
index 0000000000..7b1f46d7d1
--- /dev/null
+++ b/tests/auto/qml/qqmlcontext/data/destroyContextObject.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    property A a: A {}
+}
diff --git a/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp b/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
index 70728cf03f..ea09253346 100644
--- a/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
+++ b/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
@@ -49,6 +49,7 @@ private slots:
     void outerContextObject();
     void contextObjectHierarchy();
     void destroyContextProperty();
+    void destroyContextObject();
 
     void numericContextProperty();
     void gcDeletesContextObject();
@@ -945,6 +946,30 @@ void tst_qqmlcontext::destroyContextProperty()
     // TODO: Or are we?
 }
 
+void tst_qqmlcontext::destroyContextObject()
+{
+    QQmlEngine engine;
+    QList<QQmlRefPointer<QQmlContextData>> contexts;
+    QQmlComponent component(&engine, testFileUrl("destroyContextObject.qml"));
+    QScopedPointer<QObject> root(component.create());
+
+    QPointer<QObject> a = root->property("a").value<QObject *>();
+    QVERIFY(a);
+
+    for (QQmlRefPointer<QQmlContextData> context = QQmlData::get(a)->ownContext;
+         context; context = context->parent()) {
+        contexts.append(context);
+    }
+
+    QObject *deleted = a.data();
+    root.reset();
+
+    QVERIFY(a.isNull());
+
+    for (const auto &context : contexts)
+        QVERIFY(context->contextObject() != deleted);
+}
+
 void tst_qqmlcontext::numericContextProperty()
 {
     QQmlEngine engine;