Need to free QList contents?

Question

I have a Qlist full of objects created dynamically. Prior to terminating my program, I call myqlist.clear()

My question is: does this also delete (free) the objects which are contained in the list? Valgrind is giving me some lost blocks and I'm wondering if I misunderstood how the qlist clear method works.

Or, do I need to iterate through the qlist and delete each object?

---

Update: I can confirm that mylist.erase(iterator) is removing the item from the list, but NOT freeing the dynamically allocated object. (The object is a dynamically instantiated class). Very strange! I switched from Qlist to QLinkedList but same results. Remember, my QLinkedList is a QLinkedList< myclass> not a QLinkedList<*myclass>

Here's the actual code in case someone can find what I'm doing wrong:

// Here I define a couple important items.  Note that AMISendMessageFormat is a class
typedef QLinkedList<AMISendMessageFormat> TSentMessageQueue;
TSentMessageQueue m_sentMessageQueue;

// Here I create the message and append to my QLinkedList
AMISendMessageFormat *newMessage = new AMISendMessageFormat(messageToSend);
m_sentMessageQueue.append(*newMessage); 

// Here I delete
for (TSentMessageQueue::Iterator sMessagePtr = m_sentMessageQueue.begin(); sMessagePtr != m_sentMessageQueue.end(); )
{
    sMessagePtr = m_sentMessageQueue.erase(sMessagePtr);  
    qDebug() << "Sent size after erase: " << m_sentMessageQueue.size();  // Confirmed linked list is shrinking in size
}

And after iterating through the list and erasing, valgrind shows each of the AMISendMessageFormat objects are lost blocks!

I suspect this has something to do with erasing inside a loop with an iterator...but I can't get my head around this!

---

See detailed solution below...problem was that the append function makes a copy and adds it to the list...I though it was adding the actual object (not a copy)...so the problem was the 'new' copy was being leaked.

Blodget

A better idiom would probably to store smart pointers instead of raw pointers in your QList so you don't have to deal explicitely with that. It also makes more explicit who has ownership of those objects. – Yirinec

@Yirinec This is good advice in general, but does not really apply to Qt. – Ypres

@pmr: Nowhere inside the docs it is stated that QList takes ownership of pointers and I believe Qt has a QSharedPointer so I'm not sure how this does not apply. Am I missing something here ? – Yirinec

@Yirinec Yes, but if OP is using QObjects this is very likely (as this is a Qt question). I just feel that an answer that skips this topic is not really complete. – Ypres

@Michelle: You're appending an object, not a pointer to an object, so there's no way for append to take it. It has no knowledge whatsoever that there's a pointer involved. You dereference the pointer before append gets to see it. You must also learn not to allocate things on the heap for no good reason. If it fits on the stack, use the stack. – Garrote

How can I allocate on the stack if the number of object is dynamic? – Blodget

Answer 1

You're leaking the instance pointed to by newMessage. This has nothing to do with the list! You're not leaking from the list. Solutions:

// Best

m_sentMessageQueue << AMISendMessageFormat(messageToSend);

// Same, more writing

AMISendMessageFormat newMessage(messageToSend);
m_sentMessageQueue << newMessage;

// Rather pointless allocation on the heap

QScopedPointer<AMISendMessageFormat> newMessage(new AMISendMessageFormat(messageToSend));
m_sentMessageQueue << *newMessage; 

Note that in each case, you're storing a copy of the object into the list. Important: You must verify that AMISendMessageFormat is a properly behaved C++ class that can be safely copy-constructed and assigned to without leaking resources.

If you didn't define the copy-constructor and assignment operator, then all the data members that you use in this class must be safe to copy and assign to without leakage. All Qt and C++ standard library classes will either not compile under such use or will behave properly. If you're using naked pointers, you've shot yourself in the foot, so at the very least use the proper QSharedPointer.

Before the edit, you didn't say what your objects are.

• If you are storing raw-pointers-to-things in your list, then you'll certainly leak memory when you do clear(). A QList treats those pointers just like if they were integers, and doesn't do anything special about them. In C++, destruction of a raw pointer, just as destruction of an integer, is a NO-OP.

• If you are storing QSharedPointer or std::shared_ptr in the list, then you won't leak memory when you do clear(). Smart pointers are called that way for a reason :)

• If you're storing the objects themselves, and they are properly behaved C++ classes, then everything is fine.

You can't store QObject directly in a QList, so your "objects" cannot be QObjects - it wouldn't compile.

This works just fine and behaves properly:

QList<QString> stringList1;
QList<QSharedPointer<QString> > stringList2;

stringList1 << "Foo" << "Bar" << "Baz";
stringList2 << new QString("Foo") << new QString("Bar") << new QString("Baz");

Q_ASSERT(stringList1.at(0) == *stringList2.at(0));
stringList1.clear();
stringList2.clear(); // no memory leaks

This will leak memory, and you almost never need to write code like that:

QList<QString*> stringList3;
stringList3 << new QString("Foo") << new QString("Bar") << new QString("Baz");
stringList3.clear();

Also note that QList, and all decent C++ container types, are RAII. That means they will free the resources they use upon destruction. This means that you don't ever need to call a clear() on a list unless you really want the list cleared. This code doesn't leak resources. The destructor of the list will be called before main() returns, and the list's destructor will destruct all the strings, and they will all properly release the heap memory they allocated.

int main() {
    QList<QString> stringList1;
    stringList1 << "Foo" << "Bar" << "Baz";
    return 0;
}

Goins

Wow...I've been staring at this code for days and couldn't figure that out. Thanks – Blodget

@Michelle You seriously need to get Strostrup's "The C++ programming language, 4th edition" and read the chapters 1 through 12. With understanding. – Garrote

Answer 2

qDeleteAll(list.begin(), list.end());

Bulganin

@Alexander It will work because that is what the qDeleteAll function is designed for: doc.qt.io/qt-4.8/qtalgorithms.html#qDeleteAll-2 – Spectacled

Also note you can just use qDeleteAll(list); – Spectacled

Answer 3

This depends on your objects. Qt has a notion of object ownership, which organizes objects in trees. The parent of the tree deletes all its children as soon as it goes out of scope.

If your objects aren't managed by parent objects you need to make sure to deallocate them yourself. Qt also comes with a set of smart pointers that simplify this.

Ypres

There's no way to store QObject instance in a QList, and his list is a list of objects (not QObjects). – Garrote

@KubaOber You can store QObject* and your objects can derive from it and before the OP added his actual code it sounded like he would store pointers, because he said he allocated the objects... – Ypres