Version control: Move the maintainance of pending threads into a custom class. Also the UpdateItemStatesThread differs now between protecting the globally shared plugin and the locally shared data.

svn path=/trunk/KDE/kdebase/apps/; revision=1107125

src/CMakeLists.txt

@@ -45,6 +45,7 @@ set(dolphinprivate_LIB_SRCS
     tooltips/ktooltip.cpp
     tooltips/ktooltipwindow.cpp
     tooltips/tooltipmanager.cpp
+    versioncontrol/pendingthreadsmaintainer.cpp
     versioncontrol/updateitemstatesthread.cpp
     versioncontrol/versioncontrolobserver.cpp
     viewextensionsfactory.cpp

src/versioncontrol/pendingthreadsmaintainer.cpp

@@ -0,0 +1,77 @@
+/***************************************************************************
+ *   Copyright (C) 2010 by Peter Penz <peter.penz@gmx.at>                  *
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ *   This program is distributed in the hope that it will be useful,       *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+ *   GNU General Public License for more details.                          *
+ *                                                                         *
+ *   You should have received a copy of the GNU General Public License     *
+ *   along with this program; if not, write to the                         *
+ *   Free Software Foundation, Inc.,                                       *
+ *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA            *
+ ***************************************************************************/
+
+#include "pendingthreadsmaintainer.h"
+
+#include <kglobal.h>
+#include <QThread>
+#include <QTimer>
+
+struct PendingThreadsMaintainerSingleton
+{
+    PendingThreadsMaintainer instance;
+};
+K_GLOBAL_STATIC(PendingThreadsMaintainerSingleton, s_pendingThreadsMaintainer)
+
+
+PendingThreadsMaintainer& PendingThreadsMaintainer::instance()
+{
+    return s_pendingThreadsMaintainer->instance;
+}
+
+PendingThreadsMaintainer::~PendingThreadsMaintainer()
+{
+}
+
+void PendingThreadsMaintainer::append(QThread* thread)
+{
+    Q_ASSERT(thread != 0);
+    m_threads.append(thread);
+    m_timer->start();
+}
+
+PendingThreadsMaintainer::PendingThreadsMaintainer() :
+    QObject(),
+    m_threads(),
+    m_timer(0)
+{
+    m_timer = new QTimer(this);
+    m_timer->setSingleShot(true);
+    m_timer->setInterval(5000); // 5 seconds
+    connect(m_timer, SIGNAL(timeout()), this, SLOT(cleanup()));
+}
+
+void PendingThreadsMaintainer::cleanup()
+{
+    QList<QThread*>::iterator it = m_threads.begin();
+    while (it != m_threads.end()) {
+        if ((*it)->isFinished()) {
+            (*it)->deleteLater();
+            it = m_threads.erase(it);
+        } else {
+            ++it;
+        }
+    }
+
+    if (!m_threads.isEmpty()) {
+        m_timer->start();
+    }
+}
+
+#include "pendingthreadsmaintainer.moc"

src/versioncontrol/pendingthreadsmaintainer.h

@@ -0,0 +1,83 @@
+/***************************************************************************
+ *   Copyright (C) 2010 by Peter Penz <peter.penz@gmx.at>                  *
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ *   This program is distributed in the hope that it will be useful,       *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+ *   GNU General Public License for more details.                          *
+ *                                                                         *
+ *   You should have received a copy of the GNU General Public License     *
+ *   along with this program; if not, write to the                         *
+ *   Free Software Foundation, Inc.,                                       *
+ *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA            *
+ ***************************************************************************/
+
+#ifndef PENDINGTHREADSMAINTAINER_H
+#define PENDINGTHREADSMAINTAINER_H
+
+#include <libdolphin_export.h>
+
+#include <QObject>
+
+class QTimer;
+
+/**
+ * If the creator of a thread gets deleted, although the thread is still
+ * working, usually QThread::wait() is invoked. The drawback of this
+ * approach is that the user interface gets blocked for an undefined amount
+ * of time. If the thread does not contain references to the creator, the
+ * deleting can be forwarded to the PendingThreadsMaintainer. In the following
+ * example it is assumed, that m_thread will be 0, if it has been deleted by the
+ * creator after receiving the signal QThread::finished():
+ *
+ * \code
+ * ThreadCreator::~ThreadCreator()
+ * {
+ *     if (m_thread != 0) {
+ *         PendingThreadsMaintainer::instance().append(m_thread);
+ *         m_thread = 0;
+ *     }
+ * }
+ * \endcode
+ *
+ * The thread will get automatically deleted after it (or has already) been finished.
+ *
+ * Implementation note: Connecting to the signal QThread::finished() is
+ * not sufficient, as it is possible that the thread has already emitted
+ * the signal, but the signal has not been received yet by the thread creator.
+ * Because of this a polling is done each 5 seconds to check, whether the
+ * thread has been finished.
+ */
+class LIBDOLPHINPRIVATE_EXPORT PendingThreadsMaintainer : public QObject
+{
+    Q_OBJECT
+
+public:
+    static PendingThreadsMaintainer& instance();
+    virtual ~PendingThreadsMaintainer();
+
+    /**
+     * Appends the thread \p thread to the maintainer. The thread
+     * will be deleted by the maintainer after it has been finished.
+     */
+    void append(QThread* thread);
+
+protected:
+    PendingThreadsMaintainer();
+
+private slots:
+    void cleanup();
+
+private:
+    QList<QThread*> m_threads;
+    QTimer* m_timer;
+
+    friend class PendingThreadsMaintainerSingleton;
+};
+
+#endif

src/versioncontrol/updateitemstatesthread.cpp

@@ -19,17 +19,21 @@
 
 #include "updateitemstatesthread.h"
 
+#include <QMutexLocker>
+
 UpdateItemStatesThread::UpdateItemStatesThread() :
     QThread(),
+    m_globalPluginMutex(0),
+    m_plugin(0),
+    m_itemMutex(),
     m_retrievedItems(false),
-    m_mutex(0),
     m_itemStates()
 {
     // Several threads may share one instance of a plugin. A global
     // mutex is required to serialize the retrieval of version control
     // states inside run().
     static QMutex globalMutex;
-    m_mutex = &globalMutex;
+    m_globalPluginMutex = &globalMutex;
 }
 
 UpdateItemStatesThread::~UpdateItemStatesThread()
@@ -39,8 +43,11 @@ UpdateItemStatesThread::~UpdateItemStatesThread()
 void UpdateItemStatesThread::setData(KVersionControlPlugin* plugin,
                                      const QList<VersionControlObserver::ItemState>& itemStates)
 {
-    m_plugin = plugin;
+    QMutexLocker itemLocker(&m_itemMutex);
     m_itemStates = itemStates;
+
+    QMutexLocker pluginLocker(m_globalPluginMutex);
+    m_plugin = plugin;
 }
 
 void UpdateItemStatesThread::run()
@@ -52,11 +59,14 @@ void UpdateItemStatesThread::run()
     // plugin requires the root directory for KVersionControlPlugin::beginRetrieval(). Instead
     // of doing an expensive search, we utilize the knowledge of the implementation of
     // VersionControlObserver::addDirectory() to be sure that the last item contains the root.
+    QMutexLocker itemLocker(&m_itemMutex);
     const QString directory = m_itemStates.last().item.url().directory(KUrl::AppendTrailingSlash);
+    itemLocker.unlock();
 
-    QMutexLocker locker(m_mutex);
+    QMutexLocker pluginLocker(m_globalPluginMutex);
     m_retrievedItems = false;
     if (m_plugin->beginRetrieval(directory)) {
+        itemLocker.relock();
         const int count = m_itemStates.count();
         for (int i = 0; i < count; ++i) {
             m_itemStates[i].version = m_plugin->versionState(m_itemStates[i].item);
@@ -68,21 +78,23 @@ void UpdateItemStatesThread::run()
 
 bool UpdateItemStatesThread::beginReadItemStates()
 {
-    return m_mutex->tryLock(300);
+    return m_itemMutex.tryLock(300);
 }
 
 void UpdateItemStatesThread::endReadItemStates()
 {
-    m_mutex->unlock();
+    m_itemMutex.unlock();
 }
 
 QList<VersionControlObserver::ItemState> UpdateItemStatesThread::itemStates() const
 {
+    QMutexLocker locker(&m_itemMutex);
     return m_itemStates;
 }
 
 bool UpdateItemStatesThread::retrievedItems() const
 {
+    QMutexLocker locker(&m_itemMutex);
     return m_retrievedItems;
 }
 

src/versioncontrol/updateitemstatesthread.h

@@ -54,9 +54,11 @@ protected:
     virtual void run();
 
 private:
-    bool m_retrievedItems;
+    QMutex* m_globalPluginMutex; // Protects the m_plugin globally
     KVersionControlPlugin* m_plugin;
-    QMutex* m_mutex;
+
+    mutable QMutex m_itemMutex; // Protects m_retrievedItems and m_itemStates
+    bool m_retrievedItems;
     QList<VersionControlObserver::ItemState> m_itemStates;
 };
 

src/versioncontrol/versioncontrolobserver.cpp

@@ -28,6 +28,7 @@
 #include <kservicetypetrader.h>
 #include <kversioncontrolplugin.h>
 
+#include "pendingthreadsmaintainer.h"
 #include "updateitemstatesthread.h"
 
 #include <QAbstractProxyModel>
@@ -35,18 +36,6 @@
 #include <QMutexLocker>
 #include <QTimer>
 
-/*
- * Maintains a list of pending threads, that get regulary checked
- * whether they are finished and hence can get deleted. QThread::wait()
- * is never used to prevent any blocking of the user interface.
- */
-struct PendingThreadsSingleton
-{
-    QList<UpdateItemStatesThread*> list;
-};
-K_GLOBAL_STATIC(PendingThreadsSingleton, s_pendingThreads)
-
-
 VersionControlObserver::VersionControlObserver(QAbstractItemView* view) :
     QObject(view),
     m_pendingItemStatesUpdate(false),
@@ -92,14 +81,11 @@ VersionControlObserver::~VersionControlObserver()
         } else {
             // The version controller gets deleted, while a thread still
             // is working to get the version information. To avoid a blocking
-            // user interface, no waiting for the finished() signal of the thread is
-            // done. Instead the thread will be remembered inside the global
-            // list s_pendingThreads, which will checked regulary. The thread does
-            // not work on shared data that is part of the VersionController instance,
-            // so skipping the waiting is save.
+            // user interface, the thread will be forwarded to the
+            // PendingThreadsMaintainer, which will delete the thread later.
             disconnect(m_updateItemStatesThread, SIGNAL(finished()),
                        this, SLOT(slotThreadFinished()));
-            s_pendingThreads->list.append(m_updateItemStatesThread);
+            PendingThreadsMaintainer::instance().append(m_updateItemStatesThread);
             m_updateItemStatesThread = 0;
         }
     }
@@ -145,19 +131,6 @@ void VersionControlObserver::silentDirectoryVerification()
 
 void VersionControlObserver::verifyDirectory()
 {
-    if (!s_pendingThreads->list.isEmpty()) {
-        // Try to cleanup pending threads (see explanation in destructor)
-        QList<UpdateItemStatesThread*>::iterator it = s_pendingThreads->list.begin();
-        while (it != s_pendingThreads->list.end()) {
-            if ((*it)->isFinished()) {
-                (*it)->deleteLater();
-                it = s_pendingThreads->list.erase(it);
-            } else {
-                ++it;
-            }
-        }
-    }
-
     KUrl versionControlUrl = m_dirLister->url();
     if (!versionControlUrl.isLocalFile()) {
         return;