When and how to properly destroy QMenu context menu?

Question

I allow custom context menu to appear over a table. This is how the menu is generated, using a generic function that accepts target widget and coordinates:

#include <QMenu>
void MainWindow::makeContextMenu(const QPoint& pos, QWidget* target)
{
    QMenu *menu = new QMenu(this);
    menu->addAction(new QAction("Action 1", menu));
    menu->addAction(new QAction("Action 2", menu));
    menu->addAction(new QAction("Action 3", menu));
    // Notify window about clicking
    QObject::connect(menu, &QMenu::triggered, this, &MainWindow::menuClicked);
    // If this is a scroll area, map coordinates to real app coordinates
    if(QAbstractScrollArea* area = dynamic_cast<QAbstractScrollArea*>(target))
        menu->popup(area->viewport()->mapToGlobal(pos));
    else
        menu->popup(pos);
}

The problem is that the QMenu* menu never gets destroyed and removed from memory. It persists as MainWindow's child even after it's hidden.

What should I do? Can I set the menu to delete itself? Or should I reuse the same instance of menu or maybe save it into same pointer?

score 10 · Answer 1 · answered Jul 18 '19 at 11:32

It doesn't have to be so complicated. That's it already:

menu->setAttribute(Qt::WA_DeleteOnClose);

That way when the QMenu is closed, the class is deleted as soon as the event loop is entered again. And it doesn't matter if an action was triggered or the popup was just closed.

To prove my answer, you can test it yourself by checking when the menu is created and if the 'deleted' message is triggered with the same address:

qDebug() << "created" << (qintptr)menu;
connect(menu, &QMenu::destroyed, 
        this, [menu]() { qDebug() << "deleted" << (qintptr)menu; });

score 3 · Accepted Answer · edited May 23 '17 at 12:16

From your code, it seems like menu should be deleted after this event has taken place?

// Notify window about clicking
QObject::connect(menu, &QMenu::triggered, this, &MainWindow::menuClicked);

Can I set the menu to delete itself?

Yes, you can have the object delete itself like this:

// Notify window about clicking
QObject::connect(menu, &QMenu::triggered, this, &MainWindow::menuClicked);
QObject::connect(menu, &QMenu::triggered, menu, &QMenu::deleteLater);

If you are worried about the order of those slots being called, see this

Or should I reuse the same instance of menu or maybe save it into same pointer?

Well, you can do something like

//Your constructor
MainWindow::MainWindow(....)
{
    menu = nullptr;
    ....
}

//Make context Menu
void MainWindow::makeContextMenu(const QPoint& pos, QWidget* target)
{
    if(menu)
        delete menu; 
    menu = new QMenu(this);
    ....
}

As for the MainWindow::~MainWindow() destructor, it will take care of the menu's clean up. Since MainWindow (which is a QObject derived class) automatically deletes all children

Lastly, you can simply have the menu as a member of MainWindow, and whenever you need to have fresh actions for menu, you can use QMenu::clear to delete all existing actions.

//Your constructor
MainWindow::MainWindow(....)
{
    menu = new QMenu(this);
    ....
}

void MainWindow::makeContextMenu(const QPoint& pos, QWidget* target)
{
    menu->clear();
    //QMenu *menu = new QMenu(this);
    menu->addAction(new QAction("Action 1", menu));
    menu->addAction(new QAction("Action 2", menu));
    menu->addAction(new QAction("Action 3", menu));
    // Notify window about clicking
    QObject::connect(menu, &QMenu::triggered, this, &MainWindow::menuClicked);
    // If this is a scroll area, map coordinates to real app coordinates
    if(QAbstractScrollArea* area = dynamic_cast<QAbstractScrollArea*>(target))
        menu->popup(area->viewport()->mapToGlobal(pos));
    else
        menu->popup(pos);
}

I found out your suggestion does not solve the problem. This is because user does not have to click on any particular item in the menu... — Tomáš Zato, Sep 01 '16 at 23:48
@TomášZato, so, you are saying `&QMenu::triggered` isn't always raised? .... In that case, what about the second and third options? — WhiZTiM, Sep 01 '16 at 23:50
I considered keeping the menu in some variable only as a last resort option. And only that menu, in particular, is always displayed just once. But it would be nicer if it just deleted itself when it disapears, like QMessageBox does. — Tomáš Zato, Sep 01 '16 at 23:57
QMessageBox does that because its a modal QDialog, and has its event system wired to delete itself when its outta sight. To have a QMenu delete itself, without triggering any event that could be connected to its deleteLater member function.... and without storing the variable somewhere? its going to be more work than you want. Becuase, You will have to subclass it and override its event system to delete itself whenever its hidden — WhiZTiM, Sep 02 '16 at 00:28
Thanks for explaining. It seems to me, though, that this is kinda a newbie trap. If I didn't inspect the qApplication, I wouldn't even have found out about this problem, which is apparently un-trivial to solve. — Tomáš Zato, Sep 02 '16 at 08:30
I think solution one would work with `&QMenu::aboutToHide` instead of `&QMenu::triggered` connected to `&QMenu::deleteLater`. Tested this in my own code where I needed exactly the same thing. — jarzec, Nov 21 '17 at 11:27

score 0 · Answer 3 · answered Sep 05 '16 at 17:58

It is possible to delete QMenu when it's hidden. I designed event filter class for that purpose:

#ifndef DELETEONHIDEFILTER_H
#define DELETEONHIDEFILTER_H

#include <QObject>
#include <QEvent>

class DeleteOnHideFilter : public QObject
{
        Q_OBJECT
    public:
        explicit DeleteOnHideFilter(QObject *parent = 0) : QObject(parent) {}

    protected slots:
        bool eventFilter(QObject *obj, QEvent *event) override {
            if(event->type() == QEvent::Hide) {
                obj->deleteLater();
            }
            return false;
        }
};

#endif // DELETEONHIDEFILTER_H

It can be used for other objects as well.

When and how to properly destroy QMenu context menu?

3 Answers3