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

List:       kde-commits
Subject:    [nepomuk-core/feature/resourcemanagercleanup] libnepomukcore/resource: Rework determineUri so that t
From:       Simeon Bird <bladud () gmail ! com>
Date:       2013-03-15 3:03:23
Message-ID: 20130315030323.0B00BA60EE () git ! kde ! org
[Download RAW message or body]

Git commit 1f779b653a1d623b4113385589726ab3fdd49de3 by Simeon Bird, on behalf of David Faure.
Committed on 14/03/2013 at 14:27.
Pushed by sbird into branch 'feature/resourcemanagercleanup'.

Rework determineUri so that the RM lock isn't held during the executeQuery

M  +2    -17   libnepomukcore/resource/resource.cpp
M  +15   -6    libnepomukcore/resource/resourcedata.cpp
M  +2    -6    libnepomukcore/resource/resourcedata.h

http://commits.kde.org/nepomuk-core/1f779b653a1d623b4113385589726ab3fdd49de3

diff --git a/libnepomukcore/resource/resource.cpp b/libnepomukcore/resource/resource.cpp
index 7158802..f019b6a 100644
--- a/libnepomukcore/resource/resource.cpp
+++ b/libnepomukcore/resource/resource.cpp
@@ -700,25 +700,10 @@ void Nepomuk2::Resource::determineFinalResourceData() const
         return;
     }
 
-    QMutexLocker lock( &m_data->rm()->mutex );
-
     // Get an initialized ResourceData instance
     ResourceData* oldData = m_data;
-    ResourceData* newData = m_data->determineUri();
-
-    Q_ASSERT(oldData);
-    Q_ASSERT(newData);
-
-    // in case we get an already existing one we update all instances
-    // using the old ResourceData to avoid the overhead of calling
-    // determineUri over and over
-    if( newData != oldData ) {
-        Q_FOREACH( Resource* res, m_data->resources() ) {
-            res->m_data = newData; // one of these resources is "this", so this updates our m_data.
-            oldData->deref( res );
-            newData->ref( res );
-        }
-    }
+
+    m_data->determineUri(); // note that this can change the value of m_data
 
     if ( !oldData->cnt() )
         delete oldData;
diff --git a/libnepomukcore/resource/resourcedata.cpp b/libnepomukcore/resource/resourcedata.cpp
index 067c537..1eb371f 100644
--- a/libnepomukcore/resource/resourcedata.cpp
+++ b/libnepomukcore/resource/resourcedata.cpp
@@ -565,12 +565,11 @@ bool Nepomuk2::ResourceData::isValid() const
     return !m_uri.isEmpty() || !m_nieUrl.isEmpty() || !m_naoIdentifier.isEmpty();
 }
 
-// Caller must hold the ResourceManager mutex (since the RM owns the returned ResourceData pointer)
-Nepomuk2::ResourceData* Nepomuk2::ResourceData::determineUri()
+void Nepomuk2::ResourceData::determineUri()
 {
     QMutexLocker lock(&m_dataMutex);
     if( !m_uri.isEmpty() ) {
-        return this;
+        return;
     }
 
     // We have the following possible situations:
@@ -642,17 +641,27 @@ Nepomuk2::ResourceData* Nepomuk2::ResourceData::determineUri()
     // Move us to the final data hash now that the URI is known
     //
     if( !m_uri.isEmpty() ) {
+        lock.unlock(); // respect mutex order
+        QMutexLocker locker(&m_rm->mutex);
+        lock.relock();
         m_cacheDirty = true;
         ResourceDataHash::iterator it = m_rm->m_initializedData.find(m_uri);
         if( it == m_rm->m_initializedData.end() ) {
             m_rm->m_initializedData.insert( m_uri, this );
         }
         else {
-            return it.value();
+            ResourceData* foundData = it.value();
+
+            // in case we get an already existing one we update all instances
+            // using the old ResourceData to avoid the overhead of calling
+            // determineUri over and over
+            Q_FOREACH (Resource* res, m_resources) {
+                res->m_data = foundData; // this can include our caller
+                this->deref( res );
+                foundData->ref( res );
+            }
         }
     }
-
-    return this;
 }
 
 
diff --git a/libnepomukcore/resource/resourcedata.h b/libnepomukcore/resource/resourcedata.h
index 432653c..69ef6fd 100644
--- a/libnepomukcore/resource/resourcedata.h
+++ b/libnepomukcore/resource/resourcedata.h
@@ -138,13 +138,9 @@ namespace Nepomuk2 {
          * and add m_data into ResourceManagerPrivate::m_initializedData
          * or it will find another ResourceData instance in m_initializedData
          * which represents the same resource. The ResourceData that should be
-         * used is returned.
-         *
-         * \returns The initialized ResourceData object representing the actual resource.
-         *
-         * The resource manager mutex needs to be locked before calling this method
+         * used is set in all the associated resources.
          */
-        ResourceData* determineUri();
+        void determineUri();
 
         void invalidateCache();
 
[prev in list] [next in list] [prev in thread] [next in thread]