From 927ce8c4fe8b91110609b422b1d37d3d0aabe578 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Sat, 11 Feb 2023 17:53:32 +0100 Subject: [PATCH] QMenu: guard for destruction when emitting action signals If a slot connected to a QMenu-action destroys the QMenu, then we must not touch data members in subsequent code, and instead return immediately. We cannot use QBoolBlocker here, as that would reset the data member of QMenuPrivate even when trying to return early. Fixes: QTBUG-106718 Change-Id: I6b5ea471b1bf1f9864e1384382100f8f6c01346f Reviewed-by: Axel Spoerl (cherry picked from commit 52ce4d2d29db8a44fa2fa817cab9ebe969db082e) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/widgets/qmenu.cpp | 16 +++++++++++++--- tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp index 4e4c8ebe153..987dce71d9c 100644 --- a/src/widgets/widgets/qmenu.cpp +++ b/src/widgets/widgets/qmenu.cpp @@ -1387,9 +1387,18 @@ bool QMenuPrivate::mouseEventTaken(QMouseEvent *e) void QMenuPrivate::activateCausedStack(const QList> &causedStack, QAction *action, QAction::ActionEvent action_e, bool self) { - QBoolBlocker guard(activationRecursionGuard); + Q_Q(QMenu); + // can't use QBoolBlocker here + const bool activationRecursionGuardReset = activationRecursionGuard; + activationRecursionGuard = true; + QPointer guard(q); if (self) action->activate(action_e); + if (!guard) + return; + auto boolBlocker = qScopeGuard([this, activationRecursionGuardReset]{ + activationRecursionGuard = activationRecursionGuardReset; + }); for(int i = 0; i < causedStack.size(); ++i) { QPointer widget = causedStack.at(i); @@ -1465,9 +1474,10 @@ void QMenuPrivate::activateAction(QAction *action, QAction::ActionEvent action_e #endif } - + QPointer thisGuard(q); activateCausedStack(causedStack, action, action_e, self); - + if (!thisGuard) + return; if (action_e == QAction::Hover) { #if QT_CONFIG(accessibility) diff --git a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp index 1df16d783e2..73cdbe87fe2 100644 --- a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp +++ b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp @@ -112,6 +112,7 @@ private slots: void tearOffMenuNotDisplayed(); void QTBUG_61039_menu_shortcuts(); void screenOrientationChangedCloseMenu(); + void deleteWhenTriggered(); protected slots: void onActivated(QAction*); @@ -2013,5 +2014,22 @@ void tst_QMenu::screenOrientationChangedCloseMenu() QTRY_COMPARE(menu.isVisible(),false); } +/* + Verify that deleting the menu in a slot connected to an + action's triggered signal doesn't crash. + QTBUG-106718 +*/ +void tst_QMenu::deleteWhenTriggered() +{ + QPointer menu = new QMenu; + QAction *action = menu->addAction("Action", [&menu]{ + delete menu; + }); + menu->popup(QGuiApplication::primaryScreen()->availableGeometry().center()); + menu->setActiveAction(action); + QTest::keyClick(menu, Qt::Key_Return); + QTRY_VERIFY(!menu); +} + QTEST_MAIN(tst_QMenu) #include "tst_qmenu.moc"