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.

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;
}

Answer 2

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

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.