diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2023-01-27 13:55:46 +0100 |
---|---|---|
committer | Ulf Hermann <ulf.hermann@qt.io> | 2023-01-31 10:35:38 +0100 |
commit | 749470e6b81ca0d3f54af230af75b089ac0f2ffd (patch) | |
tree | 50b0668568e477bf1716f16958cf9ce88cbaa26b /src | |
parent | fcacc00bc80459a33b8b9583af5591c0ca6f183a (diff) |
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.
Task-number: QTBUG-95895
Task-number: QTBUG-96167
Fixes: QTBUG-91390
Change-Id: Ic3f3146bc555991068ce3367971e050f745d235d
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 718f3469f693eb179f1504a41b18280656a2325d)
Diffstat (limited to 'src')
-rw-r--r-- | src/qmlmodels/qqmllistmodel.cpp | 137 | ||||
-rw-r--r-- | src/qmlmodels/qqmllistmodel_p_p.h | 45 |
2 files changed, 50 insertions, 132 deletions
diff --git a/src/qmlmodels/qqmllistmodel.cpp b/src/qmlmodels/qqmllistmodel.cpp index 1019a367a8..a8889f3cd0 100644 --- a/src/qmlmodels/qqmllistmodel.cpp +++ b/src/qmlmodels/qqmllistmodel.cpp @@ -129,8 +129,28 @@ 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(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) }; + const int dataSizes[] = { + sizeof(StringOrTranslation), + sizeof(double), + sizeof(bool), + sizeof(ListModel *), + sizeof(QV4::PersistentValue), + sizeof(QVariantMap), + sizeof(QDateTime), + sizeof(QUrl), + sizeof(QJSValue) + }; + const int dataAlignments[] = { + alignof(StringOrTranslation), + alignof(double), + alignof(bool), + alignof(ListModel *), + alignof(QV4::PersistentValue), + alignof(QVariantMap), + alignof(QDateTime), + alignof(QUrl), + alignof(QJSValue) + }; Role *r = new Role; r->name = key; @@ -634,10 +654,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 = o->engine()->toVariant(o->asReturnedValue(), QMetaType::fromType<QUrl>(), true); maybeUrl.metaType() == QMetaType::fromType<QUrl>()) { const ListLayout::Role &r = m_layout->getRoleOrCreate(propertyName, ListLayout::Role::Url); @@ -725,10 +744,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 = o->engine()->toVariant(o->asReturnedValue(), QMetaType::fromType<QUrl>(), true); if (maybeUrl.metaType() == QMetaType::fromType<QUrl>()) { @@ -852,12 +870,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) @@ -904,13 +921,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; @@ -918,12 +935,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) @@ -979,12 +996,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: @@ -1096,67 +1109,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; @@ -1289,11 +1252,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) @@ -1410,7 +1372,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; @@ -1465,14 +1427,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: @@ -1609,8 +1565,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) { diff --git a/src/qmlmodels/qqmllistmodel_p_p.h b/src/qmlmodels/qqmllistmodel_p_p.h index 2d31ffab61..b6339354d8 100644 --- a/src/qmlmodels/qqmllistmodel_p_p.h +++ b/src/qmlmodels/qqmllistmodel_p_p.h @@ -281,43 +281,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(); @@ -336,7 +299,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); @@ -347,7 +310,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); @@ -359,8 +322,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); |