QHash and pointers - how to clear correctly

[roxton]

Hello!
I have the permanent segfault when I try to manually delete pointers from my hash table.
Here is some code.

This is the class that I use as hash table value:

class CMarkupPair: public QObject
{
  Q_OBJECT
 
public:
 
  QHash <QString, QString> pattern;
};

class CMarkupPair: public QObject
{
  Q_OBJECT

public:

  QHash <QString, QString> pattern;
};

To copy to clipboard, switch view to plain text mode 

And thats my table:

QHash <QString, CMarkupPair*> hs_markup;

QHash <QString, CMarkupPair*> hs_markup;

To copy to clipboard, switch view to plain text mode 

I tried many ways to delete values from here. For example:

QList<CMarkupPair *> l = hs_markup.values();
foreach (CMarkupPair *p, l)
          delete p;

QList<CMarkupPair *> l = hs_markup.values();
foreach (CMarkupPair *p, l)
          delete p;

To copy to clipboard, switch view to plain text mode 

Or, even worse:

QList <QString> hs_markup_keys = hs_markup.keys();
 
foreach (QString key, hs_markup_keys)
          {
           CMarkupPair *p = hs_markup.take (key);  
           delete p;
          }

QList <QString> hs_markup_keys = hs_markup.keys();
  
foreach (QString key, hs_markup_keys)
          {
           CMarkupPair *p = hs_markup.take (key);  
           delete p;
          }

To copy to clipboard, switch view to plain text mode 

And when I try to "delete p", I have the segfault. My "p" is not NULL, it's ok pointer. Please help

[ChrisW67]

Post the actual output and backtrace of the program when it crashes. Chances are you have a double-free message (i.e. two values in the list point to the same object) but we can only guess. Calling delete on a null pointer is not fatal in C++ but deleting the same object twice is.

[d_stranz]

Your CMarkupPair class is derived from QObject. When you create the CMarkupPair instances to insert into your hash table, do you give them a non-NULL QObject parent? If you are, then they are probably getting double-deleted - once when you do it, and then again when their parent instance is deleted. Remove your code that deletes the CMarkupPair instances and they will automatically be deleted when their parent instance is deleted.

You might also try calling p->deleteLater() instead of delete(), but again, deleting them yourself isn't necessary if they have a parent.

[stampede]

Or just use shared pointers on parent-less objects. Or QPointer<CMarkupPair> - it will be automatically set to 'null' when the object is deleted (by parent or manually). Don't trust raw pointers, each time you declare a raw pointer, a kitten dies.

[aamer4yu]

How about using qDeleteAll (hs_markup);
hs_markup.clear();

[roxton]

Thanks, deleteLater() resolves the problem! And the CMarkupPair instances have no parent.

How about using qDeleteAll (hs_markup);

Segfault

Thanks to all!
deleteLater() now is my magic

[d_stranz]

Don't trust raw pointers, each time you declare a raw pointer, a kitten dies.

OMG. I now have an army of dead kittens on my conscience. Do they really die, or just lose one of their 9 lives?

[stampede]

I now have an army of dead kittens on my conscience. Do they really die, or just lose one of their 9 lives?

OP: I encourage you to get used to smart pointers, you will really appreciate automated memory management. You can use "deleteLater" with shared pointers:

static void delete_markup(CMarkupPair* obj){
    obj->deleteLater();
}
...
typedef QSharedPointer<CMarkupPair> CMarkupPairPtr;
QHash <QString, CMarkupPairPtr> hs_markup;
// insert:
hs_markup["key"] = CMarkupPairPtr(new CMarkupPair, delete_markup);
// clear:
hs_markup.clear(); // automatically calls deleteLater() on every object in container

static void delete_markup(CMarkupPair* obj){
    obj->deleteLater();
}
...
typedef QSharedPointer<CMarkupPair> CMarkupPairPtr;
QHash <QString, CMarkupPairPtr> hs_markup;
// insert:
hs_markup["key"] = CMarkupPairPtr(new CMarkupPair, delete_markup);
// clear:
hs_markup.clear(); // automatically calls deleteLater() on every object in container

To copy to clipboard, switch view to plain text mode 

[anda_skoa]

For deleteLater() you can even pass this without the custom deleter helper function

QSharedPointer<CMarkupPaor> qobjectPtr(new CMarkupPair, &CMarkupPairt::deleteLater);

QSharedPointer<CMarkupPaor> qobjectPtr(new CMarkupPair, &CMarkupPairt::deleteLater);

To copy to clipboard, switch view to plain text mode 

Cheers,
_

[d_stranz]

I'm sort of puzzled why, if the OP created his CMarkupPair instances without parents, simply deleting them would cause a segfault. The only way I could see this happening is if the code as posted for the CMarkupPair class was incomplete, and it also has signals and/or slots that were still connected when the instances were deleted.

If the instances truly had no other references to them except being in the container (which has no ownership or control of the lifetime of the instances it contains), why would delete() cause a crash? Are there things that happen "behind the curtain" when QObject instances are created which can be undone only during deleteLater()?

[stampede]

Maybe it was a double-delete kind of issue, and posting two "delete" events for an object solved it - after first one is processed, another one is just removed from the event queue:

from QObject::~QObject () docs:
(...) any pending posted events for the object are removed from the event queue.