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

List:       kde-commits
Subject:    Re: [nepomuk-core/feature/resourcemanagercleanup] libnepomukcore: ResourceManager/ResourceWatcher: M
From:       Vishesh Handa <handa.vish () gmail ! com>
Date:       2013-03-15 9:22:32
Message-ID: CAOPTMKBjRj-p9E6svxtL4hrMQL=mOKWC-YE=Ac1xd5HX8dcdrw () mail ! gmail ! com
[Download RAW message or body]

On Fri, Mar 15, 2013 at 8:33 AM, Simeon Bird <bladud@gmail.com> wrote:

> Git commit cf6a1316df282c3c7e99d952e9fc351498d5f9a4 by Simeon Bird.
> Committed on 12/03/2013 at 04:38.
> Pushed by sbird into branch 'feature/resourcemanagercleanup'.
>
> ResourceManager/ResourceWatcher: Make removeResource and addResource
> happen in the ResourceWatcher thread via a QAutoConnection, avoiding
> the thread unsafety of QDBusConnection and thus the crash.
> Add convenience functions to the ResourceWatcher that start and stop it
> if necessary, so that we don't have to jump back and forth between
> threads.
>
> BUG: 305024
> FIXED-IN: 4.10.2
>

I'm not too sure if I like the idea of these added public functions. If
they were just for this resource manager, then it would be okay.

Can't we just call both the functions one after another via
QMetaObject::invokeMethod? There is obviously the call of resourceCount,
which we can just assume has been incremented on our own.


> M  +16   -0    libnepomukcore/datamanagement/resourcewatcher.cpp
> M  +20   -0    libnepomukcore/datamanagement/resourcewatcher.h
> M  +4    -14   libnepomukcore/resource/resourcemanager.cpp
>
>
> http://commits.kde.org/nepomuk-core/cf6a1316df282c3c7e99d952e9fc351498d5f9a4
>
> diff --git a/libnepomukcore/datamanagement/resourcewatcher.cpp
> b/libnepomukcore/datamanagement/resourcewatcher.cpp
> index f394ae8..d01b8a3 100644
> --- a/libnepomukcore/datamanagement/resourcewatcher.cpp
> +++ b/libnepomukcore/datamanagement/resourcewatcher.cpp
> @@ -169,6 +169,13 @@ void Nepomuk2::ResourceWatcher::addResource(const
> QUrl& resUri)
>      }
>  }
>
> +void Nepomuk2::ResourceWatcher::addResourceStart(const QUrl& resUri)
> +{
> +    addResource(resUri);
> +    if(resourceCount() <=1){
> +        start();
> +    }
> +}
>
>  void Nepomuk2::ResourceWatcher::addType(const Nepomuk2::Types::Class&
> type)
>  {
> @@ -199,6 +206,15 @@ void Nepomuk2::ResourceWatcher::removeResource(const
> QUrl& resUri)
>      }
>  }
>
> +void Nepomuk2::ResourceWatcher::removeResourceStop(const QUrl& resUri)
> +{
> +    removeResource(resUri);
> +    if(!resourceCount()){
> +        stop();
> +    }
> +}
> +
> +
>  void Nepomuk2::ResourceWatcher::removeType(const Nepomuk2::Types::Class&
> type)
>  {
>      d->m_types.removeAll(type.uri());
> diff --git a/libnepomukcore/datamanagement/resourcewatcher.h
> b/libnepomukcore/datamanagement/resourcewatcher.h
> index 05e3710..909fa0d 100644
> --- a/libnepomukcore/datamanagement/resourcewatcher.h
> +++ b/libnepomukcore/datamanagement/resourcewatcher.h
> @@ -126,6 +126,16 @@ namespace Nepomuk2 {
>          void addResource( const QUrl& resUri );
>
>          /**
> +         * \brief Add a resource to be watched and maybe stop.
> +         *
> +         * As addResource, but if the added resource is the last first
> one,
> +         * (re)-start the watcher.
> +         *
> +         * \sa addResource()
> +         */
> +        void addResourceStart(const QUrl& resUri);
> +
> +        /**
>           * \brief Add a property to be watched.
>           *
>           * Every change to a value of this property
> @@ -165,6 +175,16 @@ namespace Nepomuk2 {
>          void removeResource( const QUrl& resUri );
>
>          /**
> +         * \brief Remove a resource to be watched and maybe stop.
> +         *
> +         * As removeResource, but if the removed resource is the last one,
> +         * stop the watcher.
> +         *
> +         * \sa removeResource()
> +         */
> +        void removeResourceStop(const QUrl& resUri);
> +
> +        /**
>           * \brief Remove a property to be watched.
>           *
>           * Every change to a value of this property
> diff --git a/libnepomukcore/resource/resourcemanager.cpp
> b/libnepomukcore/resource/resourcemanager.cpp
> index 2b32be0..03b7b58 100644
> --- a/libnepomukcore/resource/resourcemanager.cpp
> +++ b/libnepomukcore/resource/resourcemanager.cpp
> @@ -423,15 +423,9 @@ void Nepomuk2::ResourceManagerPrivate::addToWatcher(
> const QUrl& uri )
>                            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) );
> -    }
> -    // (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);
>      }
> +    // add a resource and (re-)start the watcher in case this resource is
> the only one in the list of watched
> +    QMetaObject::invokeMethod( m_watcher, "addResourceStart",
> Qt::AutoConnection, Q_ARG(QUrl, uri) );
>  }
>
>  void Nepomuk2::ResourceManagerPrivate::removeFromWatcher( const QUrl& uri
> )
> @@ -439,12 +433,8 @@ void
> Nepomuk2::ResourceManagerPrivate::removeFromWatcher( const QUrl& uri )
>      if( uri.isEmpty() || !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);
> -    }
> +    // remove a resource and stop the watcher since we do not want to
> watch all changes in case there is no ResourceData left
> +    QMetaObject::invokeMethod( m_watcher, "removeResourceStop",
> Qt::AutoConnection, Q_ARG(QUrl, uri) );
>  }
>
>  #include "resourcemanager.moc"
>
>

[Attachment #3 (text/html)]

<br><br><div class="gmail_quote">On Fri, Mar 15, 2013 at 8:33 AM, Simeon Bird <span \
dir="ltr">&lt;<a href="mailto:bladud@gmail.com" \
target="_blank">bladud@gmail.com</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Git commit cf6a1316df282c3c7e99d952e9fc351498d5f9a4 by \
Simeon Bird.<br> Committed on 12/03/2013 at 04:38.<br>
Pushed by sbird into branch &#39;feature/resourcemanagercleanup&#39;.<br>
<br>
ResourceManager/ResourceWatcher: Make removeResource and addResource<br>
happen in the ResourceWatcher thread via a QAutoConnection, avoiding<br>
the thread unsafety of QDBusConnection and thus the crash.<br>
Add convenience functions to the ResourceWatcher that start and stop it<br>
if necessary, so that we don&#39;t have to jump back and forth between<br>
threads.<br>
<br>
BUG: 305024<br>
FIXED-IN: 4.10.2<br></blockquote><div><br>I&#39;m not too sure if I like the idea of \
these added public functions. If they were just for this resource manager, then it \
would be okay.<br><br>Can&#39;t we just call both the functions one after another via \
QMetaObject::invokeMethod? There is obviously the call of resourceCount, which we can \
just assume has been incremented on our own.<br> <br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
M  +16   -0    libnepomukcore/datamanagement/resourcewatcher.cpp<br>
M  +20   -0    libnepomukcore/datamanagement/resourcewatcher.h<br>
M  +4    -14   libnepomukcore/resource/resourcemanager.cpp<br>
<br>
<a href="http://commits.kde.org/nepomuk-core/cf6a1316df282c3c7e99d952e9fc351498d5f9a4" \
target="_blank">http://commits.kde.org/nepomuk-core/cf6a1316df282c3c7e99d952e9fc351498d5f9a4</a><br>
 <br>
diff --git a/libnepomukcore/datamanagement/resourcewatcher.cpp \
b/libnepomukcore/datamanagement/resourcewatcher.cpp<br> index f394ae8..d01b8a3 \
                100644<br>
--- a/libnepomukcore/datamanagement/resourcewatcher.cpp<br>
+++ b/libnepomukcore/datamanagement/resourcewatcher.cpp<br>
@@ -169,6 +169,13 @@ void Nepomuk2::ResourceWatcher::addResource(const QUrl&amp; \
resUri)<br>  }<br>
 }<br>
<br>
+void Nepomuk2::ResourceWatcher::addResourceStart(const QUrl&amp; resUri)<br>
+{<br>
+    addResource(resUri);<br>
+    if(resourceCount() &lt;=1){<br>
+        start();<br>
+    }<br>
+}<br>
<br>
 void Nepomuk2::ResourceWatcher::addType(const Nepomuk2::Types::Class&amp; type)<br>
 {<br>
@@ -199,6 +206,15 @@ void Nepomuk2::ResourceWatcher::removeResource(const QUrl&amp; \
resUri)<br>  }<br>
 }<br>
<br>
+void Nepomuk2::ResourceWatcher::removeResourceStop(const QUrl&amp; resUri)<br>
+{<br>
+    removeResource(resUri);<br>
+    if(!resourceCount()){<br>
+        stop();<br>
+    }<br>
+}<br>
+<br>
+<br>
 void Nepomuk2::ResourceWatcher::removeType(const Nepomuk2::Types::Class&amp; \
type)<br>  {<br>
     d-&gt;m_types.removeAll(type.uri());<br>
diff --git a/libnepomukcore/datamanagement/resourcewatcher.h \
b/libnepomukcore/datamanagement/resourcewatcher.h<br> index 05e3710..909fa0d \
                100644<br>
--- a/libnepomukcore/datamanagement/resourcewatcher.h<br>
+++ b/libnepomukcore/datamanagement/resourcewatcher.h<br>
@@ -126,6 +126,16 @@ namespace Nepomuk2 {<br>
         void addResource( const QUrl&amp; resUri );<br>
<br>
         /**<br>
+         * \brief Add a resource to be watched and maybe stop.<br>
+         *<br>
+         * As addResource, but if the added resource is the last first one,<br>
+         * (re)-start the watcher.<br>
+         *<br>
+         * \sa addResource()<br>
+         */<br>
+        void addResourceStart(const QUrl&amp; resUri);<br>
+<br>
+        /**<br>
          * \brief Add a property to be watched.<br>
          *<br>
          * Every change to a value of this property<br>
@@ -165,6 +175,16 @@ namespace Nepomuk2 {<br>
         void removeResource( const QUrl&amp; resUri );<br>
<br>
         /**<br>
+         * \brief Remove a resource to be watched and maybe stop.<br>
+         *<br>
+         * As removeResource, but if the removed resource is the last one,<br>
+         * stop the watcher.<br>
+         *<br>
+         * \sa removeResource()<br>
+         */<br>
+        void removeResourceStop(const QUrl&amp; resUri);<br>
+<br>
+        /**<br>
          * \brief Remove a property to be watched.<br>
          *<br>
          * Every change to a value of this property<br>
diff --git a/libnepomukcore/resource/resourcemanager.cpp \
b/libnepomukcore/resource/resourcemanager.cpp<br> index 2b32be0..03b7b58 100644<br>
--- a/libnepomukcore/resource/resourcemanager.cpp<br>
+++ b/libnepomukcore/resource/resourcemanager.cpp<br>
@@ -423,15 +423,9 @@ void Nepomuk2::ResourceManagerPrivate::addToWatcher( const \
                QUrl&amp; uri )<br>
                           m_manager, SLOT(slotPropertyAdded(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)) );<br>
         QObject::connect( m_watcher, SIGNAL(propertyRemoved(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)),<br>
                           m_manager, SLOT(slotPropertyRemoved(Nepomuk2::Resource, \
                Nepomuk2::Types::Property, QVariant)) );<br>
-        m_watcher-&gt;addResource( uri );<br>
-    }<br>
-    else {<br>
-        QMetaObject::invokeMethod( m_watcher, &quot;addResource&quot;, \
                Qt::AutoConnection, Q_ARG(QUrl, uri) );<br>
-    }<br>
-    // (re-)start the watcher in case this resource is the only one in the list of \
                watched<br>
-    if( m_watcher-&gt;resourceCount() &lt;= 1 ) {<br>
-        QMetaObject::invokeMethod(m_watcher, &quot;start&quot;, \
Qt::AutoConnection);<br>  }<br>
+    // add a resource and (re-)start the watcher in case this resource is the only \
one in the list of watched<br> +    QMetaObject::invokeMethod( m_watcher, \
&quot;addResourceStart&quot;, Qt::AutoConnection, Q_ARG(QUrl, uri) );<br>  }<br>
<br>
 void Nepomuk2::ResourceManagerPrivate::removeFromWatcher( const QUrl&amp; uri )<br>
@@ -439,12 +433,8 @@ void Nepomuk2::ResourceManagerPrivate::removeFromWatcher( const \
QUrl&amp; uri )<br>  if( uri.isEmpty() || !m_watcher )<br>
         return;<br>
<br>
-    m_watcher-&gt;removeResource( uri );<br>
-<br>
-    // stop the watcher since we do not want to watch all changes in case there is \
                no ResourceData left<br>
-    if( !m_watcher-&gt;resourceCount() ) {<br>
-        QMetaObject::invokeMethod(m_watcher, &quot;stop&quot;, \
                Qt::AutoConnection);<br>
-    }<br>
+    // remove a resource and stop the watcher since we do not want to watch all \
changes in case there is no ResourceData left<br> +    QMetaObject::invokeMethod( \
m_watcher, &quot;removeResourceStop&quot;, Qt::AutoConnection, Q_ARG(QUrl, uri) \
);<br>  }<br>
<br>
 #include &quot;resourcemanager.moc&quot;<br>
<br>
</blockquote></div><br>



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

Configure | About | News | Add a list | Sponsored by KoreLogic