I accidentally replied to Sebastian instead of the nepomuk mailing list. So
I'm copy-pasting the conversation we had. I've written more stuff in the
end.
On Sun, Apr 11, 2010 at 6:29 PM, Vishesh Handa <[email protected]> wrote:
>
> On Sat, Apr 10, 2010 at 1:59 PM, Sebastian Trüg <[email protected]> wrote:
>
>> One comment though: for empty strings you should rather use QString()
>> than QLatin1String("").
>>
>
> Thanks for the heads up!
>
> I tested the query service, and it works perfectly, BUT it only works with
> QString() and not with QLatin1String(""). I want to know what the difference
> is, so I'll investigate more when I have the time!
>
> The kded patch works according to your instructions, but I'm still not
> totally convinced! :-/ For one it gave this output when I SIGTERM or KILL it
> -
> [/home/vishesh/kde/bin/nepomukservicestub]
> nepomukstrigiservice(5970)/nepomuk (strigi service)
> Nepomuk::IndexScheduler::setIndexingSpeed: 1 ReducedSpeed
> [/home/vishesh/kde/bin/nepomukservicestub]
> nepomukqueryservice(5972)/nepomuk (query service)
> Nepomuk::Query::FolderConnection::close:
> nepomukqueryservice(5972)/nepomuk (query service)
> Nepomuk::Query::Folder::removeConnection: Folder unused. Deleting.
> nepomukqueryservice(5972)/nepomuk (query service)
> Nepomuk::Query::QueryService::slotFolderConnectionDestroyed:
> QObject(0x984dff0)
> nepomukqueryservice(5972)/nepomuk (query service)
> Nepomuk::Query::QueryService::slotFolderDestroyed: QObject(0x97e8330)
>
> and when I tried to manually kill the normal *kded4* module and start the
> trunk kded4, the DBus kded /modules/nepomuksearchmodule wasn't registered,
> and I couldn't check by adding debug statements. If you feel it's okay,
> please commit the patch. However I would like to run more tests (just to be
> sure!)
>
> When I test my code I generally add kDebug() statements, and remove them
> when creating a patch. Should I remove them or keep them, in future patches?
>
> Thanks
>
> - Vishesh Handa
>
>
On Mon, Apr 12, 2010 at 1:16 AM, Sebastian Trüg <[email protected]> wrote:
> Hi Vishesh,
>
> On 04/11/2010 02:59 PM, Vishesh Handa wrote:
> > I tested the query service, and it works perfectly, BUT it only works
> > with QString() and not with QLatin1String(""). I want to know what the
> > difference is, so I'll investigate more when I have the time!
>
> Hm, the only difference I can see is that maybe QString() gives a null
> string (QString::isNull()) while QLatin1String("") only gives an empty
> one but non-null.
>
> > The kded patch works according to your instructions, but I'm still not
> > totally convinced! :-/ For one it gave this output when I SIGTERM or
> > KILL it -
> > [/home/vishesh/kde/bin/nepomukservicestub]
> > nepomukstrigiservice(5970)/nepomuk (strigi service)
> > Nepomuk::IndexScheduler::setIndexingSpeed: 1 ReducedSpeed
> > [/home/vishesh/kde/bin/nepomukservicestub]
> > nepomukqueryservice(5972)/nepomuk (query service)
> > Nepomuk::Query::FolderConnection::close:
> > nepomukqueryservice(5972)/nepomuk (query service)
> > Nepomuk::Query::Folder::removeConnection: Folder unused. Deleting.
> > nepomukqueryservice(5972)/nepomuk (query service)
> > Nepomuk::Query::QueryService::slotFolderConnectionDestroyed:
> > QObject(0x984dff0)
> > nepomukqueryservice(5972)/nepomuk (query service)
> > Nepomuk::Query::QueryService::slotFolderDestroyed: QObject(0x97e8330)
>
> This is normal output from the query service. As soon as the search kded
> module goes down all the searches it was watching (to emit KDirNotify
> signals in case search results change) are unregistered from the query
> service.
>
> > and when I tried to manually kill the normal */kded4/* module and start
> > the trunk kded4, the DBus kded /modules/nepomuksearchmodule wasn't
> > registered, and I couldn't check by adding debug statements. If you feel
> > it's okay, please commit the patch. However I would like to run more
> > tests (just to be sure!)
>
> This is because kded4 has 2 initialization steps. Normally startkde
> takes care of calling the second one. But if you start it manually you
> also need to start the second phase manually:
>
> qdbus org.kde.kded /kded org.kde.kded.loadSecondPhase
>
> > When I test my code I generally add kDebug() statements, and remove them
> > when creating a patch. Should I remove them or keep them, in future
> patches?
>
> This is not easy to answer. I personally try to keep as many of them as
> possible for easy debugging at later stages. But having too many debug
> outputs can flood the terminal and even slow down apps. So in the end it
> is a judgment call and needs to be decided on a case-by-case basis.
>
> Cheers,
> Sebastian
>
I've tested it out and it works fine. However I changed the code a little
bit. I've added the *m_watcher->removeWatchedService( message().service() );
* to the slotServiceUnregistered function instead of the *
unregisterSearchUrl* function. It seems better this way.
Please commit the patches. :-) And, Sebastian, thanks for excellent
instructions! I'll do the rest later (I will do them! I call dibs.)
Thanks
- Vishesh Handa
Index: nepomuksearchmodule.h
===================================================================
--- nepomuksearchmodule.h (revision 1113100)
+++ nepomuksearchmodule.h (working copy)
@@ -26,6 +26,8 @@
#include <QtDBus/QDBusContext>
#include <QtCore/QMultiHash>
+class QDBusServiceWatcher;
+
namespace Nepomuk {
class SearchUrlListener;
@@ -45,15 +47,15 @@
Q_SCRIPTABLE QStringList watchedSearchUrls();
private Q_SLOTS:
- void slotServiceOwnerChanged( const QString& serviceName,
- const QString&,
- const QString& newOwner );
+ void slotServiceUnregistered( const QString& serviceName );
private:
void unrefUrl( const KUrl& url );
QHash<KUrl, SearchUrlListener*> m_queryHash;
QMultiHash<QString, KUrl> m_dbusServiceUrlHash;
+
+ QDBusServiceWatcher *m_watcher;
};
}
Index: nepomuksearchmodule.cpp
===================================================================
--- nepomuksearchmodule.cpp (revision 1113100)
+++ nepomuksearchmodule.cpp (working copy)
@@ -23,6 +23,7 @@
#include "dbusoperators_p.h"
#include <QtDBus/QDBusConnection>
+#include <QtDBus/QDBusServiceWatcher>
#include <kdebug.h>
#include <kdirnotify.h>
@@ -47,10 +48,12 @@
// connect to serviceOwnerChanged to catch crashed clients that never unregistered
// themselves
//
- connect( QDBusConnection::sessionBus().interface(),
- SIGNAL( serviceOwnerChanged( const QString&, const QString&, const QString& ) ),
- this,
- SLOT( slotServiceOwnerChanged( const QString&, const QString&, const QString& ) ) );
+ m_watcher = new QDBusServiceWatcher( QString(),
+ QDBusConnection::sessionBus(),
+ QDBusServiceWatcher::WatchForUnregistration,
+ this );
+ connect( m_watcher, SIGNAL( serviceUnregistered( const QString& ) ),
+ this, SLOT( slotServiceUnregistered( const QString& ) ) );
//
// connect to KDirLister telling us that it entered a dir
@@ -93,8 +96,10 @@
it.value()->ref();
}
- if ( calledFromDBus() )
+ if ( calledFromDBus() ) {
m_dbusServiceUrlHash.insert( message().service(), url );
+ m_watcher->addWatchedService( message().service() );
+ }
}
}
@@ -105,8 +110,9 @@
if ( isNepomukSearchUrl( url ) ) {
kDebug() << "UNREGISTER UNREGISTER UNREGISTER UNREGISTER UNREGISTER" << url;
unrefUrl( url );
- if ( calledFromDBus() )
+ if ( calledFromDBus() ) {
m_dbusServiceUrlHash.remove( message().service(), url );
+ }
}
}
@@ -117,18 +123,15 @@
}
-void Nepomuk::SearchModule::slotServiceOwnerChanged( const QString& serviceName,
- const QString&,
- const QString& newOwner )
+void Nepomuk::SearchModule::slotServiceUnregistered( const QString& serviceName )
{
- if ( newOwner.isEmpty() ) {
- QHash<QString, KUrl>::iterator it = m_dbusServiceUrlHash.find( serviceName );
- while ( it != m_dbusServiceUrlHash.end() ) {
- unrefUrl( it.value() );
- m_dbusServiceUrlHash.erase( it );
- it = m_dbusServiceUrlHash.find( serviceName );
- }
+ QHash<QString, KUrl>::iterator it = m_dbusServiceUrlHash.find( serviceName );
+ while ( it != m_dbusServiceUrlHash.end() ) {
+ unrefUrl( it.value() );
+ m_dbusServiceUrlHash.erase( it );
+ it = m_dbusServiceUrlHash.find( serviceName );
}
+ m_watcher->removeWatchedService( message().service() );
}
Index: queryservice.cpp
===================================================================
--- queryservice.cpp (revision 1113100)
+++ queryservice.cpp (working copy)
@@ -26,6 +26,7 @@
#include <QtDBus/QDBusConnectionInterface>
#include <QtDBus/QDBusObjectPath>
#include <QtDBus/QDBusMessage>
+#include <QtDBus/QDBusServiceWatcher>
#include <KPluginFactory>
#include <KUrl>
@@ -50,10 +51,12 @@
s_instance = this;
- connect( QDBusConnection::sessionBus().interface(),
- SIGNAL( serviceOwnerChanged( const QString&, const QString&, const QString& ) ),
- this,
- SLOT( slotServiceOwnerChanged( const QString&, const QString&, const QString& ) ) );
+ m_serviceWatcher = new QDBusServiceWatcher( QString(),
+ QDBusConnection::sessionBus(),
+ QDBusServiceWatcher::WatchForUnregistration, this);
+
+ connect( m_serviceWatcher, SIGNAL( serviceUnregistered(const QString& ) ),
+ this, SLOT( slotServiceUnregistered( const QString& ) ) );
}
@@ -102,6 +105,7 @@
QString dbusClient = msg.service();
m_openConnections.insert( dbusClient, conn );
m_connectionDBusServiceHash.insert( conn, dbusClient );
+ m_serviceWatcher->addWatchedService( dbusClient );
return QDBusObjectPath( dbusObjectPath );
}
@@ -155,18 +159,15 @@
}
-void Nepomuk::Query::QueryService::slotServiceOwnerChanged( const QString& serviceName,
- const QString&,
- const QString& newOwner )
+void Nepomuk::Query::QueryService::slotServiceUnregistered( const QString& serviceName )
{
- if ( newOwner.isEmpty() ) {
- QList<FolderConnection*> conns = m_openConnections.values( serviceName );
- if ( !conns.isEmpty() ) {
- kDebug() << "Service" << serviceName << "went down. Removing connections";
- // hash cleanup will be triggered automatically
- qDeleteAll( conns );
- }
+ QList<FolderConnection*> conns = m_openConnections.values( serviceName );
+ if ( !conns.isEmpty() ) {
+ kDebug() << "Service" << serviceName << "went down. Removing connections";
+ // hash cleanup will be triggered automatically
+ qDeleteAll( conns );
}
+ m_serviceWatcher->removeWatchedService(serviceName);
}
#include "queryservice.moc"
Index: queryservice.h
===================================================================
--- queryservice.h (revision 1113100)
+++ queryservice.h (working copy)
@@ -30,6 +30,7 @@
class QDBusObjectPath;
class QDBusMessage;
+class QDBusServiceWatcher;
namespace Nepomuk {
namespace Query {
@@ -62,9 +63,7 @@
Q_SCRIPTABLE QDBusObjectPath sparqlQuery( const QString& query, const RequestPropertyMapDBus& requestProps, const QDBusMessage& msg );
private Q_SLOTS:
- void slotServiceOwnerChanged( const QString& serviceName,
- const QString&,
- const QString& newOwner );
+ void slotServiceUnregistered( const QString& serviceName );
void slotFolderDestroyed( QObject* folder );
void slotFolderConnectionDestroyed( QObject* conn );
@@ -82,6 +81,7 @@
QHash<FolderConnection*, QString> m_connectionDBusServiceHash; // maps connections to their using dbus service
int m_folderConnectionCnt; // only used for unique dbus object path generation
+ QDBusServiceWatcher *m_serviceWatcher;
};
}
}
_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk
