diff options
author | Simon Hausmann <simon.hausmann@qt.io> | 2018-04-26 11:47:27 +0200 |
---|---|---|
committer | Simon Hausmann <simon.hausmann@qt.io> | 2018-05-07 08:43:02 +0000 |
commit | a71d01300371721a3d84336262fee41ccfa4563d (patch) | |
tree | d95c46940251645b1a516844d2e919030b066a9a | |
parent | b5bc53a1cb26968414282245bb72460b29dd87d2 (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>
(cherry picked from commit 01df9e5f46fd05a80f8f6fcaa91204e6184ded6f)
-rw-r--r-- | src/qml/qml/qqmlcontext.cpp | 13 | ||||
-rw-r--r-- | src/qml/qml/qqmlcontext_p.h | 5 | ||||
-rw-r--r-- | src/qml/types/qqmldelegatemodel.cpp | 2 |
3 files changed, 14 insertions, 6 deletions
diff --git a/src/qml/qml/qqmlcontext.cpp b/src/qml/qml/qqmlcontext.cpp index 0c431b1260..c06288627b 100644 --- a/src/qml/qml/qqmlcontext.cpp +++ b/src/qml/qml/qqmlcontext.cpp @@ -528,7 +528,7 @@ QQmlContextData::QQmlContextData() QQmlContextData::QQmlContextData(QQmlContext *ctxt) : engine(0), isInternal(false), isJSContext(false), isPragmaLibraryContext(false), unresolvedNames(false), hasEmittedDestruction(false), isRootObjectInCreation(false), - publicContext(ctxt), incubator(0), componentObjectIndex(-1), + stronglyReferencedByParent(false), publicContext(ctxt), incubator(0), componentObjectIndex(-1), contextObject(0), nextChild(0), prevChild(0), expressions(0), contextObjects(0), idValues(0), idValueCount(0), componentAttached(0) @@ -569,7 +569,10 @@ void QQmlContextData::invalidate() while (childContexts) { Q_ASSERT(childContexts != this); - childContexts->invalidate(); + if (childContexts->stronglyReferencedByParent && !--childContexts->refCount) + childContexts->destroy(); + else + childContexts->invalidate(); } if (prevChild) { @@ -656,12 +659,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 d01820a430..e578a1e976 100644 --- a/src/qml/qml/qqmlcontext_p.h +++ b/src/qml/qml/qqmlcontext_p.h @@ -125,7 +125,7 @@ public: QQmlContextData *parent = nullptr; QQmlEngine *engine; - void setParent(QQmlContextData *); + void setParent(QQmlContextData *, bool stronglyReferencedByParent = false); void refreshExpressions(); void addObject(QObject *); @@ -143,7 +143,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 2badef4268..370f4d4c10 100644 --- a/src/qml/types/qqmldelegatemodel.cpp +++ b/src/qml/types/qqmldelegatemodel.cpp @@ -988,7 +988,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(); } } |