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

QQmlListModel: Guard QObject pointers in the list 

If a QObject in the list is deleted from elsewhere, we need to zero it.

Amends commit 3edac1d0a70961e6f86564cc0a98339df6ac5964.

Fixes: QTBUG-94833
Pick-to: 6.2
Change-Id: Ic33a5a0baa160824a34f353806dbfd876fa7123a
Reviewed-by: Maximilian Goldstein <max.goldstein@qt.io>
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Ulf Hermann 2021-06-29 09:36:58 +02:00
parent ea3ef53488
commit b2ee7364be
3 changed files with 93 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
src/qmlmodels/qqmllistmodel.cpp
View File

@ -129,7 +129,7 @@ const ListLayout::Role &ListLayout::getRoleOrCreate(QV4::String *key, Role::Data
const ListLayout::Role &ListLayout::createRole(const QString &key, ListLayout::Role::DataType type)
{
const int dataSizes[] = { sizeof(StringOrTranslation), sizeof(double), sizeof(bool), sizeof(ListModel *), sizeof(QPointer<QObject>), sizeof(QVariantMap), sizeof(QDateTime), sizeof(QUrl), sizeof(QJSValue) };
const int dataSizes[] = { sizeof(StringOrTranslation), sizeof(double), sizeof(bool), sizeof(ListModel *), sizeof(ListElement::GuardedQObjectPointer), sizeof(QVariantMap), sizeof(QDateTime), sizeof(QUrl), sizeof(QJSValue) };
const int dataAlignments[] = { alignof(StringOrTranslation), alignof(double), alignof(bool), alignof(ListModel *), alignof(QObject *), alignof(QVariantMap), alignof(QDateTime), alignof(QUrl), alignof(QJSValue) };
Role *r = new Role;
@ -845,7 +845,8 @@ StringOrTranslation *ListElement::getStringProperty(const ListLayout::Role &role
QObject *ListElement::getQObjectProperty(const ListLayout::Role &role)
{
char *mem = getPropertyMemory(role);
QPointer<QObject> *o = reinterpret_cast<QPointer<QObject> *>(mem);
GuardedQObjectPointer *o
= reinterpret_cast<GuardedQObjectPointer *>(mem);
return o->data();
}
@ -893,13 +894,13 @@ QJSValue *ListElement::getFunctionProperty(const ListLayout::Role &role)
return f;
}
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *
ListElement::GuardedQObjectPointer *
ListElement::getGuardProperty(const ListLayout::Role &role)
{
char *mem = getPropertyMemory(role);
bool existingGuard = false;
for (size_t i = 0; i < sizeof(QTaggedPointer<QObject, ListElement::ObjectIndestructible>);
for (size_t i = 0; i < sizeof(GuardedQObjectPointer);
++i) {
if (mem[i] != 0) {
existingGuard = true;
@ -907,10 +908,10 @@ ListElement::getGuardProperty(const ListLayout::Role &role)
}
}
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *o = nullptr;
GuardedQObjectPointer *o = nullptr;
if (existingGuard)
o = reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(mem);
o = reinterpret_cast<GuardedQObjectPointer *>(mem);
return o;
}
@ -966,8 +967,8 @@ QVariant ListElement::getProperty(const ListLayout::Role &role, const QQmlListMo
break;
case ListLayout::Role::QObject:
{
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *guard =
reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(
GuardedQObjectPointer *guard =
reinterpret_cast<GuardedQObjectPointer *>(
mem);
QObject *object = guard->data();
if (object)
@ -1084,16 +1085,18 @@ int ListElement::setListProperty(const ListLayout::Role &role, ListModel *m)
}
static void
restoreQObjectOwnership(QTaggedPointer<QObject, ListElement::ObjectIndestructible> *pointer)
restoreQObjectOwnership(ListElement::GuardedQObjectPointer *pointer)
{
QQmlData *data = QQmlData::get(pointer->data(), false);
Q_ASSERT(data);
if (QObject *o = pointer->data()) {
QQmlData *data = QQmlData::get(o, 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);
// 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);
}
}
}
@ -1110,7 +1113,7 @@ static void setQObjectOwnership(char *mem, QObject *o)
ddata->indestructible = true;
ddata->explicitIndestructibleSet = false;
new (mem) QTaggedPointer<QObject, ListElement::ObjectIndestructible>(
new (mem) ListElement::GuardedQObjectPointer(
o, static_cast<ListElement::ObjectIndestructible>(ownership));
}
@ -1120,10 +1123,10 @@ int ListElement::setQObjectProperty(const ListLayout::Role &role, QObject *o)
if (role.type == ListLayout::Role::QObject) {
char *mem = getPropertyMemory(role);
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *g =
reinterpret_cast<QTaggedPointer<QObject, ListElement::ObjectIndestructible> *>(mem);
GuardedQObjectPointer *g =
reinterpret_cast<GuardedQObjectPointer *>(mem);
bool existingGuard = false;
for (size_t i = 0; i < sizeof(QTaggedPointer<QObject, ListElement::ObjectIndestructible>);
for (size_t i = 0; i < sizeof(GuardedQObjectPointer);
++i) {
if (mem[i] != 0) {
existingGuard = true;
@ -1135,7 +1138,7 @@ int ListElement::setQObjectProperty(const ListLayout::Role &role, QObject *o)
changed = g->data() != o;
if (changed)
restoreQObjectOwnership(g);
g->~QTaggedPointer();
g->~GuardedQObjectPointer();
} else {
changed = true;
}
@ -1452,13 +1455,13 @@ void ListElement::destroy(ListLayout *layout)
break;
case ListLayout::Role::QObject:
{
QTaggedPointer<QObject, ListElement::ObjectIndestructible> *guard =
GuardedQObjectPointer *guard =
getGuardProperty(r);
if (guard) {
restoreQObjectOwnership(guard);
guard->~QTaggedPointer();
guard->~GuardedQObjectPointer();
}
}
break;

48
src/qmlmodels/qqmllistmodel_p_p.h
View File

@ -278,6 +278,45 @@ private:
class ListElement
{
public:
enum ObjectIndestructible { Indestructible = 1, ExplicitlySet = 2 };
enum { BLOCK_SIZE = 64 - sizeof(int) - sizeof(ListElement *) - sizeof(ModelNodeMetaObject *) };
// This is a basic guarded QObject pointer, with tag. It cannot be copied or moved.
class GuardedQObjectPointer
{
Q_DISABLE_COPY_MOVE(GuardedQObjectPointer)
using RefCountData = QtSharedPointer::ExternalRefCountData;
using Storage = QTaggedPointer<QObject, ObjectIndestructible>;
public:
GuardedQObjectPointer(QObject *o, ObjectIndestructible ownership)
: storage(o, ownership)
, refCount(o ? RefCountData::getAndRef(o) : nullptr)
{}
~GuardedQObjectPointer()
{
if (refCount && !refCount->weakref.deref())
delete refCount;
}
QObject *data() const
{
return (refCount == nullptr || refCount->strongref.loadRelaxed() == 0)
? nullptr
: storage.data();
}
ObjectIndestructible tag() const
{
return storage.tag();
}
private:
Storage storage;
RefCountData *refCount = nullptr;
};
ListElement();
ListElement(int existingUid);
@ -285,13 +324,6 @@ public:
static QVector<int> sync(ListElement *src, ListLayout *srcLayout, ListElement *target, ListLayout *targetLayout);
enum
{
BLOCK_SIZE = 64 - sizeof(int) - sizeof(ListElement *) - sizeof(ModelNodeMetaObject *)
};
enum ObjectIndestructible { Indestructible = 1, ExplicitlySet = 2 };
private:
void destroy(ListLayout *layout);
@ -328,7 +360,7 @@ private:
ListModel *getListProperty(const ListLayout::Role &role);
StringOrTranslation *getStringProperty(const ListLayout::Role &role);
QObject *getQObjectProperty(const ListLayout::Role &role);
QTaggedPointer<QObject, ObjectIndestructible> *getGuardProperty(const ListLayout::Role &role);
GuardedQObjectPointer *getGuardProperty(const ListLayout::Role &role);
QVariantMap *getVariantMapProperty(const ListLayout::Role &role);
QDateTime *getDateTimeProperty(const ListLayout::Role &role);
QUrl *getUrlProperty(const ListLayout::Role &role);

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

@ -133,6 +133,7 @@ private slots:
void undefinedAppendShouldCauseError();
void nullPropertyCrash();
void objectDestroyed();
void destroyObject();
};
bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@ -1788,6 +1789,32 @@ void tst_qqmllistmodel::objectDestroyed()
QTRY_VERIFY(destroyed);
}
void tst_qqmllistmodel::destroyObject()
{
QQmlEngine engine;
QQmlComponent component(&engine);
component.setData(
R"(import QtQuick
ListModel {
id: model
Component.onCompleted: { model.append({"a": contextObject}); }
})",
QUrl());
QVERIFY2(component.isReady(), qPrintable(component.errorString()));
QScopedPointer<QObject> element(new QObject);
engine.rootContext()->setContextProperty(u"contextObject"_qs, element.data());
QScopedPointer<QObject> o(component.create());
QVERIFY(!o.isNull());
QQmlListModel *model = qobject_cast<QQmlListModel *>(o.data());
QVERIFY(model);
QCOMPARE(model->count(), 1);
QCOMPARE(model->get(0).property("a").toQObject(), element.data());
element.reset();
QCOMPARE(model->get(0).property("a").toQObject(), nullptr);
}
QTEST_MAIN(tst_qqmllistmodel)
#include "tst_qqmllistmodel.moc"