qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix storing QObject pointers in dynamic role models 

Storing QObject pointers in a dynamic role model man end up crashing the
application once those objects get deleted.

As it looks impossible to make QVariant::fromValue<QObject*>(...) track
the life cycle in a binary compatible way, this patch fixes the crashes
by tracking with a QPointer at the level of QVariant storage. That's the
QQmlOpenMetaObject in the case of the dynamic role model. In principle a
similar approach is already in place for storage of QML declared
properties - this patch mirrors the approach.

Test cases provided by Ben Lau <xbenlau@gmail.com> and Chris Adams
<chris.adams@jollamobile.com>.

Task-number: QTBUG-35639
Change-Id: I0aee227cd2057654c2cb843c4ebb57ed2ec9a8bf
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
This commit is contained in:
Simon Hausmann 2014-06-29 15:47:04 +08:00
parent 5132709440
commit 9f5bd5ab10
4 changed files with 166 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/qml/qml/qqmlopenmetaobject.cpp
View File

@ -187,14 +187,22 @@ public:
struct Property {
private:
QVariant m_value;
QPointer<QObject> qobjectTracker;
public:
bool valueSet = false;
const QVariant &value() const { return m_value; }
QVariant value() const {
if (QMetaType::typeFlags(m_value.userType()) & QMetaType::PointerToQObject
&& qobjectTracker.isNull())
return QVariant::fromValue<QObject*>(nullptr);
return m_value;
}
QVariant &valueRef() { return m_value; }
void setValue(const QVariant &v) {
m_value = v;
valueSet = true;
if (QMetaType::typeFlags(v.userType()) & QMetaType::PointerToQObject)
qobjectTracker = m_value.value<QObject*>();
}
};

21
tests/auto/qml/qqmllistmodel/data/dynamicroles.qml Normal file
View File

@ -0,0 +1,21 @@
import QtQuick 2.0
Item {
id: root
ListModel {
id: listModel
objectName: "listModel"
dynamicRoles: true
// have to add elements dynamically when dynamicRoles = true
function appendNewElement() {
listModel.append({"name": "test", "obj": null})
}
function setElementAgain() {
var element = listModel.get(0)
listModel.set(0, element)
}
}
}

25
tests/auto/qml/qqmllistmodel/data/qtbug38907.qml Normal file
View File

@ -0,0 +1,25 @@
import QtQuick 2.0
import QtTest 1.0
Item {
Item {
id : testItem
property string name : "testObject"
property var object : this
function testMethod() {
return -1;
}
}
ListModel {
id : listModel
dynamicRoles : true
}
function exec() {
listModel.append(testItem);
listModel.append({ item : testItem });
return true;
}
}

111
tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
View File

@ -106,6 +106,7 @@ private slots:
void get_nested_data();
void crash_model_with_multiple_roles();
void crash_model_with_unknown_roles();
void crash_model_with_dynamic_roles();
void set_model_cache();
void property_changes();
void property_changes_data();
@ -126,6 +127,7 @@ private slots:
void stringifyModelEntry();
void qobjectTrackerForDynamicModelObjects();
void crash_append_empty_array();
void dynamic_roles_crash_QTBUG_38907();
};
bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@ -984,6 +986,97 @@ void tst_qqmllistmodel::crash_model_with_unknown_roles()
model->index(0, 0, QModelIndex()).data(Qt::UserRole);
}
//QTBUG-35639
void tst_qqmllistmodel::crash_model_with_dynamic_roles()
{
{
// setting a dynamic role to a QObject value, then triggering dtor
QQmlEngine eng;
QQmlComponent component(&eng, testFileUrl("dynamicroles.qml"));
QObject *rootItem = component.create();
qWarning() << component.errorString();
QVERIFY(component.errorString().isEmpty());
QVERIFY(rootItem != 0);
QQmlListModel *model = rootItem->findChild<QQmlListModel*>("listModel");
QVERIFY(model != 0);
QMetaObject::invokeMethod(model, "appendNewElement");
QObject *testObj = new QObject;
model->setProperty(0, "obj", QVariant::fromValue<QObject*>(testObj));
delete testObj;
// delete the root item, which will cause the model dtor to run
// previously, this would crash as it attempted to delete testObj.
delete rootItem;
}
{
// setting a dynamic role to a QObject value, then triggering
// DynamicRoleModelNode::updateValues() to trigger unsafe qobject_cast
QQmlEngine eng;
QQmlComponent component(&eng, testFileUrl("dynamicroles.qml"));
QObject *rootItem = component.create();
qWarning() << component.errorString();
QVERIFY(component.errorString().isEmpty());
QVERIFY(rootItem != 0);
QQmlListModel *model = rootItem->findChild<QQmlListModel*>("listModel");
QVERIFY(model != 0);
QMetaObject::invokeMethod(model, "appendNewElement");
QObject *testObj = new QObject;
model->setProperty(0, "obj", QVariant::fromValue<QObject*>(testObj));
delete testObj;
QMetaObject::invokeMethod(model, "setElementAgain");
delete rootItem;
}
{
// setting a dynamic role to a QObject value, then triggering
// DynamicRoleModelNodeMetaObject::propertyWrite()
/*
XXX TODO: I couldn't reproduce this one simply - I think it
requires a WorkerScript sync() call, and that's non-trivial.
I thought I could do it simply via:
QQmlEngine eng;
QQmlComponent component(&eng, testFileUrl("dynamicroles.qml"));
QObject *rootItem = component.create();
qWarning() << component.errorString();
QVERIFY(component.errorString().isEmpty());
QVERIFY(rootItem != 0);
QQmlListModel *model = rootItem->findChild<QQmlListModel*>("listModel");
QVERIFY(model != 0);
QMetaObject::invokeMethod(model, "appendNewElement");
QObject *testObj = new QObject;
model->setProperty(0, "obj", QVariant::fromValue<QObject*>(testObj));
delete testObj;
QObject *testObj2 = new QObject;
model->setProperty(0, "obj", QVariant::fromValue<QObject*>(testObj2));
But it turns out that that doesn't work (the setValue() call within
setProperty() doesn't seem to trigger the right codepath, for some
reason), and you can't trigger it manually via:
QObject *testObj2 = new QObject;
void *a[] = { testObj2, 0 };
QMetaObject::metacall(dynamicNodeModel, QMetaObject::WriteProperty, 0, a);
because the dynamicNodeModel for that index cannot be retrieved
using the public API.
But, anyway, I think the above two test cases are sufficient to
show that QObject* values should be guarded internally.
*/
}
}
//QTBUG-15190
void tst_qqmllistmodel::set_model_cache()
{
@ -1556,6 +1649,24 @@ void tst_qqmllistmodel::crash_append_empty_array()
QCOMPARE(spy.count(), 0);
}
void tst_qqmllistmodel::dynamic_roles_crash_QTBUG_38907()
{
QQmlEngine eng;
QQmlComponent component(&eng, testFileUrl("qtbug38907.qml"));
QVERIFY(!component.isError());
QScopedPointer<QQuickItem> item(qobject_cast<QQuickItem*>(component.create()));
QVERIFY(item != 0);
QVariant retVal;
QMetaObject::invokeMethod(item.data(),
"exec",
Qt::DirectConnection,
Q_RETURN_ARG(QVariant, retVal));
QVERIFY(retVal.toBool());
}
QTEST_MAIN(tst_qqmllistmodel)
#include "tst_qqmllistmodel.moc"