[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [nepomuk-core/feature/resourcemanagercleanup] libnepomukcore/resource: Instead of doing MoveToThread
From:       Simeon Bird <bladud () gmail ! com>
Date:       2013-03-16 20:59:45
Message-ID: 20130316205945.36E76A60D4 () git ! kde ! org
[Download RAW message or body]

Git commit d97126070669cfe62e50a09c15d3bcf1d3a556ab by Simeon Bird.
Committed on 16/03/2013 at 21:27.
Pushed by sbird into branch 'feature/resourcemanagercleanup'.

Instead of doing MoveToThread, take the resourceManager mutex before
talking to the ResourceWatcher. This is a lot clearer and less racy.
It should not be too contended now the rm mutex is not held over the
socket communication.

M  +6    -0    libnepomukcore/resource/resourcedata.cpp
M  +17   -28   libnepomukcore/resource/resourcemanager.cpp

http://commits.kde.org/nepomuk-core/d97126070669cfe62e50a09c15d3bcf1d3a556ab

diff --git a/libnepomukcore/resource/resourcedata.cpp \
b/libnepomukcore/resource/resourcedata.cpp index 9d7abe1..0a00e19 100644
--- a/libnepomukcore/resource/resourcedata.cpp
+++ b/libnepomukcore/resource/resourcedata.cpp
@@ -318,7 +318,10 @@ bool Nepomuk2::ResourceData::store()
 void Nepomuk2::ResourceData::addToWatcher()
 {
     if( m_watchEnabled && !m_addedToWatcher ) {
+        //Obey the locking rules: the rm mutex gets locked before the dataMutex.
+        m_dataMutex.unlock();
         m_rm->addToWatcher( m_uri );
+        m_dataMutex.lock();
         m_addedToWatcher = true;
     }
 }
@@ -326,7 +329,10 @@ void Nepomuk2::ResourceData::addToWatcher()
 void Nepomuk2::ResourceData::removeFromWatcher()
 {
     if( m_addedToWatcher ) {
+        //Obey the locking rules: the rm mutex gets locked before the dataMutex.
+        m_dataMutex.unlock();
         m_rm->removeFromWatcher( m_uri );
+        m_dataMutex.lock();
         m_addedToWatcher = false;
     }
 }
diff --git a/libnepomukcore/resource/resourcemanager.cpp \
b/libnepomukcore/resource/resourcemanager.cpp index 83f21d9..f8d7ab0 100644
--- a/libnepomukcore/resource/resourcemanager.cpp
+++ b/libnepomukcore/resource/resourcemanager.cpp
@@ -409,48 +409,37 @@ void Nepomuk2::ResourceManagerPrivate::addToWatcher( const \
QUrl& uri )  {
     if( uri.isEmpty() )
         return;
-    // Lock the init mutex to make sure we don't create multiple watchers
-    initMutex.lockForRead();
+    // Lock the mutex, because the ResourceWatcher is not thread-safe.
+    // This should have a small impact, because the calling function in ResourceData \
will have +    // to take the mutex anyway.
+    QMutexLocker lock( &mutex );
     if( !m_watcher ) {
-        // Lock the initMutex for write
-        initMutex.unlock();
-        initMutex.lockForWrite();
-        // check that nothing stole in and created
-        // a ResourceWatcher while we were unlocked.
-        if( !m_watcher ) {
-          m_watcher = new ResourceWatcher(m_manager);
-          //
-          // The ResourceWatcher is not thread-safe. Thus, we need to ensure the \
                safety ourselves.
-          // We do that by simply handling all RW related operations in the manager \
                thread.
-          // This also means to invoke methods on the watcher through QMetaObject to \
                make sure they
-          // get queued in case of calls between different threads.
-          //
-          m_watcher->moveToThread(m_manager->thread());
-          QObject::connect( m_watcher, SIGNAL(propertyAdded(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)),
-                          m_manager, SLOT(slotPropertyAdded(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)) );
-          QObject::connect( m_watcher, SIGNAL(propertyRemoved(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)),
-                          m_manager, SLOT(slotPropertyRemoved(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)) );
-        m_watcher->addResource( uri );
-    }
-    else {
-        QMetaObject::invokeMethod( m_watcher, "addResource", Qt::AutoConnection, \
Q_ARG(QUrl, uri) ); +        m_watcher = new ResourceWatcher(m_manager);
+        QObject::connect( m_watcher, SIGNAL(propertyAdded(Nepomuk2::Resource, \
Nepomuk2::Types::Property, QVariant)), +                  m_manager, \
SLOT(slotPropertyAdded(Nepomuk2::Resource, Nepomuk2::Types::Property, QVariant)) ); + \
QObject::connect( m_watcher, SIGNAL(propertyRemoved(Nepomuk2::Resource, \
Nepomuk2::Types::Property, QVariant)), +                  m_manager, \
SLOT(slotPropertyRemoved(Nepomuk2::Resource, Nepomuk2::Types::Property, QVariant)) ); \
} +    m_watcher->addResource( uri );
     // (re-)start the watcher in case this resource is the only one in the list of \
watched  if( m_watcher->resourceCount() <= 1 ) {
-        QMetaObject::invokeMethod(m_watcher, "start", Qt::AutoConnection);
+        m_watcher->start();
     }
 }
 
 void Nepomuk2::ResourceManagerPrivate::removeFromWatcher( const QUrl& uri )
 {
-    if( uri.isEmpty() || !m_watcher )
+    if( uri.isEmpty())
         return;
 
-    m_watcher->removeResource( uri );
+    QMutexLocker lock( &mutex );
+    if( !m_watcher )
+        return;
 
+    m_watcher->removeResource( uri );
     // stop the watcher since we do not want to watch all changes in case there is \
no ResourceData left  if( !m_watcher->resourceCount() ) {
-        QMetaObject::invokeMethod(m_watcher, "stop", Qt::AutoConnection);
+        m_watcher->stop();
     }
 }
 


[prev in list] [next in list] [prev in thread] [next in thread]