summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimon Hausmann <simon.hausmann@qt.io>2018-04-26 11:47:27 +0200
committerSimon Hausmann <simon.hausmann@qt.io>2018-04-30 08:19:28 +0000
commit01df9e5f46fd05a80f8f6fcaa91204e6184ded6f (patch)
treee805ad3a843918a5e5c14d8db778a07c64f80d2f
parentf0458d2af4c75837257c9e4c0f130fa168b247dd (diff)
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>
-rw-r--r--src/qml/qml/qqmlcontext.cpp13
-rw-r--r--src/qml/qml/qqmlcontext_p.h5
-rw-r--r--src/qml/types/qqmldelegatemodel.cpp2
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();
}
}