From c462d33d3d132d4cfbe54e619681260193d46dfd Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Thu, 31 Aug 2023 08:58:40 +0200
Subject: [PATCH] QComboBox on macOS: guard against destruction while native
 popup is open
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since showing the native popup on macOS is blocking and processes events
the QComboBox might get destroyed while the popup is open. Guard against
this by using QPointer and returning early (dismissing the scope guard
that would otherwise reset the menu's parent, writing to freed memory).

The problem is then that the native popup remains visible, as the
destructor of QComboBox calls cleanupNativeCombobox which destroys the
platform menu (i.e. the QCocoaMenu instance), but that doesn't dismiss()
the popup. Add a call to dismiss() to the QCocoaMenu destructor to make
sure that destroying the menu closes it first.

Fixes: QTBUG-116155
Change-Id: If0ac19796603667f4c8e80c302710dc4c9aded50
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit 03d62322b239772e0440cee9ce7576147239f339)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/plugins/platforms/cocoa/qcocoamenu.mm |  9 +++++++++
 src/widgets/widgets/qcombobox.cpp         | 19 +++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 4b66d2b610e..6d2ad6f7196 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -42,6 +42,8 @@ QCocoaMenu::~QCocoaMenu()
             item->setMenuParent(nullptr);
     }
 
+    if (isOpen())
+        dismiss();
     [m_nativeMenu release];
 }
 
@@ -320,6 +322,8 @@ void QCocoaMenu::showPopup(const QWindow *parentWindow, const QRect &targetRect,
 {
     QMacAutoReleasePool pool;
 
+    QPointer<QCocoaMenu> guard = this;
+
     QPoint pos =  QPoint(targetRect.left(), targetRect.top() + targetRect.height());
     QCocoaWindow *cocoaWindow = parentWindow ? static_cast<QCocoaWindow *>(parentWindow->handle()) : nullptr;
     NSView *view = cocoaWindow ? cocoaWindow->view() : nil;
@@ -404,6 +408,11 @@ void QCocoaMenu::showPopup(const QWindow *parentWindow, const QRect &targetRect,
         }
     }
 
+    if (!guard) {
+        menuParentGuard.dismiss();
+        return;
+    }
+
     // The calls above block, and also swallow any mouse release event,
     // so we need to clear any mouse button that triggered the menu popup.
     if (cocoaWindow && !cocoaWindow->isForeignWindow())
diff --git a/src/widgets/widgets/qcombobox.cpp b/src/widgets/widgets/qcombobox.cpp
index 32b43ed4fd5..db957014c6a 100644
--- a/src/widgets/widgets/qcombobox.cpp
+++ b/src/widgets/widgets/qcombobox.cpp
@@ -2469,12 +2469,13 @@ void QComboBoxPrivate::cleanupNativePopup()
     if (!m_platformMenu)
         return;
 
+    m_platformMenu->setVisible(false);
     int count = int(m_platformMenu->tag());
     for (int i = 0; i < count; ++i)
         m_platformMenu->menuItemAt(i)->deleteLater();
 
     delete m_platformMenu;
-    m_platformMenu = 0;
+    m_platformMenu = nullptr;
 }
 
 /*!
@@ -2531,15 +2532,18 @@ bool QComboBoxPrivate::showNativePopup()
     else if (q->testAttribute(Qt::WA_MacMiniSize))
         offset = QPoint(-2, 6);
 
+    [[maybe_unused]] QPointer<QComboBox> guard(q);
     const QRect targetRect = QRect(tlw->mapFromGlobal(q->mapToGlobal(offset)), QSize());
     m_platformMenu->showPopup(tlw, QHighDpi::toNativePixels(targetRect, tlw), currentItem);
 
 #ifdef Q_OS_MACOS
-    // The Cocoa popup will swallow any mouse release event.
-    // We need to fake one here to un-press the button.
-    QMouseEvent mouseReleased(QEvent::MouseButtonRelease, q->pos(), q->mapToGlobal(QPoint(0, 0)),
-                              Qt::LeftButton, Qt::MouseButtons(Qt::LeftButton), {});
-    QCoreApplication::sendEvent(q, &mouseReleased);
+    if (guard) {
+        // The Cocoa popup will swallow any mouse release event.
+        // We need to fake one here to un-press the button.
+        QMouseEvent mouseReleased(QEvent::MouseButtonRelease, q->pos(), q->mapToGlobal(QPoint(0, 0)),
+                                Qt::LeftButton, Qt::MouseButtons(Qt::LeftButton), {});
+        QCoreApplication::sendEvent(q, &mouseReleased);
+    }
 #endif
 
     return true;
@@ -3110,7 +3114,10 @@ void QComboBoxPrivate::showPopupFromMouseEvent(QMouseEvent *e)
             // viewContainer(), we avoid creating the QComboBoxPrivateContainer.
             viewContainer()->initialClickPosition = q->mapToGlobal(e->position().toPoint());
         }
+        QPointer<QComboBox> guard = q;
         q->showPopup();
+        if (!guard)
+            return;
         // The code below ensures that regular mousepress and pick item still works
         // If it was not called the viewContainer would ignore event since it didn't have
         // a mousePressEvent first.