diff options
author | Bernhard Übelacker <bernhardu@mailbox.org> | 2017-02-12 14:10:43 +0100 |
---|---|---|
committer | Edward Welbourne <edward.welbourne@qt.io> | 2017-04-10 17:05:50 +0000 |
commit | d438be92dd7068fef94ce98e1ec039fe0ef4f3b3 (patch) | |
tree | bc78d2190469c83f656cf789ea647e5ed81bbeba | |
parent | 617d6dc2017f49a84e4aeb15a40d78462be62326 (diff) |
Avoid access to declarativeData when isDeletingChildren is set
QObject's members declarativeData and currentChildBeingDeleted share
the same memory because they are inside a union.
This leads to a problem when destructing mixed Widgets and QML objects.
Then in QObjectPrivate::deleteChildren the member currentChildBeingDeleted
is set. But unfortunatley QObjectWrapper::destroyObject retrieves
the same pointer via declarativeData.
This patch should avoid this by disallowing retrieval of declarativeData
when isDeletingChildren is set (or at least adds a Q_ASSERT).
Task-number: QTBUG-57714
Change-Id: I9ee02f79be3e8226c30076c24859b49b8dcfaecf
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
-rw-r--r-- | src/qml/jsruntime/qv4qobjectwrapper_p.h | 11 | ||||
-rw-r--r-- | src/qml/qml/qqmldata_p.h | 8 | ||||
-rw-r--r-- | src/qml/qml/qqmlengine.cpp | 30 | ||||
-rw-r--r-- | src/qml/qml/qqmlobjectcreator.cpp | 4 | ||||
-rw-r--r-- | src/qml/qml/qqmlproperty.cpp | 2 | ||||
-rw-r--r-- | src/qml/types/qqmldelegatemodel.cpp | 11 |
6 files changed, 27 insertions, 39 deletions
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h index c7c4f4dd77..3764943499 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper_p.h +++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h @@ -209,13 +209,10 @@ inline ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *obje if (Q_UNLIKELY(QQmlData::wasDeleted(object))) return QV4::Encode::null(); - QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object)); - if (Q_LIKELY(priv->declarativeData)) { - auto ddata = static_cast<QQmlData *>(priv->declarativeData); - if (Q_LIKELY(ddata->jsEngineId == engine->m_engineId && !ddata->jsWrapper.isUndefined())) { - // We own the JS object - return ddata->jsWrapper.value(); - } + auto ddata = QQmlData::get(object); + if (Q_LIKELY(ddata && ddata->jsEngineId == engine->m_engineId && !ddata->jsWrapper.isUndefined())) { + // We own the JS object + return ddata->jsWrapper.value(); } return wrap_slowPath(engine, object); diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index e271598c2d..2083326cd5 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -201,7 +201,9 @@ public: static QQmlData *get(const QObject *object, bool create = false) { QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object)); - if (priv->wasDeleted) { + // If QObjectData::isDeletingChildren is set then access to QObjectPrivate::declarativeData has + // to be avoided because QObjectPrivate::currentChildBeingDeleted is in use. + if (priv->isDeletingChildren || priv->wasDeleted) { Q_ASSERT(!create); return 0; } else if (priv->declarativeData) { @@ -269,8 +271,8 @@ bool QQmlData::wasDeleted(QObject *object) if (!priv || priv->wasDeleted) return true; - return priv->declarativeData && - static_cast<QQmlData *>(priv->declarativeData)->isQueuedForDeletion; + QQmlData *ddata = QQmlData::get(object); + return ddata && ddata->isQueuedForDeletion; } QQmlNotifierEndpoint *QQmlData::notify(int index) diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index f1c592b632..1eb892a0de 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -692,9 +692,7 @@ QQmlEnginePrivate::~QQmlEnginePrivate() void QQmlPrivate::qdeclarativeelement_destructor(QObject *o) { - QObjectPrivate *p = QObjectPrivate::get(o); - if (p->declarativeData) { - QQmlData *d = static_cast<QQmlData*>(p->declarativeData); + if (QQmlData *d = QQmlData::get(o)) { if (d->ownContext && d->context) { d->context->destroy(); d->context = 0; @@ -864,13 +862,10 @@ void QQmlData::markAsDeleted(QObject *o) void QQmlData::setQueuedForDeletion(QObject *object) { if (object) { - if (QObjectPrivate *priv = QObjectPrivate::get(object)) { - if (!priv->wasDeleted && priv->declarativeData) { - QQmlData *ddata = QQmlData::get(object, false); - if (ddata->ownContext && ddata->context) - ddata->context->emitDestruction(); - ddata->isQueuedForDeletion = true; - } + if (QQmlData *ddata = QQmlData::get(object)) { + if (ddata->ownContext && ddata->context) + ddata->context->emitDestruction(); + ddata->isQueuedForDeletion = true; } } } @@ -1319,17 +1314,11 @@ QQmlContext *QQmlEngine::contextForObject(const QObject *object) if(!object) return 0; - QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object)); - - QQmlData *data = - static_cast<QQmlData *>(priv->declarativeData); - - if (!data) - return 0; - else if (data->outerContext) + QQmlData *data = QQmlData::get(object); + if (data && data->outerContext) return data->outerContext->asQQmlContext(); - else - return 0; + + return 0; } /*! @@ -1864,6 +1853,7 @@ void QQmlData::setPendingBindingBit(QObject *obj, int coreIndex) QQmlData *QQmlData::createQQmlData(QObjectPrivate *priv) { Q_ASSERT(priv); + Q_ASSERT(!priv->isDeletingChildren); priv->declarativeData = new QQmlData; return static_cast<QQmlData *>(priv->declarativeData); } diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 09936f6e7a..3ed3ce5460 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -1075,7 +1075,9 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo { QQmlData *ddata = new (ddataMemory) QQmlData; ddata->ownMemory = false; - QObjectPrivate::get(instance)->declarativeData = ddata; + QObjectPrivate* p = QObjectPrivate::get(instance); + Q_ASSERT(!p->isDeletingChildren); + p->declarativeData = ddata; } const int parserStatusCast = type->parserStatusCast(); diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp index 7df8336f51..cec04d25a1 100644 --- a/src/qml/qml/qqmlproperty.cpp +++ b/src/qml/qml/qqmlproperty.cpp @@ -1628,7 +1628,7 @@ QMetaMethod QQmlPropertyPrivate::findSignalByName(const QMetaObject *mo, const Q */ static inline void flush_vme_signal(const QObject *object, int index, bool indexInSignalRange) { - QQmlData *data = static_cast<QQmlData *>(QObjectPrivate::get(const_cast<QObject *>(object))->declarativeData); + QQmlData *data = QQmlData::get(object); if (data && data->propertyCache) { QQmlPropertyData *property = indexInSignalRange ? data->propertyCache->signal(index) : data->propertyCache->method(index); diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp index d9a8b1d179..62a9c40c07 100644 --- a/src/qml/types/qqmldelegatemodel.cpp +++ b/src/qml/types/qqmldelegatemodel.cpp @@ -1959,9 +1959,8 @@ void QQmlDelegateModelItem::destroyObject() Q_ASSERT(object); Q_ASSERT(contextData); - QObjectPrivate *p = QObjectPrivate::get(object); - Q_ASSERT(p->declarativeData); - QQmlData *data = static_cast<QQmlData*>(p->declarativeData); + QQmlData *data = QQmlData::get(object); + Q_ASSERT(data); if (data->ownContext && data->context) data->context->clearContext(); object->deleteLater(); @@ -1978,10 +1977,8 @@ void QQmlDelegateModelItem::destroyObject() QQmlDelegateModelItem *QQmlDelegateModelItem::dataForObject(QObject *object) { - QObjectPrivate *p = QObjectPrivate::get(object); - QQmlContextData *context = p->declarativeData - ? static_cast<QQmlData *>(p->declarativeData)->context - : 0; + QQmlData *d = QQmlData::get(object); + QQmlContextData *context = d ? d->context : 0; for (context = context ? context->parent : 0; context; context = context->parent) { if (QQmlDelegateModelItem *cacheItem = qobject_cast<QQmlDelegateModelItem *>( context->contextObject)) { |