From a5542751bc147c7f74131006339b55e6339b3e9f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20Vr=C3=A1til?= <dvratil@redhat.com>
Date: Thu, 17 Apr 2014 15:11:57 +0200
Subject: [PATCH 1/7] Use per-thread QDBusConnections
This moves DBusConnectionPool from Nepomuk search code to Akonadi and makes
use of it in search infrastructure and couple other classes that interact
with DBus from non-main thread.
QDBusConnection is not thread-safe in Qt 4, so we need to workaround it by
having a connection for each thread. Qt 5 should be OK, so we can remove this
in Frameworks.
This should fix random crashes I've been seeing when SearchTaskManager::addTask()
was called from multiple threads simultaneously.
---
server/CMakeLists.txt | 2 +-
server/src/dbusconnectionpool.cpp | 62 +++++++++++++++++++++++++++++
server/src/dbusconnectionpool.h | 41 +++++++++++++++++++
server/src/handler/fetchhelper.cpp | 9 ++---
server/src/nepomuk/dbusconnectionpool.cpp | 59 ---------------------------
server/src/nepomuk/dbusconnectionpool.h | 38 ------------------
server/src/nepomuk/queryserviceclient.cpp | 4 +-
server/src/search/agentsearchinstance.cpp | 5 ++-
server/src/search/searchmanager.cpp | 3 +-
server/src/search/searchtaskmanager.cpp | 11 ++---
server/src/search/searchtaskmanager.h | 2 -
server/src/storage/itemretrievalmanager.cpp | 6 +--
server/src/storagejanitor.cpp | 3 +-
13 files changed, 124 insertions(+), 121 deletions(-)
create mode 100644 server/src/dbusconnectionpool.cpp
create mode 100644 server/src/dbusconnectionpool.h
delete mode 100644 server/src/nepomuk/dbusconnectionpool.cpp
delete mode 100644 server/src/nepomuk/dbusconnectionpool.h
diff --git a/server/CMakeLists.txt b/server/CMakeLists.txt
index 571bead..1709348 100644
--- a/server/CMakeLists.txt
+++ b/server/CMakeLists.txt
@@ -106,6 +106,7 @@ set(libakonadiprivate_SRCS
src/collectionscheduler.cpp
src/clientcapabilities.cpp
src/clientcapabilityaggregator.cpp
+ src/dbusconnectionpool.cpp
src/handler.cpp
src/handlerhelper.cpp
src/intervalcheck.cpp
@@ -206,7 +207,6 @@ if (Soprano_FOUND)
src/search/nepomuksearchengine.cpp
src/nepomuk/dbusoperators.cpp
src/nepomuk/queryserviceclient.cpp
- src/nepomuk/dbusconnectionpool.cpp
src/nepomuk/result.cpp
)
diff --git a/server/src/dbusconnectionpool.cpp b/server/src/dbusconnectionpool.cpp
new file mode 100644
index 0000000..9aede4f
--- /dev/null
+++ b/server/src/dbusconnectionpool.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2010 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
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB. If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include "dbusconnectionpool.h"
+#include <QCoreApplication>
+#include <QThread>
+#include <QThreadStorage>
+
+namespace {
+QAtomicInt s_connectionCounter;
+
+class DBusConnectionPoolPrivate
+{
+public:
+ DBusConnectionPoolPrivate()
+ : m_connection( QDBusConnection::connectToBus(
+ QDBusConnection::SessionBus,
+ QString::fromLatin1("AkonadiServer-%1").arg(newNumber()) ) )
+ {
+ }
+ ~DBusConnectionPoolPrivate() {
+ QDBusConnection::disconnectFromBus( m_connection.name() );
+ }
+
+ QDBusConnection connection() const { return m_connection; }
+
+private:
+ static int newNumber() {
+ return s_connectionCounter.fetchAndAddAcquire( 1 );
+ }
+ QDBusConnection m_connection;
+};
+}
+
+QThreadStorage<DBusConnectionPoolPrivate *> s_perThreadConnection;
+
+QDBusConnection Akonadi::Server::DBusConnectionPool::threadConnection()
+{
+ if ( !QCoreApplication::instance() || QCoreApplication::instance()->thread() == QThread::currentThread() ) {
+ return QDBusConnection::sessionBus(); // main thread, use the default session bus
+ }
+ if ( !s_perThreadConnection.hasLocalData() ) {
+ s_perThreadConnection.setLocalData( new DBusConnectionPoolPrivate );
+ }
+ return s_perThreadConnection.localData()->connection();
+}
diff --git a/server/src/dbusconnectionpool.h b/server/src/dbusconnectionpool.h
new file mode 100644
index 0000000..4f8a93e
--- /dev/null
+++ b/server/src/dbusconnectionpool.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2010 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
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB. If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifndef DBUSCONNECTIONPOOL_H
+#define DBUSCONNECTIONPOOL_H
+
+#include <QDBusConnection>
+
+namespace Akonadi {
+namespace Server {
+namespace DBusConnectionPool {
+
+/**
+ * Returns a new QDBusConnection for each thread, because QDBusConnection is
+ * not thread-safe in Qt 4.
+ *
+ * FIXME: Remove in KF5
+ */
+QDBusConnection threadConnection();
+
+}
+}
+}
+
+#endif
diff --git a/server/src/handler/fetchhelper.cpp b/server/src/handler/fetchhelper.cpp
index 6284cc9..a6888a3 100644
--- a/server/src/handler/fetchhelper.cpp
+++ b/server/src/handler/fetchhelper.cpp
@@ -38,6 +38,7 @@
#include "utils.h"
#include "intervalcheck.h"
#include "agentmanagerinterface.h"
+#include "dbusconnectionpool.h"
#include <QtCore/QLocale>
#include <QtCore/QStringList>
@@ -252,16 +253,12 @@ bool FetchHelper::isScopeLocal( const Scope &scope )
query.next();
const QString resourceName = query.value( 0 ).toString();
- // Workaround for QDBusConnectionPrivate not being thread-safe in Qt 4, fixed in Qt 5.2
- // TODO: Remove in KF5
- const QDBusConnection connection = QDBusConnection::connectToBus( QDBusConnection::SessionBus,
- QString::fromLatin1( mConnection->sessionId() ) );
+
org::freedesktop::Akonadi::AgentManager manager( AkDBus::serviceName( AkDBus::Control ),
QLatin1String( "/AgentManager" ),
- connection );
+ DBusConnectionPool::threadConnection() );
const QString typeIdentifier = manager.agentInstanceType( resourceName );
const QVariantMap properties = manager.agentCustomProperties( typeIdentifier );
- QDBusConnection::disconnectFromBus( QString::fromLatin1( mConnection->sessionId() ) );
return properties.value( QLatin1String( "HasLocalStorage" ), false ).toBool();
}
diff --git a/server/src/nepomuk/dbusconnectionpool.cpp b/server/src/nepomuk/dbusconnectionpool.cpp
deleted file mode 100644
index 598d16f..0000000
--- a/server/src/nepomuk/dbusconnectionpool.cpp
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * This file is part of the Nepomuk KDE project.
- * Copyright (C) 2010 Sebastian Trueg <trueg@kde.org>
- * Copyright (C) 2010 David Faure <faure@kde.org>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB. If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#include "dbusconnectionpool.h"
-#include <QThreadStorage>
-
-namespace {
-QAtomicInt s_connectionCounter;
-
-class DBusConnectionPoolPrivate
-{
-public:
- DBusConnectionPoolPrivate()
- : m_connection( QDBusConnection::connectToBus(
- QDBusConnection::SessionBus,
- QString::fromLatin1( "NepomukQueryServiceConnection%1" ).arg( newNumber() ) ) )
- {
- }
- ~DBusConnectionPoolPrivate() {
- QDBusConnection::disconnectFromBus( m_connection.name() );
- }
-
- QDBusConnection connection() const { return m_connection; }
-
-private:
- static int newNumber() {
- return s_connectionCounter.fetchAndAddAcquire( 1 );
- }
- QDBusConnection m_connection;
-};
-}
-
-QThreadStorage<DBusConnectionPoolPrivate *> s_perThreadConnection;
-
-QDBusConnection DBusConnectionPool::threadConnection()
-{
- if ( !s_perThreadConnection.hasLocalData() ) {
- s_perThreadConnection.setLocalData( new DBusConnectionPoolPrivate );
- }
- return s_perThreadConnection.localData()->connection();
-}
diff --git a/server/src/nepomuk/dbusconnectionpool.h b/server/src/nepomuk/dbusconnectionpool.h
deleted file mode 100644
index c5ac746..0000000
--- a/server/src/nepomuk/dbusconnectionpool.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * This file is part of the Nepomuk KDE project.
- * Copyright (C) 2010 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
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB. If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#ifndef _DBUS_CONNECTION_POOL_H_
-#define _DBUS_CONNECTION_POOL_H_
-
-#include <QtDBus/QDBusConnection>
-
-namespace DBusConnectionPool
-{
-/**
- * The DBusConnectionPool works around the problem
- * of QDBusConnection not being thread-safe. As soon as that
- * has been fixed (either directly in libdbus or with a work-
- * around in Qt) this method can be dropped in favor of
- * QDBusConnection::sessionBus().
- */
-QDBusConnection threadConnection();
-}
-
-#endif
diff --git a/server/src/nepomuk/queryserviceclient.cpp b/server/src/nepomuk/queryserviceclient.cpp
index 089a80b..b27a751 100644
--- a/server/src/nepomuk/queryserviceclient.cpp
+++ b/server/src/nepomuk/queryserviceclient.cpp
@@ -24,7 +24,7 @@
#include "result.h"
#include "queryserviceinterface.h"
#include "queryinterface.h"
-#include <dbusconnectionpool.h>
+#include "dbusconnectionpool.h"
#include <QtDBus/QDBusInterface>
#include <QtDBus/QDBusConnection>
@@ -40,7 +40,7 @@ public:
Private()
: queryServiceInterface( 0 ),
queryInterface( 0 ),
- dbusConnection( DBusConnectionPool::threadConnection() ),
+ dbusConnection( Akonadi::Server::DBusConnectionPool::threadConnection() ),
m_queryActive( false ),
loop( 0 ) {
}
diff --git a/server/src/search/agentsearchinstance.cpp b/server/src/search/agentsearchinstance.cpp
index 54ffab0..ca6ef87 100644
--- a/server/src/search/agentsearchinstance.cpp
+++ b/server/src/search/agentsearchinstance.cpp
@@ -21,6 +21,7 @@
#include "agentsearchinterface.h"
#include "searchtaskmanager.h"
#include "akdbus.h"
+#include "dbusconnectionpool.h"
using namespace Akonadi::Server;
@@ -43,7 +44,7 @@ bool AgentSearchInstance::init()
mInterface = new OrgFreedesktopAkonadiAgentSearchInterface(
AkDBus::agentServiceName( mId, AkDBus::Agent ),
QLatin1String( "/Search" ),
- QDBusConnection::sessionBus() );
+ DBusConnectionPool::threadConnection() );
if ( !mInterface || !mInterface->isValid() ) {
delete mInterface;
@@ -52,7 +53,7 @@ bool AgentSearchInstance::init()
}
mServiceWatcher = new QDBusServiceWatcher( AkDBus::agentServiceName( mId, AkDBus::Agent ),
- QDBusConnection::sessionBus(),
+ DBusConnectionPool::threadConnection(),
QDBusServiceWatcher::WatchForOwnerChange,
this );
connect( mServiceWatcher, SIGNAL(serviceOwnerChanged(QString,QString,QString)),
diff --git a/server/src/search/searchmanager.cpp b/server/src/search/searchmanager.cpp
index 139d058..1eafb27 100644
--- a/server/src/search/searchmanager.cpp
+++ b/server/src/search/searchmanager.cpp
@@ -26,6 +26,7 @@
#include "agentsearchengine.h"
#include "nepomuksearchengine.h"
#include "notificationmanager.h"
+#include "dbusconnectionpool.h"
#include "searchrequest.h"
#include "searchtaskmanager.h"
#include "storage/datastore.h"
@@ -98,7 +99,7 @@ SearchManager::SearchManager( const QStringList &searchEngines, QObject *parent
new SearchManagerAdaptor( this );
- QDBusConnection::sessionBus().registerObject(
+ DBusConnectionPool::threadConnection().registerObject(
QLatin1String( "/SearchManager" ),
this,
QDBusConnection::ExportAdaptors );
diff --git a/server/src/search/searchtaskmanager.cpp b/server/src/search/searchtaskmanager.cpp
index 51bb516..e7980a4 100644
--- a/server/src/search/searchtaskmanager.cpp
+++ b/server/src/search/searchtaskmanager.cpp
@@ -23,6 +23,7 @@
#include "akdbus.h"
#include "connection.h"
#include "storage/selectquerybuilder.h"
+#include "dbusconnectionpool.h"
#include <entities.h>
#include <QSqlError>
@@ -36,9 +37,6 @@ SearchTaskManager *SearchTaskManager::sInstance = 0;
SearchTaskManager::SearchTaskManager()
: QObject()
, mShouldStop( false )
- , mAgentManager( AkDBus::serviceName( AkDBus::Control ), QLatin1String( "/AgentManager" ),
- QDBusConnection::sessionBus() )
-
{
sInstance = this;
@@ -125,13 +123,16 @@ void SearchTaskManager::addTask( SearchTask *task )
}
mInstancesLock.lock();
+
+ org::freedesktop::Akonadi::AgentManager agentManager( AkDBus::serviceName( AkDBus::Control ), QLatin1String( "/AgentManager" ),
+ DBusConnectionPool::threadConnection() );
do {
const QString resourceId = query.value( 1 ).toString();
if ( !mInstances.contains( resourceId ) ) {
akDebug() << "Resource" << resourceId << "does not implement Search interface, skipping";
- } else if ( !mAgentManager.agentInstanceOnline( resourceId ) ) {
+ } else if ( !agentManager.agentInstanceOnline( resourceId ) ) {
akDebug() << "Agent" << resourceId << "is offline, skipping";
- } else if ( mAgentManager.agentInstanceStatus( resourceId ) > 2 ) { // 2 == Broken, 3 == Not Configured
+ } else if ( agentManager.agentInstanceStatus( resourceId ) > 2 ) { // 2 == Broken, 3 == Not Configured
akDebug() << "Agent" << resourceId << "is broken or not configured";
} else {
const qint64 collectionId = query.value( 0 ).toLongLong();
diff --git a/server/src/search/searchtaskmanager.h b/server/src/search/searchtaskmanager.h
index 06e1b52..9b7972b 100644
--- a/server/src/search/searchtaskmanager.h
+++ b/server/src/search/searchtaskmanager.h
@@ -97,8 +97,6 @@ class SearchTaskManager : public QObject
TasksMap::Iterator cancelRunningTask( TasksMap::Iterator &iter );
bool allResourceTasksCompleted( SearchTask* ) const;
- org::freedesktop::Akonadi::AgentManager mAgentManager;
-
QMap<QString, AgentSearchInstance* > mInstances;
QMutex mInstancesLock;
diff --git a/server/src/storage/itemretrievalmanager.cpp b/server/src/storage/itemretrievalmanager.cpp
index d49ba66..ce88410 100644
--- a/server/src/storage/itemretrievalmanager.cpp
+++ b/server/src/storage/itemretrievalmanager.cpp
@@ -20,6 +20,7 @@
#include "itemretrievalmanager.h"
#include "itemretrievalrequest.h"
#include "itemretrievaljob.h"
+#include "dbusconnectionpool.h"
#include "resourceinterface.h"
@@ -40,10 +41,7 @@ ItemRetrievalManager *ItemRetrievalManager::sInstance = 0;
ItemRetrievalManager::ItemRetrievalManager( QObject *parent )
: QObject( parent ),
- mDBusConnection(
- QDBusConnection::connectToBus(
- QDBusConnection::SessionBus,
- QString::fromLatin1( "AkonadiServerItemRetrievalManager" ) ) )
+ mDBusConnection( DBusConnectionPool::threadConnection() )
{
// make sure we are created from the retrieval thread and only once
Q_ASSERT( QThread::currentThread() != QCoreApplication::instance()->thread() );
diff --git a/server/src/storagejanitor.cpp b/server/src/storagejanitor.cpp
index ebe7a24..af1a407 100644
--- a/server/src/storagejanitor.cpp
+++ b/server/src/storagejanitor.cpp
@@ -26,6 +26,7 @@
#include "storage/parthelper.h"
#include "resourcemanager.h"
#include "entities.h"
+#include "dbusconnectionpool.h"
#include <akdbus.h>
#include <akdebug.h>
@@ -63,7 +64,7 @@ void StorageJanitorThread::run()
StorageJanitor::StorageJanitor( QObject *parent )
: QObject( parent )
- , m_connection( QDBusConnection::connectToBus( QDBusConnection::SessionBus, QLatin1String( staticMetaObject.className() ) ) )
+ , m_connection( DBusConnectionPool::threadConnection() )
, m_lostFoundCollectionId( -1 )
{
DataStore::self();
--
1.9.3