Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix memory leak with ListModel.get
This is a regression introduced with commit
3cc589c, which in turn fixed a leak
with QV4::QObjectWrapper objects. Unfortunately the allocate() call into
the persistent (weak) value storage in the list model introduced a leak
of the weak value itself. This is fixed by replacing the free standing
weak value allocation with the use of the existing jsWrapper weak value
in the declarative data (QQmlData). That weak value is freed property in
the destroy() method of the QV4::QObjectWRapper. The extra QQmlData
allocation is hidden behind a unified allocation, similar to what we do
in void QQmlType::create(QObject **, void **, size_t) const.

Task-number: QTBUG-66189
Change-Id: I5351e3e484542709a6b210e84aa19b14d28e11ad
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
(cherry picked from commit 22d43f74e264626d0c28654c42c91839f9de45b5)
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>
  • Loading branch information
tronical committed Feb 13, 2018
1 parent e38a98a commit f8edd75
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/qml/types/qqmllistmodel.cpp
Expand Up @@ -249,7 +249,12 @@ QObject *ListModel::getOrCreateModelObject(QQmlListModel *model, int elementInde
{
ListElement *e = elements[elementIndex];
if (e->m_objectCache == 0) {
e->m_objectCache = new QObject;
void *memory = operator new(sizeof(QObject) + sizeof(QQmlData));
void *ddataMemory = ((char *)memory) + sizeof(QObject);
e->m_objectCache = new (memory) QObject;
QQmlData *ddata = new (ddataMemory) QQmlData;
ddata->ownMemory = false;
QObjectPrivate::get(e->m_objectCache)->declarativeData = ddata;
(void)new ModelNodeMetaObject(e->m_objectCache, model, elementIndex);
}
return e->m_objectCache;
Expand Down Expand Up @@ -2318,8 +2323,7 @@ QQmlV4Handle QQmlListModel::get(int index) const
QObject *object = m_listModel->getOrCreateModelObject(const_cast<QQmlListModel *>(this), index);
result = scope.engine->memoryManager->allocObject<QV4::ModelObject>(object, const_cast<QQmlListModel *>(this), index);
// Keep track of the QObjectWrapper in persistent value storage
QV4::Value *val = scope.engine->memoryManager->m_weakValues->allocate();
*val = result;
QQmlData::get(object)->jsWrapper.set(scope.engine, result);
}
}

Expand Down
28 changes: 28 additions & 0 deletions tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
Expand Up @@ -128,6 +128,7 @@ private slots:
void about_to_be_signals();
void modify_through_delegate();
void bindingsOnGetResult();
void qobjectTrackerForDynamicModelObjects();
};

bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
Expand Down Expand Up @@ -1487,6 +1488,33 @@ void tst_qqmllistmodel::bindingsOnGetResult()
QVERIFY(obj->property("success").toBool());
}

void tst_qqmllistmodel::qobjectTrackerForDynamicModelObjects()
{
QQmlEngine engine;
QQmlComponent component(&engine);
component.setData(
"import QtQuick 2.0\n"
"Item {\n"
" ListModel {\n"
" id: testModel\n"
" objectName: \"testModel\"\n"
" ListElement { name: \"Joe\"; age: 22 }\n"
" }\n"
"}\n", QUrl());
QScopedPointer<QObject> scene(component.create());
QQmlListModel *model = scene->findChild<QQmlListModel*>("testModel");
QQmlExpression expr(engine.rootContext(), model, "get(0);");
QVariant v = expr.evaluate();
QVERIFY2(!expr.hasError(), QTest::toString(expr.error().toString()));

QObject *obj = v.value<QObject*>();
QVERIFY(obj);

QQmlData *ddata = QQmlData::get(obj, /*create*/false);
QVERIFY(ddata);
QVERIFY(!ddata->jsWrapper.isNullOrUndefined());
}

QTEST_MAIN(tst_qqmllistmodel)

#include "tst_qqmllistmodel.moc"

0 comments on commit f8edd75

Please sign in to comment.