July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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

Page  
1

Hi there,

I have the following code:

  1. void QmlApplicationViewer::updateModel()
  2. {
  3.     QList<QObject*> Model;
  4.     for(size_t i = 0; i < 100; ++i)
  5.         Model << new UserData(QString::number(i));
  6.  
  7.     QDeclarativeContext* pContext = rootContext();
  8.     QList<QObject*> PreviousData = pContext->contextProperty("Model").value<QList<QObject*> >();
  9.     pContext->setContextProperty("Model", QVariant::fromValue(Model));
  10.     foreach(QObject* pEntry, PreviousData)
  11.     {
  12.         pEntry->deleteLater();
  13.     }
  14. }

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:

  1. import QtQuick 1.0
  2.  
  3. Rectangle {
  4.     width: 360
  5.     height: 360
  6.     Column {
  7.        Repeater{
  8.            model: Model
  9.            delegate: Text{
  10.                    text: model.modelData.name;
  11.            }
  12.        }
  13.     }
  14.  
  15. }

UserData:
  1. class UserData: public QObject
  2. {
  3.     Q_OBJECT
  4.  
  5.     Q_PROPERTY(QString name READ name WRITE setName NOTIFY nameChanged)
  6.  
  7.     QString m_strName;
  8. public:
  9.     UserData(const QString& strName, QObject* pParent = 0):
  10.         QObject(pParent),
  11.         m_strName(strName)
  12.     {
  13.     }
  14.  
  15.     QString name() const{
  16.         return m_strName;
  17.     }
  18.  
  19.     void setName(const QString& strName)
  20.     {
  21.         m_strName = strName;
  22.     }
  23. signals:
  24.     void nameChanged(const QString&);
  25. };

QmlApplicationViewer – is the class generated by the QtCreator.

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

17 replies

July 19, 2011

Vijay Bhaska Reddy Vijay Bhaska Reddy
Lab Rat
399 posts

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.

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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

July 19, 2011

Chuck Gao Chuck Gao
Lab Rat
342 posts

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

 Signature 

Chuck

July 19, 2011

Vijay Bhaska Reddy Vijay Bhaska Reddy
Lab Rat
399 posts
ixSci wrote:
It does not. QList just destroys itself it doesn’t free memory allocated for QObject* Memory is freed by extracting model from the QML

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..

  1. 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.

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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?

July 19, 2011

Vijay Bhaska Reddy Vijay Bhaska Reddy
Lab Rat
399 posts

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

July 19, 2011

Lukas Geyer Lukas Geyer
Lab Rat
2074 posts

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

July 19, 2011

Chuck Gao Chuck Gao
Lab Rat
342 posts

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.

 Signature 

Chuck

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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.

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?

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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.

July 19, 2011

Lukas Geyer Lukas Geyer
Lab Rat
2074 posts

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.

ixSci wrote:
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.

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

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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

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 [doc.qt.nokia.com]

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…

July 19, 2011

Lukas Geyer Lukas Geyer
Lab Rat
2074 posts

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

July 19, 2011

ixSci ixSci
Lab Rat
206 posts

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.

July 20, 2011

ixSci ixSci
Lab Rat
206 posts

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

Page  
1

  ‹‹ QML Text element with Text.AlignJustify      Dynamically create qml objects ››

You must log in to post a reply. Not a member yet? Register here!