commit 94fb16b7c69c5a1e14a3a068038fd7c952546b58 Author: Harald Sitter Date: Wed Feb 20 14:32:38 2019 +0100 wip fix race on kcrash restarts diff --git a/src/kdbusservice.cpp b/src/kdbusservice.cpp index 35dfe36..c39f72d 100644 --- a/src/kdbusservice.cpp +++ b/src/kdbusservice.cpp @@ -2,6 +2,7 @@ Copyright (c) 2011 David Faure Copyright (c) 2011 Kevin Ottens + Copyright (c) 2019 Harald Sitter This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public @@ -74,26 +75,45 @@ public: int exitValue; }; -KDBusService::KDBusService(StartupOptions options, QObject *parent) - : QObject(parent), d(new KDBusServicePrivate) -{ - new KDBusServiceAdaptor(this); - new KDBusServiceExtensionsAdaptor(this); - QDBusConnectionInterface *bus = nullptr; +// Wraps a serviceName registration. +class Registration : public QObject { + Q_OBJECT +public: + Registration(KDBusServicePrivate *d_, KDBusService::StartupOptions options_) + : d(d_) + , options(options_) + { + if (!QDBusConnection::sessionBus().isConnected() || !(bus = QDBusConnection::sessionBus().interface())) { + d->errorMessage = QLatin1String("Session bus not found\n" + "To circumvent this problem try the following command (with Linux and bash)\n" + "export $(dbus-launch)"); + } else { + generateServiceName(); + } + } - if (!QDBusConnection::sessionBus().isConnected() || !(bus = QDBusConnection::sessionBus().interface())) { - d->errorMessage = QLatin1String("Session bus not found\n" - "To circumvent this problem try the following command (with Linux and bash)\n" - "export $(dbus-launch)"); + void run() { + qDebug() << "dbus start"; + if (bus) { + registerOnBus(); + } + + if (!d->registered && ((options & KDBusService::NoExitOnFailure) == 0)) { + qCritical() << d->errorMessage; + exit(1); + } } - if (bus) { + private: + + void generateServiceName() + { d->serviceName = d->generateServiceName(); - QString objectPath = QLatin1Char('/') + d->serviceName; + objectPath = QLatin1Char('/') + d->serviceName; objectPath.replace(QLatin1Char('.'), QLatin1Char('/')); objectPath.replace(QLatin1Char('-'), QLatin1Char('_')); // see spec change at https://bugs.freedesktop.org/show_bug.cgi?id=95129 - if (options & Multiple) { + if (options & KDBusService::Multiple) { bool inSandbox = false; if (!qEnvironmentVariableIsEmpty("XDG_RUNTIME_DIR")) { inSandbox = QFileInfo::exists(QString::fromUtf8(qgetenv("XDG_RUNTIME_DIR")) + QLatin1String("/flatpak-info")); @@ -105,57 +125,121 @@ KDBusService::KDBusService(StartupOptions options, QObject *parent) d->serviceName += QLatin1Char('-') + QString::number(QCoreApplication::applicationPid()); } } + } + void registerOnBus() + { QDBusConnection::sessionBus().registerObject(QStringLiteral("/MainApplication"), QCoreApplication::instance(), - QDBusConnection::ExportAllSlots | - QDBusConnection::ExportScriptableProperties | - QDBusConnection::ExportAdaptors); + QDBusConnection::ExportAllSlots | + QDBusConnection::ExportScriptableProperties | + QDBusConnection::ExportAdaptors); QDBusConnection::sessionBus().registerObject(objectPath, this, - QDBusConnection::ExportAdaptors); + QDBusConnection::ExportAdaptors); + + qDebug() << "registered objects -> trying to register service"; + + attemptRegistration(); + if (!d->registered && options & KDBusService::Unique) { + qDebug() << "failed to register or activate; let's wait a bit"; + + // If we got here the activation failed for whatever reason. + // This may be a timing issue if the previous instance crashed and + // this new instance is already attempt to claim the service name + // before DBus takes away the name from the defunct previous + // instance. To prevent this reace condition we'll wait a sensible + // amount of time for the service name to get lost and then try + // to claim it again. + // This notably happens with KCrash autostarts of Unique services. + waitForNameLost(); + + attemptRegistration(); + } + + if (d->registered) { + qDebug() << "registered weeh!"; + if (QCoreApplication *app = QCoreApplication::instance()) { + connect(app, SIGNAL(aboutToQuit()), this, SLOT(unregister())); + } + } + } + + void attemptRegistration() + { + qDebug() << Q_FUNC_INFO; + Q_ASSERT(!d->registered); d->registered = bus->registerService(d->serviceName) == QDBusConnectionInterface::ServiceRegistered; - if (!d->registered) { - if (options & Unique) { - // Already running so it's ok! - QVariantMap platform_data; - platform_data.insert(QStringLiteral("desktop-startup-id"), QString::fromUtf8(qgetenv("DESKTOP_STARTUP_ID"))); - if (QCoreApplication::arguments().count() > 1) { - OrgKdeKDBusServiceInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); - iface.setTimeout(5 * 60 * 1000); // Application can take time to answer - QDBusReply reply = iface.CommandLine(QCoreApplication::arguments(), QDir::currentPath(), platform_data); - if (reply.isValid()) { - exit(reply.value()); - } else { - d->errorMessage = reply.error().message(); - } + if (d->registered) { + return; + } + + if (options & KDBusService::Unique) { + // Already running so it's ok! + QVariantMap platform_data; + platform_data.insert(QStringLiteral("desktop-startup-id"), QString::fromUtf8(qgetenv("DESKTOP_STARTUP_ID"))); + if (QCoreApplication::arguments().count() > 1) { + OrgKdeKDBusServiceInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); + iface.setTimeout(5 * 60 * 1000); // Application can take time to answer + QDBusReply reply = iface.CommandLine(QCoreApplication::arguments(), QDir::currentPath(), platform_data); + if (reply.isValid()) { + exit(reply.value()); } else { - OrgFreedesktopApplicationInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); - iface.setTimeout(5 * 60 * 1000); // Application can take time to answer - QDBusReply reply = iface.Activate(platform_data); - if (reply.isValid()) { - exit(0); - } else { - d->errorMessage = reply.error().message(); - } + d->errorMessage = reply.error().message(); } } else { - d->errorMessage = QLatin1String("Couldn't register name '") - + d->serviceName - + QLatin1String("' with DBUS - another process owns it already!"); + OrgFreedesktopApplicationInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); + iface.setTimeout(5 * 60 * 1000); // Application can take time to answer + QDBusReply reply = iface.Activate(platform_data); + if (reply.isValid()) { + exit(0); + } else { + d->errorMessage = reply.error().message(); + } } - } else { - if (QCoreApplication *app = QCoreApplication::instance()) { - connect(app, SIGNAL(aboutToQuit()), this, SLOT(unregister())); - } + d->errorMessage = QLatin1String("Couldn't register name '") + + d->serviceName + + QLatin1String("' with DBUS - another process owns it already!"); } } - if (!d->registered && ((options & NoExitOnFailure) == 0)) { - qCritical() << d->errorMessage; - exit(1); + void waitForNameLost() + { + bus->connection().connect(QStringLiteral("org.freedesktop.DBus"), + QStringLiteral("/org/freedesktop/DBus"), + QStringLiteral("org.freedesktop.DBus"), + QStringLiteral("NameLost"), + this, + SLOT(onNameLost(QString,QDBusMessage))); + QTimer quitTimer; + quitTimer.start(250); + connect(&quitTimer, &QTimer::timeout, &nameLostLoop, &QEventLoop::quit); + nameLostLoop.exec(); } + + void onNameLost(const QString &name, QDBusMessage) + { + qDebug() << Q_FUNC_INFO << name; + // our name got free'd + nameLostLoop.quit(); + } + + QDBusConnectionInterface *bus = nullptr; + KDBusServicePrivate *d = nullptr; + KDBusService::StartupOptions options; + QEventLoop nameLostLoop; + QString objectPath; +}; + +KDBusService::KDBusService(StartupOptions options, QObject *parent) + : QObject(parent), d(new KDBusServicePrivate) +{ + new KDBusServiceAdaptor(this); + new KDBusServiceExtensionsAdaptor(this); + + Registration registration(d, options); + registration.run(); } KDBusService::~KDBusService() @@ -240,3 +324,5 @@ int KDBusService::CommandLine(const QStringList &arguments, const QString &worki // TODO (via hook) KStartupInfo::appStarted(platform_data.value("desktop-startup-id")) return d->exitValue; } + +#include "kdbusservice.moc"