path: root/x11-themes/qtcurve/files/qtcurve-1.9.0-rare_crash_hang_fix.patch

From b7da5ec7e2965332e3922dfb03a3d100aa203b94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20J=2EV=2E=20Bertin?= <rjvbertin@gmail.com>
Date: Fri, 8 Dec 2017 10:10:47 +0100
Subject: address a rare crash/hang on exit

Under rare circumstances Qt would attempt to deliver signals to stale
style instances, or leave orphaned style instances after unloading the
plugin. This is addressed by disconnecting the (currently only) signal
originating from an external application and the plugin now ensures it
leaves no orphaned style instances behind.
Also, moves the code handling pre-exit disconnection to a subclass.

Differential Revision: https://phabricator.kde.org/D9229
---
 qt5/style/qtcurve.cpp        | 72 ++++++++++++++++++++++++++++++++------------
 qt5/style/qtcurve.h          |  5 +--
 qt5/style/qtcurve_plugin.cpp | 15 ++++++---
 3 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/qt5/style/qtcurve.cpp b/qt5/style/qtcurve.cpp
index 1bf6e1d..07eca0f 100644
--- a/qt5/style/qtcurve.cpp
+++ b/qt5/style/qtcurve.cpp
@@ -101,6 +101,26 @@
 
 namespace QtCurve {
 
+class Style::DBusHelper {
+public:
+    DBusHelper()
+        : m_dBus(0)
+        , m_dbusConnected(false)
+    {}
+    ~DBusHelper()
+    {
+        if (m_dBus) {
+            m_dBus->disconnect();
+            m_dBus->deleteLater();
+            m_dBus = 0;
+        }
+    }
+
+    std::once_flag m_aboutToQuitInit;
+    QDBusInterface *m_dBus;
+    bool m_dbusConnected;
+};
+
 static inline void setPainterPen(QPainter *p, const QColor &col, const qreal width=1.0)
 {
     p->setPen(QPen(col, width));
@@ -321,6 +341,7 @@ static void parseWindowLine(const QString &line, QList<int> &data)
 #endif
 
 Style::Style() :
+    m_dBusHelper(new DBusHelper()),
     m_popupMenuCols(0L),
     m_sliderCols(0L),
     m_defBtnCols(0L),
@@ -343,13 +364,11 @@ Style::Style() :
     m_progressBarAnimateTimer(0),
     m_animateStep(0),
     m_titlebarHeight(0),
-    m_dBus(0),
     m_shadowHelper(new ShadowHelper(this)),
     m_sViewSBar(0L),
     m_windowManager(new WindowManager(this)),
     m_blurHelper(new BlurHelper(this)),
-    m_shortcutHandler(new ShortcutHandler(this)),
-    m_dbusConnected(false)
+    m_shortcutHandler(new ShortcutHandler(this))
 {
     const char *env = getenv(QTCURVE_PREVIEW_CONFIG);
 #ifdef QTC_QT5_ENABLE_KDE
@@ -394,6 +413,23 @@ void Style::init(bool initial)
 #ifdef QTC_QT5_ENABLE_KDE
             connect(KWindowSystem::self(), &KWindowSystem::compositingChanged, this, &Style::compositingToggled);
 #endif
+            // prepare the cleanup handler
+            if (QCoreApplication::instance()) {
+                std::call_once(m_dBusHelper->m_aboutToQuitInit, [this] {
+                    connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, [this] () {
+                            // disconnect from the session DBus. We're no longer interested in the
+                            // information it might send when the app we're serving is shutting down.
+                            disconnectDBus();
+                            // Stop listening to select signals. We shouldn't stop emitting signals
+                            // (like QObject::destroyed) but we can reduce the likelihood that pending
+                            // signals will be sent to us post-mortem.
+#ifdef QTC_QT5_ENABLE_KDE
+                            disconnect(KWindowSystem::self(), &KWindowSystem::compositingChanged,
+                                       this, &Style::compositingToggled);
+#endif
+                        } );
+                } );
+            }
         }
     }
 
@@ -663,14 +699,11 @@ void Style::init(bool initial)
 
 void Style::connectDBus()
 {
-    if (m_dbusConnected)
+    if (m_dBusHelper->m_dbusConnected)
         return;
     auto bus = QDBusConnection::sessionBus();
     if (bus.isConnected()) {
-        m_dbusConnected = true;
-        if (QCoreApplication::instance()) {
-            connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &Style::disconnectDBus);
-        }
+        m_dBusHelper->m_dbusConnected = true;
         bus.connect(QString(), "/KGlobalSettings", "org.kde.KGlobalSettings",
                     "notifyChange", this, SLOT(kdeGlobalSettingsChange(int, int)));
 #ifndef QTC_QT5_ENABLE_KDE
@@ -699,12 +732,15 @@ void Style::connectDBus()
 
 void Style::disconnectDBus()
 {
-    if (!m_dbusConnected)
+    if (!m_dBusHelper->m_dbusConnected)
         return;
-    m_dbusConnected = false;
     auto bus = QDBusConnection::sessionBus();
+    if (!bus.isConnected())
+        return;
+    m_dBusHelper->m_dbusConnected = false;
     if (getenv("QTCURVE_DEBUG")) {
         qWarning() << Q_FUNC_INFO << this << "Disconnecting from" << bus.name() << "/" << bus.baseService();
+        dumpObjectInfo();
     }
     bus.disconnect(QString(), "/KGlobalSettings", "org.kde.KGlobalSettings",
                    "notifyChange",
@@ -739,9 +775,7 @@ Style::~Style()
         m_plugin->m_styleInstances.removeAll(this);
     }
     freeColors();
-    if (m_dBus) {
-        delete m_dBus;
-    }
+    delete m_dBusHelper;
 }
 
 void Style::freeColor(QSet<QColor *> &freedColors, QColor **cols)
@@ -4467,10 +4501,10 @@ void Style::emitMenuSize(QWidget *w, unsigned short size, bool force)
         if (oldSize != size) {
             w->setProperty(constMenuSizeProperty, size);
             qtcX11SetMenubarSize(wid, size);
-            if(!m_dBus)
-                m_dBus = new QDBusInterface("org.kde.kwin", "/QtCurve",
+            if(!m_dBusHelper->m_dBus)
+                m_dBusHelper->m_dBus = new QDBusInterface("org.kde.kwin", "/QtCurve",
                                              "org.kde.QtCurve");
-            m_dBus->call(QDBus::NoBlock, "menuBarSize",
+            m_dBusHelper->m_dBus->call(QDBus::NoBlock, "menuBarSize",
                           (unsigned int)wid, (int)size);
         }
     }
@@ -4479,10 +4513,10 @@ void Style::emitMenuSize(QWidget *w, unsigned short size, bool force)
 void Style::emitStatusBarState(QStatusBar *sb)
 {
     if (opts.statusbarHiding & HIDE_KWIN) {
-        if (!m_dBus)
-            m_dBus = new QDBusInterface("org.kde.kwin", "/QtCurve",
+        if (!m_dBusHelper->m_dBus)
+            m_dBusHelper->m_dBus = new QDBusInterface("org.kde.kwin", "/QtCurve",
                                         "org.kde.QtCurve");
-        m_dBus->call(QDBus::NoBlock, "statusBarState",
+        m_dBusHelper->m_dBus->call(QDBus::NoBlock, "statusBarState",
                      (unsigned int)qtcGetWid(sb->window()),
                      sb->isVisible());
     }
diff --git a/qt5/style/qtcurve.h b/qt5/style/qtcurve.h
index 56960a5..ecfa2e7 100644
--- a/qt5/style/qtcurve.h
+++ b/qt5/style/qtcurve.h
@@ -522,6 +522,9 @@ private:
                                       const QWidget *widget) const;
 
 private:
+    class DBusHelper;
+    DBusHelper *m_dBusHelper;
+
     mutable Options opts;
     QColor m_highlightCols[TOTAL_SHADES + 1],
         m_backgroundCols[TOTAL_SHADES + 1],
@@ -564,14 +567,12 @@ private:
     mutable QList<int> m_mdiButtons[2]; // 0=left, 1=right
     mutable int m_titlebarHeight;
 
-    QDBusInterface *m_dBus;
     ShadowHelper *m_shadowHelper;
     mutable QScrollBar *m_sViewSBar;
     mutable QMap<QWidget*, QSet<QWidget*> > m_sViewContainers;
     WindowManager *m_windowManager;
     BlurHelper *m_blurHelper;
     ShortcutHandler *m_shortcutHandler;
-    bool m_dbusConnected;
 #ifdef QTC_QT5_ENABLE_KDE
     KSharedConfigPtr m_configFile;
     KSharedConfigPtr m_kdeGlobals;
diff --git a/qt5/style/qtcurve_plugin.cpp b/qt5/style/qtcurve_plugin.cpp
index ce363ac..481fffc 100644
--- a/qt5/style/qtcurve_plugin.cpp
+++ b/qt5/style/qtcurve_plugin.cpp
@@ -129,6 +129,11 @@ StylePlugin::create(const QString &key)
     if (key.toLower() == "qtcurve") {
         qtc = new Style;
         qtc->m_plugin = this;
+        // keep track of all style instances we allocate, for instance
+        // for KLineEdit widgets which apparently insist on overriding
+        // certain things (cf. KLineEditStyle). We want to be able to
+        // delete those instances as properly and as early as
+        // possible during the global destruction phase.
         m_styleInstances << qtc;
     } else {
         qtc = nullptr;
@@ -151,12 +156,14 @@ StylePlugin::~StylePlugin()
     qtcInfo("Deleting QtCurve plugin (%p)\n", this);
     if (!m_styleInstances.isEmpty()) {
         qtcWarn("there remain(s) %d Style instance(s)\n", m_styleInstances.count());
-        QList<Style*>::Iterator it = m_styleInstances.begin();
-        while (it != m_styleInstances.end()) {
-            Style *that = *it;
-            it = m_styleInstances.erase(it);
+        foreach (Style *that, m_styleInstances) {
+            // don't let ~Style() touch m_styleInstances from here.
+            that->m_plugin = nullptr;
+            // each instance should already have disconnected from the D-Bus
+            // and disconnected from receiving select signals.
             delete that;
         }
+        m_styleInstances.clear();
     }
     if (firstPlInstance == this) {
         firstPlInstance = nullptr;
-- 
cgit v0.11.2