kdebase-workspace (4:4.4.5-7+squeeze1) 34_backport_krunner_crash_fix_threading.diff

Summary

 plasma/generic/runners/places/placesrunner.cpp |   55 ++++++++++++++++++++-----
 plasma/generic/runners/places/placesrunner.h   |   22 ++++++++++
 2 files changed, 66 insertions(+), 11 deletions(-)

    
download this patch

Patch contents

From: Aaron J. Seigo <aseigo@kde.org>
Subject: Fix for krunner occasionally crashes (threading related)
Origin: backport http://websvn.kde.org/?view=revision&revision=1098125
Bug: https://bugs.kde.org/show_bug.cgi?id=236496#c11
Bug: https://bugs.kde.org/show_bug.cgi?id=224212
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607974
Last-Update: 2011-01-18
Forwarded: yes
Applied-Upstream: 4.5.0

move the kfileplaces usage to the main thread; really non-optimal, but KIO
forces our hand. if this works out in trunk, i'll backport to the 4.4 branch.

--- a/plasma/generic/runners/places/placesrunner.cpp
+++ b/plasma/generic/runners/places/placesrunner.cpp
@@ -19,14 +19,20 @@
 
 #include "placesrunner.h"
 
+#include <QCoreApplication>
+#include <QThread>
+#include <QTimer>
+
 #include <KDebug>
 #include <KIcon>
 #include <KRun>
 #include <KUrl>
 
+//Q_DECLARE_METATYPE(Plasma::RunnerContext)
 PlacesRunner::PlacesRunner(QObject* parent, const QVariantList &args)
         : Plasma::AbstractRunner(parent, args)
 {
+//    qRegisterMetaType
     Q_UNUSED(args)
     setObjectName("Places");
     addSyntax(Plasma::RunnerSyntax(":q:", i18n("Finds file manager locations that match :q:")));
@@ -34,7 +40,7 @@ PlacesRunner::PlacesRunner(QObject* pare
 
     // makes sure we have the bookmarks and what not that the places
     // model uses set up in the main thread, otherwise crashes ensue
-    KFilePlacesModel places;
+    m_helper = new PlacesRunnerHelper(this);
 }
 
 PlacesRunner::~PlacesRunner()
@@ -43,21 +49,48 @@ PlacesRunner::~PlacesRunner()
 
 void PlacesRunner::match(Plasma::RunnerContext &context)
 {
+    if (QThread::currentThread() == QCoreApplication::instance()->thread()) {
+        // from the main thread
+        //kDebug() << "calling";
+        m_helper->match(&context);
+    } else {
+        // from the non-gui thread
+        //kDebug() << "emitting";
+        emit doMatch(&context);
+    }
+    //m_helper->match(c);
+}
+
+PlacesRunnerHelper::PlacesRunnerHelper(PlacesRunner *runner)
+    : QObject(runner)
+{
+    Q_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread());
+    connect(runner, SIGNAL(doMatch(Plasma::RunnerContext*)),
+            this, SLOT(match(Plasma::RunnerContext*)),
+            Qt::BlockingQueuedConnection);
+}
+
+void PlacesRunnerHelper::match(Plasma::RunnerContext *c)
+{
+    Plasma::RunnerContext &context = *c;
+    if (!context.isValid()) {
+        return;
+    }
+
     const QString term = context.query();
-    QList<Plasma::QueryMatch> matches;
 
     if (term.length() < 3) {
         return;
     }
 
+    QList<Plasma::QueryMatch> matches;
     const bool all = term.compare(i18n("places"), Qt::CaseInsensitive) == 0;
-    KFilePlacesModel places;
-    for (int i = 0; i <= places.rowCount(); i++) {
-        QModelIndex current_index = places.index(i, 0);
+    for (int i = 0; i <= m_places.rowCount(); i++) {
+        QModelIndex current_index = m_places.index(i, 0);
         Plasma::QueryMatch::Type type = Plasma::QueryMatch::NoMatch;
         qreal relevance = 0;
 
-        const QString text = places.text(current_index);
+        const QString text = m_places.text(current_index);
         if ((all && !text.isEmpty()) || text.compare(term, Qt::CaseInsensitive) == 0) {
             type = Plasma::QueryMatch::ExactMatch;
             relevance = all ? 0.9 : 1.0;
@@ -67,18 +100,18 @@ void PlacesRunner::match(Plasma::RunnerC
         }
 
         if (type != Plasma::QueryMatch::NoMatch) {
-            Plasma::QueryMatch match(this);
+            Plasma::QueryMatch match(static_cast<PlacesRunner *>(parent()));
             match.setType(type);
             match.setRelevance(relevance);
-            match.setIcon(KIcon(places.icon(current_index)));
+            match.setIcon(KIcon(m_places.icon(current_index)));
             match.setText(text);
 
             //if we have to mount it set the device udi instead of the URL, as we can't open it directly
             KUrl url;
-            if (places.isDevice(current_index) && places.setupNeeded(current_index)) {
-                url = places.deviceForIndex(current_index).udi();
+            if (m_places.isDevice(current_index) && m_places.setupNeeded(current_index)) {
+                url = m_places.deviceForIndex(current_index).udi();
             } else {
-                url = places.url(current_index);
+                url = m_places.url(current_index);
             }
 
             match.setData(url);
--- a/plasma/generic/runners/places/placesrunner.h
+++ b/plasma/generic/runners/places/placesrunner.h
@@ -24,6 +24,22 @@
 #include <plasma/abstractrunner.h>
 #include <kfileplacesmodel.h>
 
+class PlacesRunner;
+
+class PlacesRunnerHelper : public QObject
+{
+    Q_OBJECT
+
+public:
+    PlacesRunnerHelper(PlacesRunner *runner);
+
+public Q_SLOTS:
+    void match(Plasma::RunnerContext *context);
+
+private:
+    KFilePlacesModel m_places;
+};
+
 class PlacesRunner : public Plasma::AbstractRunner
 {
     Q_OBJECT
@@ -35,8 +51,14 @@ public:
     void match(Plasma::RunnerContext &context);
     void run(const Plasma::RunnerContext &context, const Plasma::QueryMatch &action);
 
+Q_SIGNALS:
+    void doMatch(Plasma::RunnerContext *context);
+
 private slots:
     void setupComplete(QModelIndex, bool);
+
+private:
+    PlacesRunnerHelper *m_helper;
 };
 
 K_EXPORT_PLASMA_RUNNER(placesrunner, PlacesRunner)