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

qqmllistmodel: Fix QObjects getting garbage collected 

Previously QObject's could get garbage collected while in use.
This change fixes this by making them indestructible while in use and then restoring their previous state.

[ChangeLog][QtQml][QQmlListModel][Important Behavior Changes] ListModels now take ownership of the objects added to them, only releasing them after the object has been removed again. This may break existing solutions which rely on the object not being owned by the model.

Fixes: QTBUG-91390
Change-Id: Ifd37c90e13fb0b6ad8a5a06e89f9fc9a429f6316
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 16fc4cf366)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Maximilian Goldstein 2021-06-14 12:45:14 +02:00 committed by Qt Cherry-pick Bot
parent 8e85b3089c
commit d30fad2a71
3 changed files with 97 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/qmlmodels/qqmllistmodel.cpp
View File

@ -893,22 +893,24 @@ QJSValue *ListElement::getFunctionProperty(const ListLayout::Role &role)
return f;
}
QPointer<QObject> *ListElement::getGuardProperty(const ListLayout::Role &role)
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *
ListElement::getGuardProperty(const ListLayout::Role &role)
{
char *mem = getPropertyMemory(role);
bool existingGuard = false;
for (size_t i=0 ; i < sizeof(QPointer<QObject>) ; ++i) {
for (size_t i = 0; i < sizeof(QTaggedPointer<QObject, ListElement::ObjectIndestructible>);
++i) {
if (mem[i] != 0) {
existingGuard = true;
break;
}
}
QPointer<QObject> *o = nullptr;
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *o = nullptr;
if (existingGuard)
o = reinterpret_cast<QPointer<QObject> *>(mem);
o = reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(mem);
return o;
}
@ -964,10 +966,12 @@ QVariant ListElement::getProperty(const ListLayout::Role &role, const QQmlListMo
break;
case ListLayout::Role::QObject:
{
QPointer<QObject> *guard = reinterpret_cast<QPointer<QObject> *>(mem);
QObject *object = guard->data();
if (object)
data = QVariant::fromValue(object);
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *guard =
reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(
mem);
QObject *object = guard->data();
if (object)
data = QVariant::fromValue(object);
}
break;
case ListLayout::Role::VariantMap:
@ -1079,15 +1083,48 @@ int ListElement::setListProperty(const ListLayout::Role &role, ListModel *m)
return roleIndex;
}
static void
restoreQObjectOwnership(QTaggedPointer<QObject, ListElement::ObjectIndestructible> *pointer)
{
QQmlData *data = QQmlData::get(pointer->data(), false);
Q_ASSERT(data);
// Only restore the previous state if the object hasn't become explicitly
// owned
if (!data->explicitIndestructibleSet) {
data->indestructible = (pointer->tag() & ListElement::Indestructible);
data->explicitIndestructibleSet = (pointer->tag() & ListElement::ExplicitlySet);
}
}
static void setQObjectOwnership(char *mem, QObject *o)
{
QQmlData *ddata = QQmlData::get(o, false);
const int ownership = (!ddata || ddata->indestructible ? ListElement::Indestructible : 0)
| (ddata && ddata->explicitIndestructibleSet ? ListElement::ExplicitlySet : 0);
// If ddata didn't exist above, force its creation now
if (!ddata)
ddata = QQmlData::get(o, true);
ddata->indestructible = true;
ddata->explicitIndestructibleSet = false;
new (mem) QTaggedPointer<QObject, ListElement::ObjectIndestructible>(
o, static_cast<ListElement::ObjectIndestructible>(ownership));
}
int ListElement::setQObjectProperty(const ListLayout::Role &role, QObject *o)
{
int roleIndex = -1;
if (role.type == ListLayout::Role::QObject) {
char *mem = getPropertyMemory(role);
QPointer<QObject> *g = reinterpret_cast<QPointer<QObject> *>(mem);
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *g =
reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(mem);
bool existingGuard = false;
for (size_t i=0 ; i < sizeof(QPointer<QObject>) ; ++i) {
for (size_t i = 0; i < sizeof(QTaggedPointer<QObject, ListElement::ObjectIndestructible>);
++i) {
if (mem[i] != 0) {
existingGuard = true;
break;
@ -1096,11 +1133,15 @@ int ListElement::setQObjectProperty(const ListLayout::Role &role, QObject *o)
bool changed;
if (existingGuard) {
changed = g->data() != o;
g->~QPointer();
if (changed)
restoreQObjectOwnership(g);
g->~QTaggedPointer();
} else {
changed = true;
}
new (mem) QPointer<QObject>(o);
setQObjectOwnership(mem, o);
if (changed)
roleIndex = role.index;
}
@ -1238,7 +1279,8 @@ void ListElement::setBoolPropertyFast(const ListLayout::Role &role, bool b)
void ListElement::setQObjectPropertyFast(const ListLayout::Role &role, QObject *o)
{
char *mem = getPropertyMemory(role);
new (mem) QPointer<QObject>(o);
setQObjectOwnership(mem, o);
}
void ListElement::setListPropertyFast(const ListLayout::Role &role, ListModel *m)
@ -1410,9 +1452,14 @@ void ListElement::destroy(ListLayout *layout)
break;
case ListLayout::Role::QObject:
{
QPointer<QObject> *guard = getGuardProperty(r);
if (guard)
guard->~QPointer();
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *guard =
getGuardProperty(r);
if (guard) {
restoreQObjectOwnership(guard);
guard->~QTaggedPointer();
}
}
break;
case ListLayout::Role::VariantMap:

4
src/qmlmodels/qqmllistmodel_p_p.h
View File

@ -290,6 +290,8 @@ public:
BLOCK_SIZE = 64 - sizeof(int) - sizeof(ListElement *) - sizeof(ModelNodeMetaObject *)
};
enum ObjectIndestructible { Indestructible = 1, ExplicitlySet = 2 };
private:
void destroy(ListLayout *layout);
@ -326,7 +328,7 @@ private:
ListModel *getListProperty(const ListLayout::Role &role);
StringOrTranslation *getStringProperty(const ListLayout::Role &role);
QObject *getQObjectProperty(const ListLayout::Role &role);
QPointer<QObject> *getGuardProperty(const ListLayout::Role &role);
QTaggedPointer<QObject, ObjectIndestructible> *getGuardProperty(const ListLayout::Role &role);
QVariantMap *getVariantMapProperty(const ListLayout::Role &role);
QDateTime *getDateTimeProperty(const ListLayout::Role &role);
QUrl *getUrlProperty(const ListLayout::Role &role);

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

@ -132,6 +132,7 @@ private slots:
void nestedListModelIteration();
void undefinedAppendShouldCauseError();
void nullPropertyCrash();
void objectDestroyed();
};
bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@ -1757,6 +1758,36 @@ void tst_qqmllistmodel::nullPropertyCrash()
QScopedPointer<QObject>(component.create());
}
// QTBUG-91390
void tst_qqmllistmodel::objectDestroyed()
{
QQmlEngine engine;
QQmlComponent component(&engine);
component.setData(
R"(import QtQuick
ListModel {
id: model
Component.onCompleted: { model.append({"a": contextObject}); }
})",
QUrl());
QObject *obj = new QObject;
bool destroyed = false;
connect(obj, &QObject::destroyed, [&]() { destroyed = true; });
engine.rootContext()->setContextProperty(u"contextObject"_qs, obj);
engine.setObjectOwnership(obj, QJSEngine::JavaScriptOwnership);
QScopedPointer<QObject>(component.create());
QVERIFY(!destroyed);
engine.collectGarbage();
QTest::qSleep(250);
QVERIFY(!destroyed);
engine.evaluate(u"model.clear();"_qs);
engine.collectGarbage();
QTRY_VERIFY(destroyed);
}
QTEST_MAIN(tst_qqmllistmodel)
#include "tst_qqmllistmodel.moc"