File 0f511184-kderuntime-nepomuk-47branch-memleak.diff of Package kdebase4-runtime

Overview Repositories Revisions Requests Users Attributes Meta

File 0f511184-kderuntime-nepomuk-47branch-memleak.diff of Package kdebase4-runtime

commit 0f511184ae25364618ba244f6afda5570b02c388
Author: Sebastian Trueg <trueg@kde.org>
Date:   Mon Oct 24 17:47:25 2011 +0200

Run the MetaDataMover with an event loop.
    
    It is using the exact same approach as the file indexer does: a new
    thread is created and started and the MetaDataMover is then
    QObject::moveToThread'ed to it.
    
    This fixes mem leaks caused by DBus events that are not cleaned up.
    
    BUG: 226676

diff --git a/nepomuk/services/filewatch/metadatamover.cpp b/nepomuk/services/filewatch/metadatamover.cpp
index 36ff533..f247fa5 100644
--- a/nepomuk/services/filewatch/metadatamover.cpp
+++ b/nepomuk/services/filewatch/metadatamover.cpp
@@ -1,5 +1,5 @@
 /* This file is part of the KDE Project
-   Copyright (c) 2009-2010 Sebastian Trueg <trueg@kde.org>
+   Copyright (c) 2009-2011 Sebastian Trueg <trueg@kde.org>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -34,42 +34,44 @@
 
 
 Nepomuk::MetadataMover::MetadataMover( Soprano::Model* model, QObject* parent )
-    : QThread( parent ),
-      m_stopped(false),
+    : QObject( parent ),
+      m_queueMutex(QMutex::Recursive),
       m_model( model )
 {
-    QTimer* timer = new QTimer( this );
-    connect( timer, SIGNAL( timeout() ), this, SLOT( slotClearRecentlyFinishedRequests() ) );
-    timer->setInterval( 30000 );
-    timer->start();
-}
-
+    // setup the main update queue timer
+    m_queueTimer = new QTimer(this);
+    connect(m_queueTimer, SIGNAL(timeout()),
+            this, SLOT(slotWorkUpdateQueue()),
+            Qt::DirectConnection);
 
-Nepomuk::MetadataMover::~MetadataMover()
-{
+    // setup the cleanup timer which removes requests that are done
+    m_recentlyFinishedRequestsTimer = new QTimer(this);
+    connect( m_recentlyFinishedRequestsTimer, SIGNAL( timeout() ),
+             this, SLOT( slotClearRecentlyFinishedRequests() ),
+             Qt::DirectConnection );
+    m_recentlyFinishedRequestsTimer->setInterval( 30000 );
 }
 
 
-void Nepomuk::MetadataMover::stop()
+Nepomuk::MetadataMover::~MetadataMover()
 {
-    QMutexLocker lock( &m_queueMutex );
-    m_stopped = true;
-    m_queueWaiter.wakeAll();
 }
 
 
 void Nepomuk::MetadataMover::moveFileMetadata( const KUrl& from, const KUrl& to )
 {
-    kDebug() << from << to;
+//    kDebug() << from << to;
     Q_ASSERT( !from.path().isEmpty() && from.path() != "/" );
     Q_ASSERT( !to.path().isEmpty() && to.path() != "/" );
-    m_queueMutex.lock();
+
+    QMutexLocker lock(&m_queueMutex);
+
     UpdateRequest req( from, to );
     if ( !m_updateQueue.contains( req ) &&
          !m_recentlyFinishedRequests.contains( req ) )
         m_updateQueue.enqueue( req );
-    m_queueMutex.unlock();
-    m_queueWaiter.wakeAll();
+
+    QTimer::singleShot(0, this, SLOT(slotStartUpdateTimer()));
 }
 
 
@@ -83,79 +85,68 @@ void Nepomuk::MetadataMover::removeFileMetadata( const KUrl& file )
 void Nepomuk::MetadataMover::removeFileMetadata( const KUrl::List& files )
 {
     kDebug() << files;
-    m_queueMutex.lock();
+    QMutexLocker lock(&m_queueMutex);
+
     foreach( const KUrl& file, files ) {
         UpdateRequest req( file );
         if ( !m_updateQueue.contains( req ) &&
              !m_recentlyFinishedRequests.contains( req ) )
             m_updateQueue.enqueue( req );
     }
-    m_queueMutex.unlock();
-    m_queueWaiter.wakeAll();
+
+    QTimer::singleShot(0, this, SLOT(slotStartUpdateTimer()));
 }
 
 
-// FIXME: somehow detect when the nepomuk storage service is down and wait for it to come up again (how about having methods for that in Nepomuk::Service?)
-void Nepomuk::MetadataMover::run()
+void Nepomuk::MetadataMover::slotWorkUpdateQueue()
 {
-    m_stopped = false;
-    while( !m_stopped ) {
-
-        // lock for initial iteration
-        m_queueMutex.lock();
-
-        // work the queue
-        while( !m_updateQueue.isEmpty() ) {
-            UpdateRequest updateRequest = m_updateQueue.dequeue();
-            m_recentlyFinishedRequests.insert( updateRequest );
-
-            // unlock after queue utilization
-            m_queueMutex.unlock();
+    // lock for initial iteration
+    QMutexLocker lock(&m_queueMutex);
 
-            kDebug() << "========================= handling" << updateRequest.source() << updateRequest.target();
+    // work the queue
+    if( !m_updateQueue.isEmpty() ) {
+        UpdateRequest updateRequest = m_updateQueue.dequeue();
+        m_recentlyFinishedRequests.insert( updateRequest );
 
-            // an empty second url means deletion
-            if( updateRequest.target().isEmpty() ) {
-                KUrl url = updateRequest.source();
-                removeMetadata( url );
-            }
-            else {
-                KUrl from = updateRequest.source();
-                KUrl to = updateRequest.target();
+        // unlock after queue utilization
+        lock.unlock();
 
-                // We do NOT get deleted messages for overwritten files! Thus, we
-                // have to remove all metadata for overwritten files first.
-                removeMetadata( to );
+//        kDebug() << "========================= handling" << updateRequest.source() << updateRequest.target();
 
-                // and finally update the old statements
-                updateMetadata( from, to );
-            }
+        // an empty second url means deletion
+        if( updateRequest.target().isEmpty() ) {
+            removeMetadata( updateRequest.source() );
+        }
+        else {
+            const KUrl from = updateRequest.source();
+            const KUrl to = updateRequest.target();
 
-            kDebug() << "========================= done with" << updateRequest.source() << updateRequest.target();
+            // We do NOT get deleted messages for overwritten files! Thus, we
+            // have to remove all metadata for overwritten files first.
+            removeMetadata( to );
 
-            // lock for next iteration
-            m_queueMutex.lock();
+            // and finally update the old statements
+            updateMetadata( from, to );
         }
 
-        // wait for more input
-        kDebug() << "Waiting...";
-        m_queueWaiter.wait( &m_queueMutex );
-        m_queueMutex.unlock();
-        kDebug() << "Woke up.";
+//        kDebug() << "========================= done with" << updateRequest.source() << updateRequest.target();
+    }
+    else {
+        kDebug() << "All update requests handled. Stopping timer.";
+        m_queueTimer->stop();
     }
 }
 
 
 void Nepomuk::MetadataMover::removeMetadata( const KUrl& url )
 {
-    kDebug() << url;
+//    kDebug() << url;
 
     if ( url.isEmpty() ) {
         kDebug() << "empty path. Looks like a bug somewhere...";
     }
     else {
         const bool isFolder = url.url().endsWith('/');
-        kDebug() << "removing metadata for file" << url;
         Nepomuk::removeResources(QList<QUrl>() << url);
 
         if( isFolder ) {
@@ -173,7 +164,6 @@ void Nepomuk::MetadataMover::removeMetadata( const KUrl& url )
                                                     "}" )
                                 .arg( Soprano::Node::resourceToN3( Nepomuk::Vocabulary::NIE::url() ),
                                         url.url(KUrl::AddTrailingSlash) );
-            kDebug() << query;
 
             //
             // We cannot use one big loop since our updateMetadata calls below can change the iterator
@@ -232,6 +222,23 @@ void Nepomuk::MetadataMover::slotClearRecentlyFinishedRequests()
             ++it;
         }
     }
+
+    if(m_recentlyFinishedRequests.isEmpty()) {
+        kDebug() << "No more old requests. Stopping timer.";
+        m_recentlyFinishedRequestsTimer->stop();
+    }
+}
+
+
+// start the timer in the update thread
+void Nepomuk::MetadataMover::slotStartUpdateTimer()
+{
+    if(!m_queueTimer->isActive()) {
+        m_queueTimer->start();
+    }
+    if(!m_recentlyFinishedRequestsTimer->isActive()) {
+        m_recentlyFinishedRequestsTimer->start();
+    }
 }
 
 #include "metadatamover.moc"
diff --git a/nepomuk/services/filewatch/metadatamover.h b/nepomuk/services/filewatch/metadatamover.h
index 9697abe..1e96dac 100644
--- a/nepomuk/services/filewatch/metadatamover.h
+++ b/nepomuk/services/filewatch/metadatamover.h
@@ -1,5 +1,5 @@
 /* This file is part of the KDE Project
-   Copyright (c) 2009 Sebastian Trueg <trueg@kde.org>
+   Copyright (c) 2009-2011 Sebastian Trueg <trueg@kde.org>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -19,9 +19,7 @@
 #ifndef _NEPOMUK_METADATA_MOVER_H_
 #define _NEPOMUK_METADATA_MOVER_H_
 
-#include <QtCore/QThread>
 #include <QtCore/QMutex>
-#include <QtCore/QWaitCondition>
 #include <QtCore/QQueue>
 #include <QtCore/QSet>
 #include <QtCore/QDateTime>
@@ -30,12 +28,14 @@
 
 #include "updaterequest.h"
 
+class QTimer;
+
 namespace Soprano {
     class Model;
 }
 
 namespace Nepomuk {
-    class MetadataMover : public QThread
+    class MetadataMover : public QObject
     {
         Q_OBJECT
 
@@ -48,8 +48,6 @@ namespace Nepomuk {
         void removeFileMetadata( const KUrl& file );
         void removeFileMetadata( const KUrl::List& files );
 
-        void stop();
-
     Q_SIGNALS:
         /**
          * Emitted for files (and folders) that have been moved but
@@ -62,10 +60,14 @@ namespace Nepomuk {
 
     private Q_SLOTS:
         void slotClearRecentlyFinishedRequests();
+        void slotWorkUpdateQueue();
 
-    private:
-        void run();
+        /**
+         * Start the update queue from the main thread.
+         */
+        void slotStartUpdateTimer();
 
+    private:
         /**
          * Remove the metadata for file \p url
          */
@@ -89,8 +91,9 @@ namespace Nepomuk {
         QSet<UpdateRequest> m_recentlyFinishedRequests;
 
         QMutex m_queueMutex;
-        QWaitCondition m_queueWaiter;
-        bool m_stopped;
+
+        QTimer* m_queueTimer;
+        QTimer* m_recentlyFinishedRequestsTimer;
 
         Soprano::Model* m_model;
     };
diff --git a/nepomuk/services/filewatch/nepomukfilewatch.cpp b/nepomuk/services/filewatch/nepomukfilewatch.cpp
index 3f038db..6d4e56e 100644
--- a/nepomuk/services/filewatch/nepomukfilewatch.cpp
+++ b/nepomuk/services/filewatch/nepomukfilewatch.cpp
@@ -1,5 +1,5 @@
 /* This file is part of the KDE Project
-   Copyright (c) 2007-2010 Sebastian Trueg <trueg@kde.org>
+   Copyright (c) 2007-2011 Sebastian Trueg <trueg@kde.org>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -130,11 +130,13 @@ Nepomuk::FileWatch::FileWatch( QObject* parent, const QList<QVariant>& )
     m_pathExcludeRegExpCache->rebuildCacheFromFilterList( defaultExcludeFilterList() );
 
     // start the mover thread
-    m_metadataMover = new MetadataMover( mainModel(), this );
+    m_metadataMoverThread = new QThread(this);
+    m_metadataMoverThread->start();
+    m_metadataMover = new MetadataMover( mainModel() );
     connect( m_metadataMover, SIGNAL(movedWithoutData(QString)),
              this, SLOT(slotMovedWithoutData(QString)),
              Qt::QueuedConnection );
-    m_metadataMover->start();
+    m_metadataMover->moveToThread(m_metadataMoverThread);
 
     m_fileModificationQueue = new ActiveFileQueue(this);
     connect(m_fileModificationQueue, SIGNAL(urlTimeout(KUrl)),
@@ -187,8 +189,8 @@ Nepomuk::FileWatch::FileWatch( QObject* parent, const QList<QVariant>& )
 Nepomuk::FileWatch::~FileWatch()
 {
     kDebug();
-    m_metadataMover->stop();
-    m_metadataMover->wait();
+    m_metadataMoverThread->quit();
+    m_metadataMoverThread->wait();
 }
 
 
@@ -207,11 +209,8 @@ void Nepomuk::FileWatch::watchFolder( const QString& path )
 void Nepomuk::FileWatch::slotFileMoved( const QString& urlFrom, const QString& urlTo )
 {
     if( !ignorePath( urlFrom ) || !ignorePath( urlTo ) ) {
-        KUrl from( urlFrom );
-        KUrl to( urlTo );
-
-        kDebug() << from << to;
-
+        const KUrl from( urlFrom );
+        const KUrl to( urlTo );
         m_metadataMover->moveFileMetadata( from, to );
     }
 }
diff --git a/nepomuk/services/filewatch/nepomukfilewatch.h b/nepomuk/services/filewatch/nepomukfilewatch.h
index a6f4e2c..fbebfb8 100644
--- a/nepomuk/services/filewatch/nepomukfilewatch.h
+++ b/nepomuk/services/filewatch/nepomukfilewatch.h
@@ -1,5 +1,5 @@
 /* This file is part of the KDE Project
-   Copyright (c) 2007-2010 Sebastian Trueg <trueg@kde.org>
+   Copyright (c) 2007-2011 Sebastian Trueg <trueg@kde.org>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -38,6 +38,7 @@ class KInotify;
 class KUrl;
 class RegExpCache;
 class ActiveFileQueue;
+class QThread;
 
 namespace Nepomuk {
 
@@ -108,6 +109,7 @@ namespace Nepomuk {
          */
         bool ignorePath( const QString& path );
 
+        QThread* m_metadataMoverThread;
         MetadataMover* m_metadataMover;
 
 #ifdef BUILD_KINOTIFY

Places

File 0f511184-kderuntime-nepomuk-47branch-memleak.diff of Package kdebase4-runtime

Places