closing parent widget during QMenu::exec()

[hartmut]

I have an open QDialog and create a popup menu and show it with QMenu::exec().

QMenu *menu=new QMenu(mydialog);
...
QAction *ret=menu->exec();
delete menu;
On a Mac it is possible to close the dialog while the menu is open. The menu is closed and destroyed automatically since it is a child of the dialog. However, QMenu::exec() returns after dialog and menu have been destroyed, and the line "delete menu" in the above code creates a crash. Unter Windows this situation never appears because clicking on the dialog's close button closes the menu first and only a second click closes the dialog.

1) Can I just omit the line "delete menu"? Would it create a memory leak if the menu is closed regularly?
2) Is there some way to determine if a QObject still exists or is already destroyed?

[d_stranz]

1) Can I just omit the line "delete menu"? Would it create a memory leak if the menu is closed regularly?

You can omit the line. Since you create the menu with the dialog as its parent, it will be deleted when the dialog is deleted. With QObject-based classes, it is better to call "deleteLater()" instead of delete to ensure that Qt can clean things up properly.


2) Is there some way to determine if a QObject still exists or is already destroyed?

Connect a slot to QObject::destroyed().

You don't show enough code to know how you are creating your QAction instances. If these are created as children of the QMenu instance, then deleting the QMenu would also cause them to be destroyed and you'd end up with an invalid pointer as the returned QAction.

Sounds like you are trying to reinvent a wheel though. QWidget already supports context menus. You simply add QAction instances using QWidget::addAction() and call QWidget::setContextMenuPolicy() with Qt::DefaultContextMenu. Connect your QAction::triggered() signals to the slots you want executed. Qt will automatically create, pop up, and hide the contect menu on a right-mouse press.

[hartmut]

If I don't delete the menu at all and call it several times before I close the dialog, then all menu instances are kept in memory until I close the dialog, right?
But it seems to work with the following code :)

QMenu *menu=new QMenu(mydialog);
...
menu->deleteLater();
QAction *ret=menu->exec();
The menu is called if a button is pressed and is displayed below the button. It works more or less like a Combobox but with the possibility of submenus. The menu content depends on other dialog features. Therefore I create the menu right before it is displayed.
The QActions are children of the menu. After "QAction *act=menu->exec()" I use "act->data().toInt()" to determine the selection and afterwards I would delete the menu.
If the dialog is closed while the menu is open, then menu->exec() returns NULL which I take care of anyway.

[anda_skoa]

1) Can I just omit the line "delete menu"? Would it create a memory leak if the menu is closed regularly?

The menu would be deleted when the dialog is deleted.
You could end up with multple menu instance until that happens.

To avoid that you could keep the menu in a member variable and only create it the first time it is used.



2) Is there some way to determine if a QObject still exists or is already destroyed?
Easily done using QPointer:


QPointer<QMenu> menu(new QMenu(mydialog));

menu->exec();

delete menu.data();

QPointer is a smart pointer for tracking QObjects. It gets reset to 0 if the object is deleted.


If I don't delete the menu at all and call it several times before I close the dialog, then all menu instances are kept in memory until I close the dialog, right?

Yes



But it seems to work with the following code :)

QMenu *menu=new QMenu(mydialog);
...
menu->deleteLater();
QAction *ret=menu->exec();

That could delete the menu while it is opened.
deleteLater() creates a "delete event" and puts it into the event queue.
QMenu::exec() processes events.



The menu is called if a button is pressed and is displayed below the button.

QPushButton::setMenu()?

Cheers,
_

[d_stranz]

QMenu *menu=new QMenu(mydialog);
...
menu->deleteLater();
QAction *ret=menu->exec();

As anda_skoa says, it works by accident. I am surprised it doesn't blow up, but maybe the QMenu::exec() is running its own event loop so the deleteLater() call isn't processed until the menu is closed. Lucky for you.

If you are creating the actions dynamically each time the button is clicked, then why create the QMenu instance on the heap at all? (i.e, using operator new()). Instead, create it on the stack and it will automatically go out of scope (and be cleanly destroyed) when the button's clicked slot exits:



void MyDialog::onButtonClicked()
{
QMenu menu( mydialog );
// ... add actions
QAction * ret = menu.exec();
// do something with the action
} // menu goes out of scope

[hartmut]

Creating the menu on the stack "QMenu menu( mydialog );" does not work. The program crashes immediately if the dialog is closed during menu.exec(). The call stack shows "QWidget::~QWidget()" -> "QObjectPrivate::deleteChildren()" -> "free".
Creating the menu on the stack with "QMenu menu( 0 );" does not crash but the menu remains open while the dialog is closed, which is not what we want.

Now I use the trick with QPointer<QMenu>. This works perfectly. Thank you all for your help.

[d_stranz]

Why would you want to close the dialog that owns the menu which you are executing an action owned by the menu? The QPointer<QMenu> "trick" is exactly that - in this case it looks like a kludge that works around code that likely deletes QWidget instances inappropriately. Yeah, maybe it works, but it probably isn't actually a good solution to the problem.

It is generally bad in Qt to delete your parent object. If the ownership hierarchy is dialog -> menu -> actions, and the triggered() slot for an action results in the dialog being deleted, then that's a fragile piece of code. The safer way would be to make the QAction instances children of whatever owns the dialog - something further up the hierarchy of ownership that stays in scope once the dialog is destroyed. That way, executing the triggered() slot doesn't result in trying to delete the object that owns the action.

[anda_skoa]

Why would you want to close the dialog that owns the menu which you are executing an action owned by the menu?

In this case it is out of the control of the application if I understand hartmut correctly.

It can also happen in other cases, when some application logic deletes windows/dialogs based on external events (e.g. timer, network) while some internals of the window/dialog is spinning a nested event loop.

Creating a modal dialog or menu on the stack only works if you don't pass a parent (which you usually want to do to have window relationship set up correctly) or you are absolutely sure that no code path exists that can delete the window/dialog.

Using QPointer, on the other hand, works in all cases.



The QPointer<QMenu> "trick" is exactly that - in this case it looks like a kludge that works around code that likely deletes QWidget instances inappropriately. Yeah, maybe it works, but it probably isn't actually a good solution to the problem.

It actually is. It is even recommended in e.g. KDE's guidelines, because it happens a lot that external events (in the case of KDE application things like D-Bus messages, but more generically network events or timers) can lead to things being deleted.

See e.g. https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0 for one analysis of the problem.

Cheers,
_

[hartmut]

Exactly, it is not me who closes the dialog. On a Mac (I have OS X 10.11.2) it is just possible for the user to click on the red [x] in the top left corner of the dialog while the menu is open. Then QMenu::exec() returns after everything has been destroyed. So I have no choice but to deal with it.
I was surprised myself, because if I try the same thing under Windows, only the menu disappears but the dialog remains open. Once the menu is closed I have to click a second time on the dialog's close button to really close it.