Memory leak or misunderstanding with QList<QObject*> as model

ixSci

Hi there,

I have the following code:

@void QmlApplicationViewer::updateModel()
{
QList<QObject*> Model;
for(size_t i = 0; i < 100; ++i)
Model << new UserData(QString::number(i));

QDeclarativeContext* pContext = rootContext();
QList<QObject*> PreviousData = pContext->contextProperty("Model").value<QList<QObject*> >();
pContext->setContextProperty("Model", QVariant::fromValue(Model));
foreach(QObject* pEntry, PreviousData)
{
    pEntry->deleteLater();
}

}@
This code is fired up every second by the timer and constantly leaking. I don't understand why it is leaking at all. Am I missed something or I found a bug?

Additional info:
QML file:
@import QtQuick 1.0

Rectangle {
width: 360
height: 360
Column {
Repeater{
model: Model
delegate: Text{
text: model.modelData.name;
}
}
}

}
@
UserData:
@class UserData: public QObject
{
Q_OBJECT

Q_PROPERTY(QString name READ name WRITE setName NOTIFY nameChanged)

QString m_strName;

public:
UserData(const QString& strName, QObject* pParent = 0):
QObject(pParent),
m_strName(strName)
{
}

QString name() const{
    return m_strName;
}

void setName(const QString& strName)
{
    m_strName = strName;
}

signals:
void nameChanged(const QString&);
};@

QmlApplicationViewer - is the class generated by the QtCreator.

full project: http://www.sendspace.com/file/z37b0d

thisisbhaskar

The first problem I see is that your "Model" variable is local to your function, so ever time you go come out of function, memory allocated for "Model" is delete making all its "QObject*" objects to leak. Try to allocate memory for "Model" on heap, and delete "PreviousData" after for loop.

ixSci

-It does not. QList just destroys itself it doesn't free memory allocated for QObject*-
Memory is freed by extracting model from the QML

Chuck Gao

I think you need read about QML/C++ ownership first.

thisisbhaskar

[quote author="ixSci" date="1311067566"]-It does not. QList just destroys itself it doesn't free memory allocated for QObject*-
Memory is freed by extracting model from the QML[/quote]

You are trying to extract model which has been destroyed already. Try to debug and see how many times your for loop runs.. it must be zero if I am not wrong..

@pContext->setContextProperty("Model", QVariant::fromValue(Model));@

You are passing a value here as Model. But Model will be destroyed after you come out of your function. So basically you are trying to use something that has been destroyed already.

ixSci

Vijay Bhaska Reddy, QVariant::fromValue makes copy of the Model and creates QVariant from that copy. Am I wrong? So Model variable is irrelevant after I've set it to the property as I understand it.

Chuck Gao, could you be more specific?

thisisbhaskar

Is your for foreach loop running correct number of times..
And why do you do deleteLater rather delete pEntry??

lgeyer

... and, if I remeber correctly, setting context properties after your QML file has been evaluated might lead to performance issues.

Chuck Gao

When you create your "Model", it have CppOwnership by default. But when you set it to QML property, it will transfer the ownership to javascript if your "Model" have no parent. And the "Model" will be collected by javescript's garbage collection and destroy when ref is 0. Also, create it from heap is the right way.

ixSci

bq. Is your for foreach loop running correct number of times..

I didn't check the all 100 times but it executes many times I don't have any suspicions about it.

bq. And why do you do deleteLater rather delete pEntry??

Because those model element could be still in use in the QML itself and the app will crash with just a delete statement.

I'm pretty sure that the leak is not in the UserData* pointers but in the QtDeclarative itself but maybe I'm using it wrong?

ixSci

bq. When you create your “Model”, it have CppOwnership by default. But when you set it to QML property, it will transfer the ownership to javascript if your “Model” have no parent. And the “Model” will be collected by javescript’s garbage collection and destroy when ref is 0. Also, create it from heap is the right way.

So, I should not have the leaks in such a case, right? GC should delete it. But I have leaks. moreover when I extract the Model property I guarantee that the items will be deleted.

lgeyer

Why do you re-create your model every second at all? They are not meant to be used that way. Models are created once and then have their data updated and changed.

You are doing hundreds of memory allocations and deallocations every second. This is generally not a good idea - having a working garbage collection or not. If I remember correctly setting context properties requires the declarative engine to re-evaluate all bindings - which is again another expensive operation every second.

Rethinking your application design might be the most successful solution for your problem.

[quote author="ixSci" date="1311069363"]So, I should not have the leaks in such a case, right? GC should delete it. But I have leaks. moreover when I extract the Model property I guarantee that the items will be deleted.[/quote]

GC might delete your object, but does not have to. deleteLater() possibly won't delete your object if the script engine still references it.

ixSci

Lukas Geyer, I'd like to do it another way but qt docs states unambiguously:

bq. Note: There is no way for the view to know that the contents of a QList have changed. If the QList changes, it will be necessary to reset the model by calling QDeclarativeContext::setContextProperty() again.

It is from "here":http://doc.qt.nokia.com/4.7-snapshot/qdeclarativemodels.html

bq. GC might delete your object, but does not have to. deleteLater() possibly won’t delete your object if the script engine still references it.

I checked destructor and it is executed. I didn't check it for all the object, though. Maybe the answer is here. So JavaScript owns each object in model? So it increments ref count? I have an app which has the similar(actually the project in this thread is just an excerpt from the actual app) code and it grows from ~30 Mb to ~300 Mb in few days. So if the GC works right then there should not be any problems because I delete objects with deleteLater() and GC works...

lgeyer

Is it possible to switch over to a QAbstractItemModel, like QStandardItemModel?

ixSci

Maybe I will in the future. But for now I want to get rid of leaks in that code. Feature should work correctly :)

I'll wait for another suggestions and will file a bug tomorrow if solution won't be found.

ixSci

http://bugreports.qt.nokia.com/browse/QTCOMPONENTS-917

SteveKing

Could you change the data you enter to be different each second? It would be interesting to see if the QML even registers the change in data.

I'd also be concerned about ripping one model out from under the QML and replacing it with another. Without the beginRemoveRows etc. from an ItemModel how does the QML know that the model has changed?

Have you tried setting the model as a property of a class and then emitting a signal when the model is changed?

ixSci

bq.
I’d also be concerned about ripping one model out from under the QML and replacing it with another. Without the beginRemoveRows etc. from an ItemModel how does the QML know that the model has changed?

It doesn't. So the user have to reset property each time the model changed.