From 3ef96cc1844ef7fcb82a6ff247b5c1fb628a3266 Mon Sep 17 00:00:00 2001
From: Axel Spoerl <axel.spoerl@qt.io>
Date: Fri, 11 Aug 2023 18:20:28 +0200
Subject: [PATCH] Harden QMenuPrivate::hideMenu() against menu argument
 becoming stale
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

QMenuPrivate::hideMenu() hides a menu and reposts events in case a new
action has become the active one. For that purpose, it waits 20ms and
60ms for visual effects to be rendered.
If the last action has been triggered, the menu's deleteLater slot has
been called before hideMenu(). In that case, it gets deleted while
events are processed during the waiting periods.
Subsequently, menu becomes stale.

This patch replaces the QMenu * argument with a QPointer. Early returns
are inserted after waiting. QSignalBlocker is replaced by manual signal
blocking, to prevent the signal blocker from recovering the state of a
stale object, when it goes out of scope.

An auto test is not added: The error scenario occurs, when a menu is
triggered by keyboard input, while a mouse event is sent to the main
window from outside the menu. Such a test scenario is complex to match
and exposed to flakiness.

Fixes: QTBUG-115597
Change-Id: I4f937fe66fb1b5cf78ebee70fd0006712172cb12
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
(cherry picked from commit e66cbdf68455263aa6b77d4ea9a4e621837dbad3)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/widgets/widgets/qmenu.cpp | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp
index 391aee9cdaf..c4c127c8a8a 100644
--- a/src/widgets/widgets/qmenu.cpp
+++ b/src/widgets/widgets/qmenu.cpp
@@ -569,10 +569,16 @@ void QMenuPrivate::hideMenu(QMenu *menu)
     };
 
 #if QT_CONFIG(effects)
-    QSignalBlocker blocker(menu);
+    // If deleteLater has been called and the event loop spins, while waiting
+    // for visual effects to happen, menu might become stale.
+    // To prevent a QSignalBlocker from restoring a stale object, block and restore signals manually.
+    QPointer<QMenu> stillAlive(menu);
+    const bool signalsBlocked = menu->signalsBlocked();
+    menu->blockSignals(true);
+
     aboutToHide = true;
     // Flash item which is about to trigger (if any).
-    if (menu->style()->styleHint(QStyle::SH_Menu_FlashTriggeredItem)
+    if (menu && menu->style()->styleHint(QStyle::SH_Menu_FlashTriggeredItem)
         && currentAction && currentAction == actionAboutToTrigger
         && menu->actions().contains(currentAction)) {
         QEventLoop eventLoop;
@@ -583,6 +589,9 @@ void QMenuPrivate::hideMenu(QMenu *menu)
         QTimer::singleShot(60, &eventLoop, SLOT(quit()));
         eventLoop.exec();
 
+        if (!stillAlive)
+            return;
+
         // Select and wait 20 ms.
         menu->setActiveAction(activeAction);
         QTimer::singleShot(20, &eventLoop, SLOT(quit()));
@@ -590,10 +599,16 @@ void QMenuPrivate::hideMenu(QMenu *menu)
     }
 
     aboutToHide = false;
-    blocker.unblock();
+
+    if (stillAlive)
+        menu->blockSignals(signalsBlocked);
+    else
+        return;
+
 #endif // QT_CONFIG(effects)
     if (activeMenu == menu)
         activeMenu = nullptr;
+
     menu->d_func()->causedPopup.action = nullptr;
     menu->close();
     menu->d_func()->causedPopup.widget = nullptr;