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

ListModel: Use PersistentValue to keep track of objects 

This is the simplest way to prevent the garbage collector from
destroying the object while at the same time allowing a manual
destruction.

Pick-to: 6.5 6.4 6.2
Task-number: QTBUG-95895
Task-number: QTBUG-96167
Fixes: QTBUG-91390
Change-Id: Ic3f3146bc555991068ce3367971e050f745d235d
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ulf Hermann 2023-01-27 13:55:46 +01:00
parent 9dae539949
commit 718f3469f6
4 changed files with 71 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
src/qmlmodels/qqmllistmodel.cpp
View File

@ -98,7 +98,7 @@ const ListLayout::Role &ListLayout::createRole(const QString &key, ListLayout::R
sizeof(double),
sizeof(bool),
sizeof(ListModel *),
sizeof(ListElement::GuardedQObjectPointer),
sizeof(QV4::PersistentValue),
sizeof(QVariantMap),
sizeof(QDateTime),
sizeof(QUrl),
@ -618,10 +618,9 @@ void ListModel::set(int elementIndex, QV4::Object *object, QVector<int> *roles)
roleIndex = e->setFunctionProperty(r, jsv);
} else if (QV4::Object *o = propertyValue->as<QV4::Object>()) {
if (QV4::QObjectWrapper *wrapper = o->as<QV4::QObjectWrapper>()) {
QObject *o = wrapper->object();
const ListLayout::Role &role = m_layout->getRoleOrCreate(propertyName, ListLayout::Role::QObject);
if (role.type == ListLayout::Role::QObject)
roleIndex = e->setQObjectProperty(role, o);
roleIndex = e->setQObjectProperty(role, wrapper);
} else if (QVariant maybeUrl = QV4::ExecutionEngine::toVariant(
o->asReturnedValue(), QMetaType::fromType<QUrl>(), true);
maybeUrl.metaType() == QMetaType::fromType<QUrl>()) {
@ -710,10 +709,9 @@ void ListModel::set(int elementIndex, QV4::Object *object, ListModel::SetElement
}
} else if (QV4::Object *o = propertyValue->as<QV4::Object>()) {
if (QV4::QObjectWrapper *wrapper = o->as<QV4::QObjectWrapper>()) {
QObject *o = wrapper->object();
const ListLayout::Role &r = m_layout->getRoleOrCreate(propertyName, ListLayout::Role::QObject);
if (r.type == ListLayout::Role::QObject)
e->setQObjectPropertyFast(r, o);
e->setQObjectPropertyFast(r, wrapper);
} else {
QVariant maybeUrl = QV4::ExecutionEngine::toVariant(
o->asReturnedValue(), QMetaType::fromType<QUrl>(), true);
@ -838,12 +836,11 @@ StringOrTranslation *ListElement::getStringProperty(const ListLayout::Role &role
return s;
}
QObject *ListElement::getQObjectProperty(const ListLayout::Role &role)
QV4::QObjectWrapper *ListElement::getQObjectProperty(const ListLayout::Role &role)
{
char *mem = getPropertyMemory(role);
GuardedQObjectPointer *o
= reinterpret_cast<GuardedQObjectPointer *>(mem);
return o->data();
QV4::PersistentValue *g = reinterpret_cast<QV4::PersistentValue *>(mem);
return g->as<QV4::QObjectWrapper>();
}
QVariantMap *ListElement::getVariantMapProperty(const ListLayout::Role &role)
@ -890,13 +887,13 @@ QJSValue *ListElement::getFunctionProperty(const ListLayout::Role &role)
return f;
}
ListElement::GuardedQObjectPointer *
QV4::PersistentValue *
ListElement::getGuardProperty(const ListLayout::Role &role)
{
char *mem = getPropertyMemory(role);
bool existingGuard = false;
for (size_t i = 0; i < sizeof(GuardedQObjectPointer);
for (size_t i = 0; i < sizeof(QV4::PersistentValue);
++i) {
if (mem[i] != 0) {
existingGuard = true;
@ -904,12 +901,12 @@ ListElement::getGuardProperty(const ListLayout::Role &role)
}
}
GuardedQObjectPointer *o = nullptr;
QV4::PersistentValue *g = nullptr;
if (existingGuard)
o = reinterpret_cast<GuardedQObjectPointer *>(mem);
g = reinterpret_cast<QV4::PersistentValue *>(mem);
return o;
return g;
}
ListModel *ListElement::getListProperty(const ListLayout::Role &role)
@ -965,12 +962,8 @@ QVariant ListElement::getProperty(const ListLayout::Role &role, const QQmlListMo
break;
case ListLayout::Role::QObject:
{
GuardedQObjectPointer *guard =
reinterpret_cast<GuardedQObjectPointer *>(
mem);
QObject *object = guard->data();
if (object)
data = QVariant::fromValue(object);
QV4::PersistentValue *guard = reinterpret_cast<QV4::PersistentValue *>(mem);
data = QVariant::fromValue(guard->as<QV4::QObjectWrapper>()->object());
}
break;
case ListLayout::Role::VariantMap:
@ -1082,67 +1075,17 @@ int ListElement::setListProperty(const ListLayout::Role &role, ListModel *m)
return roleIndex;
}
static void
restoreQObjectOwnership(ListElement::GuardedQObjectPointer *pointer)
{
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);
}
}
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);
if (!ddata->explicitIndestructibleSet)
ddata->indestructible = ownership != 0;
new (mem) ListElement::GuardedQObjectPointer(
o, static_cast<ListElement::ObjectIndestructible>(ownership));
}
int ListElement::setQObjectProperty(const ListLayout::Role &role, QObject *o)
int ListElement::setQObjectProperty(const ListLayout::Role &role, QV4::QObjectWrapper *o)
{
int roleIndex = -1;
if (role.type == ListLayout::Role::QObject) {
char *mem = getPropertyMemory(role);
GuardedQObjectPointer *g =
reinterpret_cast<GuardedQObjectPointer *>(mem);
bool existingGuard = false;
for (size_t i = 0; i < sizeof(GuardedQObjectPointer);
++i) {
if (mem[i] != 0) {
existingGuard = true;
break;
}
}
bool changed;
if (existingGuard) {
changed = g->data() != o;
if (changed)
restoreQObjectOwnership(g);
g->~GuardedQObjectPointer();
} else {
changed = true;
}
setQObjectOwnership(mem, o);
if (changed)
roleIndex = role.index;
if (isMemoryUsed<QVariantMap>(mem))
reinterpret_cast<QV4::PersistentValue *>(mem)->set(o->engine(), *o);
else
new (mem) QV4::PersistentValue(o->engine(), o);
roleIndex = role.index;
}
return roleIndex;
@ -1275,11 +1218,10 @@ void ListElement::setBoolPropertyFast(const ListLayout::Role &role, bool b)
*value = b;
}
void ListElement::setQObjectPropertyFast(const ListLayout::Role &role, QObject *o)
void ListElement::setQObjectPropertyFast(const ListLayout::Role &role, QV4::QObjectWrapper *o)
{
char *mem = getPropertyMemory(role);
setQObjectOwnership(mem, o);
new (mem) QV4::PersistentValue(o->engine(), o);
}
void ListElement::setListPropertyFast(const ListLayout::Role &role, ListModel *m)
@ -1396,7 +1338,7 @@ QVector<int> ListElement::sync(ListElement *src, ListLayout *srcLayout, ListElem
break;
case ListLayout::Role::QObject:
{
QObject *object = src->getQObjectProperty(srcRole);
QV4::QObjectWrapper *object = src->getQObjectProperty(srcRole);
roleIndex = target->setQObjectProperty(targetRole, object);
}
break;
@ -1451,14 +1393,8 @@ void ListElement::destroy(ListLayout *layout)
break;
case ListLayout::Role::QObject:
{
GuardedQObjectPointer *guard =
getGuardProperty(r);
if (guard) {
restoreQObjectOwnership(guard);
guard->~GuardedQObjectPointer();
}
if (QV4::PersistentValue *guard = getGuardProperty(r))
guard->~PersistentValue();
}
break;
case ListLayout::Role::VariantMap:
@ -1595,8 +1531,7 @@ int ListElement::setJsProperty(const ListLayout::Role &role, const QV4::Value &d
QV4::ScopedObject o(scope, d);
QV4::QObjectWrapper *wrapper = o->as<QV4::QObjectWrapper>();
if (role.type == ListLayout::Role::QObject && wrapper) {
QObject *o = wrapper->object();
roleIndex = setQObjectProperty(role, o);
roleIndex = setQObjectProperty(role, wrapper);
} else if (role.type == ListLayout::Role::VariantMap) {
roleIndex = setVariantMapProperty(role, o);
} else if (role.type == ListLayout::Role::Url) {

45
src/qmlmodels/qqmllistmodel_p_p.h
View File

@ -245,43 +245,6 @@ 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);
~ListElement();
@ -300,7 +263,7 @@ private:
int setDoubleProperty(const ListLayout::Role &role, double n);
int setBoolProperty(const ListLayout::Role &role, bool b);
int setListProperty(const ListLayout::Role &role, ListModel *m);
int setQObjectProperty(const ListLayout::Role &role, QObject *o);
int setQObjectProperty(const ListLayout::Role &role, QV4::QObjectWrapper *o);
int setVariantMapProperty(const ListLayout::Role &role, QV4::Object *o);
int setVariantMapProperty(const ListLayout::Role &role, QVariantMap *m);
int setDateTimeProperty(const ListLayout::Role &role, const QDateTime &dt);
@ -311,7 +274,7 @@ private:
void setStringPropertyFast(const ListLayout::Role &role, const QString &s);
void setDoublePropertyFast(const ListLayout::Role &role, double n);
void setBoolPropertyFast(const ListLayout::Role &role, bool b);
void setQObjectPropertyFast(const ListLayout::Role &role, QObject *o);
void setQObjectPropertyFast(const ListLayout::Role &role, QV4::QObjectWrapper *o);
void setListPropertyFast(const ListLayout::Role &role, ListModel *m);
void setVariantMapFast(const ListLayout::Role &role, QV4::Object *o);
void setDateTimePropertyFast(const ListLayout::Role &role, const QDateTime &dt);
@ -323,8 +286,8 @@ private:
QVariant getProperty(const ListLayout::Role &role, const QQmlListModel *owner, QV4::ExecutionEngine *eng);
ListModel *getListProperty(const ListLayout::Role &role);
StringOrTranslation *getStringProperty(const ListLayout::Role &role);
QObject *getQObjectProperty(const ListLayout::Role &role);
GuardedQObjectPointer *getGuardProperty(const ListLayout::Role &role);
QV4::QObjectWrapper *getQObjectProperty(const ListLayout::Role &role);
QV4::PersistentValue *getGuardProperty(const ListLayout::Role &role);
QVariantMap *getVariantMapProperty(const ListLayout::Role &role);
QDateTime *getDateTimeProperty(const ListLayout::Role &role);
QUrl *getUrlProperty(const ListLayout::Role &role);

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

@ -0,0 +1,21 @@
import QtQml
import QtQml.Models
ListModel {
id: filesModel
property Component testComponent: Component {
id: testComponent
QtObject {
required property string name
}
}
Component.onCompleted: {
filesModel.clear()
for(let i = 0; i < 10; i++) {
filesModel.append({
path: testComponent.createObject(null, { name: "" + i })
})
}
gc()
}
}

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

@ -118,6 +118,7 @@ private slots:
void destroyComponentObject();
void objectOwnershipFlip();
void enumsInListElement();
void protectQObjectFromGC();
};
bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@ -1855,7 +1856,7 @@ void tst_qqmllistmodel::destroyComponentObject()
Q_RETURN_ARG(QVariant, retVal));
QVERIFY(retVal.toBool());
QTRY_VERIFY(created.isNull());
QTRY_VERIFY(list->get(0).property("obj").isUndefined());
QTRY_VERIFY(list->get(0).property("obj").isNull());
QCOMPARE(list->count(), 1);
}
@ -1909,6 +1910,25 @@ void tst_qqmllistmodel::enumsInListElement()
}
}
void tst_qqmllistmodel::protectQObjectFromGC()
{
QQmlEngine engine;
QQmlComponent component(&engine, testFileUrl("protectQObjectFromGC.qml"));
QVERIFY2(component.isReady(), qPrintable(component.errorString()));
QScopedPointer<QObject> root(component.create());
QVERIFY(!root.isNull());
QQmlListModel *listModel = qobject_cast<QQmlListModel *>(root.data());
QVERIFY(listModel);
QCOMPARE(listModel->count(), 10);
for (int i = 0; i < 10; ++i) {
QObject *element = qjsvalue_cast<QObject *>(listModel->get(i).property("path"));
QVERIFY(element);
QCOMPARE(element->property("name").toString(), QString::number(i));
}
}
QTEST_MAIN(tst_qqmllistmodel)
#include "tst_qqmllistmodel.moc"