From 87dc44dab87821360106bb5443f2930445938a5b Mon Sep 17 00:00:00 2001
From: Harald Sitter <sitter@kde.org>
Date: Mon, 6 Aug 2018 13:19:13 +0200
Subject: [PATCH] use QDBusServiceWatcher instead of pinging the client

Summary:
the motivation here is to terminate the "server" helper when the client
disappears. dbus supports this use case natively via service registration
events which we can easily handle via QDBusServiceWatcher.
instead of repeatedly poking the client we'll simply monitor its dbus
service now. this is cheaper, less code and doesn't risk timing out
randomly.

Test Plan:
- on neon ISO build kpmcore & calamares & pm
- calamares manages to actually partition stuff
- partitionmanager also starts properly

- also the same again on the installed system.

Reviewers: stikonas, bshah

Reviewed By: bshah

Subscribers: bshah

Differential Revision: https://phabricator.kde.org/D14646
---
 src/util/externalcommand.h         |  5 ++---
 src/util/externalcommandhelper.cpp | 34 ++++++++++++------------------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/util/externalcommand.h b/src/util/externalcommand.h
index 5561ddb..35b1a81 100644
--- a/src/util/externalcommand.h
+++ b/src/util/externalcommand.h
@@ -43,11 +43,10 @@ struct ExternalCommandPrivate;
 class DBusThread : public QThread
 {
     Q_OBJECT
+    // We register on DBus so the helper can monitor us and terminate if we
+    // terminate.
     Q_CLASSINFO("D-Bus Interface", "org.kde.kpmcore.applicationinterface")
     void run() override;
-
-public Q_SLOTS:
-    Q_SCRIPTABLE void ping() { return; };
 };
 
 /** An external command.
diff --git a/src/util/externalcommandhelper.cpp b/src/util/externalcommandhelper.cpp
index a0c1104..03adbf8 100644
--- a/src/util/externalcommandhelper.cpp
+++ b/src/util/externalcommandhelper.cpp
@@ -31,11 +31,12 @@
  *
  * KAuth helper runs in the background until application exits.
  * To avoid forever running helper in case of application crash
- * ExternalCommand class opens DBus interface that we ping.
- * If helper is not busy than it exits when ping fails. Otherwise,
+ * ExternalCommand class opens a DBus service that we monitor for changes.
+ * If helper is not busy then it exits when the client services gets
+ * unregistered. Otherwise,
  * we wait for the current job to finish before exiting, so even in case
  * of main application crash, we do not leave partially moved data.
- * 
+ *
  * This helper also starts another DBus interface where it listens to
  * command execution requests from the application that started the helper.
  * These requests are validated using public key cryptography, to prevent
@@ -65,25 +66,18 @@ ActionReply ExternalCommandHelper::init(const QVariantMap& args)
 
     m_loop = std::make_unique<QEventLoop>();
     HelperSupport::progressStep(QVariantMap());
-    auto timeout = [this] () {
-            auto *interface = new org::kde::kpmcore::applicationinterface(QStringLiteral("org.kde.kpmcore.applicationinterface"),
-                              QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
-            interface->setTimeout(2000); // 2 seconds;
-            auto pendingCall = interface->ping();
 
-            QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pendingCall, this);
-            auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
-                    if (watcher->isError()) {
-                        qWarning() << watcher->error();
-                        m_loop->exit();
-                    }
-                    };
-            connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
-    };
+    // End the loop and return only once the client is done using us.
+    auto serviceWatcher =
+            new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"),
+                                    QDBusConnection::systemBus(),
+                                    QDBusServiceWatcher::WatchForUnregistration,
+                                    this);
+    connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered,
+            [this]() {
+        m_loop->exit();
+    });
 
-    QTimer *timer = new QTimer(this);
-    connect(timer, &QTimer::timeout, this, timeout);
-    timer->start(5000); // 5 seconds
     m_loop->exec();
     reply.addData(QStringLiteral("success"), true);