[vlc-commits] [Git][videolan/vlc][master] 3 commits: qt: prevent crash due to QList rvalue destruction

Felix Paul Kühne (@fkuehne) gitlab at videolan.org
Sun Feb 16 13:30:52 UTC 2025



Felix Paul Kühne pushed to branch master at VideoLAN / VLC


Commits:
c9a20a27 by Fatih Uzunoglu at 2025-02-16T12:57:02+00:00
qt: prevent crash due to QList rvalue destruction

- - - - -
b51945d5 by Fatih Uzunoglu at 2025-02-16T12:57:02+00:00
qt: unset source instead of deleting the quick window in `CompositorPlatform::unloadGUI()`

This fixes closing the window using the CSD close button crashing the application.

It should not be necessary to delete the window because it has a parent window and
the parent window already gets deleted.

- - - - -
26437461 by Fatih Uzunoglu at 2025-02-16T12:57:02+00:00
qt: remove native event filter in `CSDWin32EventHandler` destructor

- - - - -


4 changed files:

- modules/gui/qt/maininterface/compositor_platform.cpp
- modules/gui/qt/maininterface/mainctx_win32.cpp
- modules/gui/qt/util/csdbuttonmodel.cpp
- modules/gui/qt/util/csdbuttonmodel.hpp


Changes:

=====================================
modules/gui/qt/maininterface/compositor_platform.cpp
=====================================
@@ -115,7 +115,7 @@ void CompositorPlatform::unloadGUI()
 {
     m_rootWindow->removeEventFilter(this);
     m_interfaceWindowHandler.reset();
-    delete m_quickWindow;
+    m_quickWindow->setSource(QUrl());
     commonGUIDestroy();
 }
 


=====================================
modules/gui/qt/maininterface/mainctx_win32.cpp
=====================================
@@ -137,6 +137,11 @@ public:
         updateCSDSettings();
     }
 
+    ~CSDWin32EventHandler()
+    {
+        QApplication::instance()->removeNativeEventFilter(this);
+    }
+
     static int resizeBorderWidth(QWindow *window)
     {
         // NOTE: When possible, Qt makes the application DPI aware, so `GetSystemMetrics()` would
@@ -467,26 +472,34 @@ private:
 
     CSDButton *overlappingButton(const QPoint point)
     {
-        for (auto button : m_buttonmodel->windowCSDButtons())
+        const auto& buttons = m_buttonmodel->windowCSDButtons();
+        const auto it = std::find_if(buttons.begin(), buttons.end(), [&point](const auto& button)
         {
             QRect rect = button->rect();
             rect.setY(rect.y() + 2); // leave a small gap for top resize which is always within the caption area
             if (rect.contains(point))
-                return button;
-        }
-        return nullptr;
+                return true;
+            return false;
+        });
+        if (it == buttons.end())
+            return nullptr;
+
+        return *it;
     }
 
     void hoverExclusive(CSDButton::ButtonType type)
     {
-        for (auto button : m_buttonmodel->windowCSDButtons()) {
+        const auto& buttons = m_buttonmodel->windowCSDButtons();
+        std::for_each(buttons.begin(), buttons.end(), [type](const auto& button)
+        {
             button->setShowHovered(button->type() == type);
-        }
+        });
     }
 
     void handleButtonActionExclusive(CSDButton::ButtonType type, bool pressed)
     {
-        for (auto button : m_buttonmodel->windowCSDButtons())
+        const auto& buttons = m_buttonmodel->windowCSDButtons();
+        std::for_each(buttons.begin(), buttons.end(), [type, pressed](const auto& button)
         {
             if (pressed)
             {
@@ -500,21 +513,25 @@ private:
                 else
                     button->unsetExternalPressed();
             }
-        }
+        });
     }
 
     void resetPressedState()
     {
-        for (auto button : m_buttonmodel->windowCSDButtons())
+        const auto& buttons = m_buttonmodel->windowCSDButtons();
+        std::for_each(buttons.begin(), buttons.end(), [](const auto& button)
+        {
             button->unsetExternalPressed();
+        });
     }
 
     void setAllUnhovered()
     {
-        for (auto button : m_buttonmodel->windowCSDButtons())
+        const auto& buttons = m_buttonmodel->windowCSDButtons();
+        std::for_each(buttons.begin(), buttons.end(), [](const auto& button)
         {
             button->setShowHovered(false);
-        }
+        });
     }
 
     bool m_useClientSideDecoration;


=====================================
modules/gui/qt/util/csdbuttonmodel.cpp
=====================================
@@ -122,7 +122,7 @@ CSDButtonModel::CSDButtonModel(MainCtx *mainCtx, QObject *parent)
     newButton(CSDButton::Close, &CSDButtonModel::closeButtonClicked);
 }
 
-QList<CSDButton *> CSDButtonModel::windowCSDButtons() const
+const QList<CSDButton *> &CSDButtonModel::windowCSDButtons() const
 {
     return m_windowCSDButtons;
 }


=====================================
modules/gui/qt/util/csdbuttonmodel.hpp
=====================================
@@ -101,7 +101,10 @@ class CSDButtonModel : public QObject
 public:
     CSDButtonModel(MainCtx *mainCtx, QObject *parent = nullptr);
 
-    QList<CSDButton *> windowCSDButtons() const;
+    // QList may cause crash on destruction (QTBUG-132126),
+    // return by reference so that the QList rvalue is not
+    // destructed:
+    const QList<CSDButton *>& windowCSDButtons() const;
 
 private slots:
     void minimizeButtonClicked();



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/890c92d0711efea14c7e4e30b923ffd5d927ca6b...26437461335cf8f7ea55d76dfc45f412ceaebf93

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/890c92d0711efea14c7e4e30b923ffd5d927ca6b...26437461335cf8f7ea55d76dfc45f412ceaebf93
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list