diff --git a/src/akonadictl/akonadistarter.h b/src/akonadictl/akonadistarter.h --- a/src/akonadictl/akonadistarter.h +++ b/src/akonadictl/akonadistarter.h @@ -21,18 +21,17 @@ #define AKONADISTARTER_H #include +#include class AkonadiStarter : public QObject { Q_OBJECT public: explicit AkonadiStarter(QObject *parent = nullptr); Q_REQUIRED_RESULT bool start(bool verbose); -private Q_SLOTS: - void serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner); - private: + QDBusServiceWatcher mWatcher; bool mRegistered = false; }; diff --git a/src/akonadictl/akonadistarter.cpp b/src/akonadictl/akonadistarter.cpp --- a/src/akonadictl/akonadistarter.cpp +++ b/src/akonadictl/akonadistarter.cpp @@ -35,18 +35,19 @@ AkonadiStarter::AkonadiStarter(QObject *parent) : QObject(parent) + , mWatcher(Akonadi::DBus::serviceName(Akonadi::DBus::ControlLock), QDBusConnection::sessionBus(), + QDBusServiceWatcher::WatchForRegistration) { - QDBusServiceWatcher *watcher = new QDBusServiceWatcher(Akonadi::DBus::serviceName(Akonadi::DBus::ControlLock), - QDBusConnection::sessionBus(), - QDBusServiceWatcher::WatchForOwnerChange, this); - - connect(watcher, &QDBusServiceWatcher::serviceOwnerChanged, - this, &AkonadiStarter::serviceOwnerChanged); + connect(&mWatcher, &QDBusServiceWatcher::serviceRegistered, + this, [this]() { + mRegistered = true; + QCoreApplication::instance()->quit(); + }); } bool AkonadiStarter::start(bool verbose) { - qCDebug(AKONADICTL_LOG) << "Starting Akonadi Server..."; + qCInfo(AKONADICTL_LOG) << "Starting Akonadi Server..."; QStringList serverArgs; if (Akonadi::Instance::hasIdentifier()) { @@ -74,18 +75,7 @@ return false; } - qCDebug(AKONADICTL_LOG) << " done."; + qCInfo(AKONADICTL_LOG) << " done."; return true; } -void AkonadiStarter::serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner) -{ - Q_UNUSED(name); - Q_UNUSED(oldOwner); - if (newOwner.isEmpty()) { - return; - } - - mRegistered = true; - QCoreApplication::instance()->quit(); -} diff --git a/src/core/agentmanager.cpp b/src/core/agentmanager.cpp --- a/src/core/agentmanager.cpp +++ b/src/core/agentmanager.cpp @@ -30,6 +30,7 @@ #include #include +#include using namespace Akonadi; using namespace AkRanges; @@ -309,18 +310,6 @@ return instance; } -void AgentManagerPrivate::serviceOwnerChanged(const QString &, const QString &oldOwner, const QString &) -{ - if (oldOwner.isEmpty()) { - if (mTypes.isEmpty()) { // just to be safe - readAgentTypes(); - } - if (mInstances.isEmpty()) { - readAgentInstances(); - } - } -} - void AgentManagerPrivate::createDBusInterface() { mTypes.clear(); @@ -370,11 +359,18 @@ d->createDBusInterface(); - QDBusServiceWatcher *watcher = new QDBusServiceWatcher(ServerManager::serviceName(ServerManager::Control), - KDBusConnectionPool::threadConnection(), - QDBusServiceWatcher::WatchForOwnerChange, this); - connect(watcher, &QDBusServiceWatcher::serviceOwnerChanged, - this, [this](const QString &arg1, const QString &arg2 , const QString &arg3) { d->serviceOwnerChanged(arg1, arg2, arg3); }); + d->mServiceWatcher = std::make_unique( + ServerManager::serviceName(ServerManager::Control), KDBusConnectionPool::threadConnection(), + QDBusServiceWatcher::WatchForRegistration); + connect(d->mServiceWatcher.get(), &QDBusServiceWatcher::serviceRegistered, + this, [this]() { + if (d->mTypes.isEmpty()) { // just to be safe + d->readAgentTypes(); + } + if (d->mInstances.isEmpty()) { + d->readAgentInstances(); + } + }); } // @endcond @@ -395,7 +391,8 @@ AgentType::List AgentManager::types() const { - // Maybe the Control process is up and ready but we haven't been to the event loop yet so serviceOwnerChanged wasn't called yet. + // Maybe the Control process is up and ready but we haven't been to the event loop yet so + // QDBusServiceWatcher hasn't notified us yet. // In that case make sure to do it here, to avoid going into Broken state. if (d->mTypes.isEmpty()) { d->readAgentTypes(); diff --git a/src/core/agentmanager_p.h b/src/core/agentmanager_p.h --- a/src/core/agentmanager_p.h +++ b/src/core/agentmanager_p.h @@ -27,6 +27,10 @@ #include +#include + +class QDBusServiceWatcher; + namespace Akonadi { @@ -85,7 +89,6 @@ void synchronizeTags(const AgentInstance &instance); void synchronizeRelations(const AgentInstance &instance); - void serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner); void createDBusInterface(); AgentType fillAgentType(const QString &identifier) const; @@ -99,6 +102,8 @@ QHash mTypes; QHash mInstances; + + std::unique_ptr mServiceWatcher; }; } diff --git a/src/core/jobs/specialcollectionshelperjobs.cpp b/src/core/jobs/specialcollectionshelperjobs.cpp --- a/src/core/jobs/specialcollectionshelperjobs.cpp +++ b/src/core/jobs/specialcollectionshelperjobs.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #define LOCK_WAIT_TIMEOUT_SECONDS 30 @@ -562,8 +563,6 @@ Private(GetLockJob *qq); void doStart(); // slot - void serviceOwnerChanged(const QString &name, const QString &oldOwner, - const QString &newOwner); // slot void timeout(); // slot GetLockJob *const q; @@ -589,10 +588,15 @@ //qCDebug(AKONADICORE_LOG) << "Got lock immediately."; q->emitResult(); } else { - QDBusServiceWatcher *watcher = new QDBusServiceWatcher(dbusServiceName(), KDBusConnectionPool::threadConnection(), - QDBusServiceWatcher::WatchForOwnerChange, q); - //qCDebug(AKONADICORE_LOG) << "Waiting for lock."; - connect(watcher, SIGNAL(serviceOwnerChanged(QString,QString,QString)), q, SLOT(serviceOwnerChanged(QString,QString,QString))); + auto watcher = new QDBusServiceWatcher(dbusServiceName(), KDBusConnectionPool::threadConnection(), + QDBusServiceWatcher::WatchForUnregistration, q); + connect(watcher, &QDBusServiceWatcher::serviceUnregistered, + q, [this]() { + if (KDBusConnectionPool::threadConnection().registerService(dbusServiceName())) { + mSafetyTimer->stop(); + q->emitResult(); + } + }); mSafetyTimer = new QTimer(q); mSafetyTimer->setSingleShot(true); @@ -602,20 +606,6 @@ } } -void GetLockJob::Private::serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner) -{ - Q_UNUSED(name); - Q_UNUSED(oldOwner); - - if (newOwner.isEmpty()) { - const bool gotIt = KDBusConnectionPool::threadConnection().registerService(dbusServiceName()); - if (gotIt) { - mSafetyTimer->stop(); - q->emitResult(); - } - } -} - void GetLockJob::Private::timeout() { qCWarning(AKONADICORE_LOG) << "Timeout trying to get lock. Check who has acquired the name" << dbusServiceName() << "on DBus, using qdbus or qdbusviewer."; diff --git a/src/core/jobs/specialcollectionshelperjobs_p.h b/src/core/jobs/specialcollectionshelperjobs_p.h --- a/src/core/jobs/specialcollectionshelperjobs_p.h +++ b/src/core/jobs/specialcollectionshelperjobs_p.h @@ -208,7 +208,6 @@ Private *const d; Q_PRIVATE_SLOT(d, void doStart()) - Q_PRIVATE_SLOT(d, void serviceOwnerChanged(QString, QString, QString)) }; // ===================== helper functions ============================ diff --git a/src/core/servermanager.h b/src/core/servermanager.h --- a/src/core/servermanager.h +++ b/src/core/servermanager.h @@ -231,7 +231,6 @@ friend class ServerManagerPrivate; ServerManager(ServerManagerPrivate *dd); ServerManagerPrivate *const d; - Q_PRIVATE_SLOT(d, void serviceOwnerChanged(const QString &, const QString &, const QString &)) Q_PRIVATE_SLOT(d, void checkStatusChanged()) Q_PRIVATE_SLOT(d, void timeout()) //@endcond diff --git a/src/core/servermanager.cpp b/src/core/servermanager.cpp --- a/src/core/servermanager.cpp +++ b/src/core/servermanager.cpp @@ -44,6 +44,8 @@ #include #include #include +#include +#include using namespace Akonadi; @@ -69,31 +71,13 @@ delete instance; } - void serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner) - { - if (name == ServerManager::serviceName(ServerManager::ControlLock) && !oldOwner.isEmpty() && newOwner.isEmpty()) { - // Control.Lock has disappeared during startup, which means that akonadi_control - // has terminated, most probably because it was not able to start akonadiserver - // process. Don't wait 30 seconds for sefetyTimeout, but go into Broken state - // immediately. - if (mState == ServerManager::Starting) { - setState(ServerManager::Broken); - return; - } - } - - serverProtocolVersion = -1; - checkStatusChanged(); - } - void checkStatusChanged() { setState(instance->state()); } void setState(ServerManager::State state) { - if (mState != state) { mState = state; Q_EMIT instance->stateChanged(state); @@ -128,6 +112,7 @@ Firstrun *mFirstRunner = nullptr; static Internal::ClientType clientType; QString mBrokenReason; + std::unique_ptr serviceWatcher; }; int ServerManagerPrivate::serverProtocolVersion = -1; @@ -145,19 +130,36 @@ qRegisterMetaType(); - QDBusServiceWatcher *watcher = new QDBusServiceWatcher(ServerManager::serviceName(ServerManager::Server), - KDBusConnectionPool::threadConnection(), - QDBusServiceWatcher::WatchForOwnerChange, this); - watcher->addWatchedService(ServerManager::serviceName(ServerManager::Control)); - watcher->addWatchedService(ServerManager::serviceName(ServerManager::ControlLock)); - watcher->addWatchedService(ServerManager::serviceName(ServerManager::UpgradeIndicator)); + d->serviceWatcher = std::make_unique( + ServerManager::serviceName(ServerManager::Server), KDBusConnectionPool::threadConnection(), + QDBusServiceWatcher::WatchForRegistration | QDBusServiceWatcher::WatchForUnregistration); + d->serviceWatcher->addWatchedService(ServerManager::serviceName(ServerManager::Control)); + d->serviceWatcher->addWatchedService(ServerManager::serviceName(ServerManager::ControlLock)); + d->serviceWatcher->addWatchedService(ServerManager::serviceName(ServerManager::UpgradeIndicator)); // this (and also the two connects below) are queued so that they trigger after AgentManager is done loading // the current agent types and instances // this ensures the invariant of AgentManager reporting a consistent state if ServerManager::state() == Running // that's the case with direct connections as well, but only after you enter the event loop once - connect(watcher, SIGNAL(serviceOwnerChanged(QString,QString,QString)), - this, SLOT(serviceOwnerChanged(QString,QString,QString)), Qt::QueuedConnection); + connect(d->serviceWatcher.get(), &QDBusServiceWatcher::serviceRegistered, + this, [this]() { + d->serverProtocolVersion = -1; + d->checkStatusChanged(); + }, Qt::QueuedConnection); + connect(d->serviceWatcher.get(), &QDBusServiceWatcher::serviceUnregistered, + this, [this](const QString &name) { + if (name == ServerManager::serviceName(ServerManager::ControlLock) && d->mState == ServerManager::Starting) { + // Control.Lock has disappeared during startup, which means that akonadi_control + // has terminated, most probably because it was not able to start akonadiserver + // process. Don't wait 30 seconds for sefetyTimeout, but go into Broken state + // immediately. + d->setState(ServerManager::Broken); + return; + } + + d->serverProtocolVersion = -1; + d->checkStatusChanged(); + }, Qt::QueuedConnection); // AgentManager is dangerous to use for agents themselves if (Internal::clientType() != Internal::User) { diff --git a/src/server/akonadi.h b/src/server/akonadi.h --- a/src/server/akonadi.h +++ b/src/server/akonadi.h @@ -23,7 +23,10 @@ #include #include +#include + class QProcess; +class QDBusServiceWatcher; namespace Akonadi { @@ -81,7 +84,6 @@ private Q_SLOTS: void doQuit(); - void serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner); void connectionDisconnected(); private: @@ -93,6 +95,8 @@ protected: AkonadiServer(QObject *parent = nullptr); + std::unique_ptr mControlWatcher; + AkLocalServer *mCmdServer = nullptr; AkLocalServer *mNtfServer = nullptr; diff --git a/src/server/akonadi.cpp b/src/server/akonadi.cpp --- a/src/server/akonadi.cpp +++ b/src/server/akonadi.cpp @@ -214,12 +214,14 @@ connectionSettings.setValue(QStringLiteral("DBUS/Address"), QLatin1String(dbusAddress)); } - QDBusServiceWatcher *watcher = new QDBusServiceWatcher(DBus::serviceName(DBus::Control), - QDBusConnection::sessionBus(), - QDBusServiceWatcher::WatchForOwnerChange, this); - - connect(watcher, &QDBusServiceWatcher::serviceOwnerChanged, - this, &AkonadiServer::serviceOwnerChanged); + mControlWatcher = std::make_unique( + DBus::serviceName(DBus::Control), QDBusConnection::sessionBus(), + QDBusServiceWatcher::WatchForUnregistration); + connect(mControlWatcher.get(), &QDBusServiceWatcher::serviceUnregistered, + this, [this]() { + qCCritical(AKONADISERVER_LOG) << "Control process died, committing suicide!"; + quit(); + }); // Unhide all the items that are actually hidden. // The hidden flag was probably left out after an (abrupt) @@ -394,16 +396,6 @@ DbConfig::configuredDatabase()->stopInternalServer(); } -void AkonadiServer::serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner) -{ - Q_UNUSED(name); - Q_UNUSED(oldOwner); - if (newOwner.isEmpty()) { - qCCritical(AKONADISERVER_LOG) << "Control process died, committing suicide!"; - quit(); - } -} - CacheCleaner *AkonadiServer::cacheCleaner() { return mCacheCleaner; diff --git a/src/server/search/agentsearchinstance.h b/src/server/search/agentsearchinstance.h --- a/src/server/search/agentsearchinstance.h +++ b/src/server/search/agentsearchinstance.h @@ -23,6 +23,8 @@ #include #include +#include + class QDBusServiceWatcher; class OrgFreedesktopAkonadiAgentSearchInterface; @@ -44,13 +46,10 @@ OrgFreedesktopAkonadiAgentSearchInterface *interface() const; -private Q_SLOTS: - void serviceOwnerChanged(const QString &service, const QString &oldName, const QString &newName); - private: QString mId; OrgFreedesktopAkonadiAgentSearchInterface *mInterface; - QDBusServiceWatcher *mServiceWatcher; + std::unique_ptr mServiceWatcher; }; } // namespace Server diff --git a/src/server/search/agentsearchinstance.cpp b/src/server/search/agentsearchinstance.cpp --- a/src/server/search/agentsearchinstance.cpp +++ b/src/server/search/agentsearchinstance.cpp @@ -30,7 +30,6 @@ AgentSearchInstance::AgentSearchInstance(const QString &id) : mId(id) , mInterface(nullptr) - , mServiceWatcher(nullptr) { } @@ -54,26 +53,17 @@ return false; } - mServiceWatcher = new QDBusServiceWatcher(DBus::agentServiceName(mId, DBus::Agent), - DBusConnectionPool::threadConnection(), - QDBusServiceWatcher::WatchForOwnerChange, - this); - connect(mServiceWatcher, &QDBusServiceWatcher::serviceOwnerChanged, - this, &AgentSearchInstance::serviceOwnerChanged); + mServiceWatcher = std::make_unique( + DBus::agentServiceName(mId, DBus::Agent), DBusConnectionPool::threadConnection(), + QDBusServiceWatcher::WatchForUnregistration); + connect(mServiceWatcher.get(), &QDBusServiceWatcher::serviceUnregistered, + this, [this]() { + SearchTaskManager::instance()->unregisterInstance(mId); + }); return true; } -void AgentSearchInstance::serviceOwnerChanged(const QString &service, const QString &oldName, const QString &newName) -{ - Q_UNUSED(service); - Q_UNUSED(oldName); - - if (newName.isEmpty()) { - SearchTaskManager::instance()->unregisterInstance(mId); - } -} - void AgentSearchInstance::search(const QByteArray &searchId, const QString &query, qlonglong collectionId) { diff --git a/src/server/storage/itemretrievalmanager.h b/src/server/storage/itemretrievalmanager.h --- a/src/server/storage/itemretrievalmanager.h +++ b/src/server/storage/itemretrievalmanager.h @@ -32,6 +32,7 @@ #include class OrgFreedesktopAkonadiResourceInterface; +class QDBusServiceWatcher; namespace Akonadi { @@ -79,7 +80,6 @@ private Q_SLOTS: void init() override; - void serviceOwnerChanged(const QString &serviceName, const QString &oldOwner, const QString &newOwner); void processRequest(); void triggerCollectionSync(const QString &resource, qint64 colId); void triggerCollectionTreeSync(const QString &resource); @@ -101,6 +101,7 @@ // resource dbus interface cache std::unordered_map> mResourceInterfaces; + std::unique_ptr mDBusWatcher; }; } // namespace Server diff --git a/src/server/storage/itemretrievalmanager.cpp b/src/server/storage/itemretrievalmanager.cpp --- a/src/server/storage/itemretrievalmanager.cpp +++ b/src/server/storage/itemretrievalmanager.cpp @@ -25,13 +25,14 @@ #include "resourceinterface.h" +#include #include #include #include #include #include -#include +#include using namespace Akonadi; using namespace Akonadi::Server; @@ -72,8 +73,17 @@ AkThread::init(); QDBusConnection conn = DBusConnectionPool::threadConnection(); - connect(conn.interface(), &QDBusConnectionInterface::serviceOwnerChanged, - this, &ItemRetrievalManager::serviceOwnerChanged); + mDBusWatcher = std::make_unique( + QStringLiteral("org.freedesktop.Akonadi.Resource*"), conn, + QDBusServiceWatcher::WatchForUnregistration); + connect(mDBusWatcher.get(), &QDBusServiceWatcher::serviceUnregistered, + this, [this](const QString &serviceName) { + const auto service = DBus::parseAgentServiceName(serviceName); + if (service.has_value() && service->agentType == DBus::Resource) { + qCInfo(AKONADISERVER_LOG) << "ItemRetrievalManager has lost connection to resource" << serviceName << ", discarding cached interface."; + mResourceInterfaces.erase(service->identifier); + } + }); connect(this, &ItemRetrievalManager::requestAdded, this, &ItemRetrievalManager::processRequest, Qt::QueuedConnection); @@ -85,21 +95,6 @@ return sInstance; } -// called within the retrieval thread -void ItemRetrievalManager::serviceOwnerChanged(const QString &serviceName, const QString &oldOwner, const QString &newOwner) -{ - Q_UNUSED(newOwner); - if (oldOwner.isEmpty()) { - return; - } - const auto service = DBus::parseAgentServiceName(serviceName); - if (!service.has_value() || service->agentType != DBus::Resource) { - return; - } - qCDebug(AKONADISERVER_LOG) << "ItemRetrievalManager lost connection to resource" << serviceName << ", discarding cached interface"; - mResourceInterfaces.erase(service->identifier); -} - // called within the retrieval thread org::freedesktop::Akonadi::Resource *ItemRetrievalManager::resourceInterface(const QString &id) {