From ab9fab9763277783363f8c6d4b62405c3b0b0413 Mon Sep 17 00:00:00 2001
From: Chris Adams <chris.adams@jollamobile.com>
Date: Wed, 31 Jul 2019 12:45:14 +1000
Subject: [PATCH] Don't emit QObject::destroyed() within Identity::destroy()

QObject::destroyed() should not be emitted manually, as that can
cause unwanted side effects.

Specifically, in this case, the
QDBusConnectionPrivate::objectDestroyed() slot was invoked with
invalidated object parameter (perhaps due to duplicate invocation)
resulting in a warning in QObject::disconnect().

Instead, ensure the object is unregistered from DBus immediately.
---
 src/signond/signondaemonadaptor.cpp | 29 ++++++++++++++++++++++++++++-
 src/signond/signondaemonadaptor.h   |  3 +++
 src/signond/signonidentity.cpp      | 13 ++++++++-----
 src/signond/signonidentity.h        |  1 +
 4 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/src/signond/signondaemonadaptor.cpp b/src/signond/signondaemonadaptor.cpp
index 8b35e4bd..abd8fd3a 100644
--- a/src/signond/signondaemonadaptor.cpp
+++ b/src/signond/signondaemonadaptor.cpp
@@ -29,6 +29,13 @@
 
 namespace SignonDaemonNS {
 
+struct RegisteredIdentity {
+    RegisteredIdentity(const QDBusConnection &connection, QObject *identity)
+        : conn(connection), ident(identity) {}
+    QDBusConnection conn;
+    QObject *ident = nullptr;
+};
+
 SignonDaemonAdaptor::SignonDaemonAdaptor(SignonDaemon *parent):
     QDBusAbstractAdaptor(parent),
     m_parent(parent)
@@ -38,6 +45,7 @@ SignonDaemonAdaptor::SignonDaemonAdaptor(SignonDaemon *parent):
 
 SignonDaemonAdaptor::~SignonDaemonAdaptor()
 {
+    qDeleteAll(m_registeredIdentities);
 }
 
 void SignonDaemonAdaptor::registerNewIdentity(const QString &applicationContext,
@@ -46,7 +54,10 @@ void SignonDaemonAdaptor::registerNewIdentity(const QString &applicationContext,
     Q_UNUSED(applicationContext);
 
     QObject *identity = m_parent->registerNewIdentity();
-    objectPath = registerObject(parentDBusContext().connection(), identity);
+    QDBusConnection dbusConnection(parentDBusContext().connection());
+    objectPath = registerObject(dbusConnection, identity);
+    m_registeredIdentities.append(new RegisteredIdentity(dbusConnection, identity));
+    connect(identity, SIGNAL(unregistered()), this, SLOT(onIdentityUnregistered()));
 
     SignonDisposable::destroyUnused();
 }
@@ -130,6 +141,22 @@ void SignonDaemonAdaptor::getIdentity(const quint32 id,
     SignonDisposable::destroyUnused();
 }
 
+void SignonDaemonAdaptor::onIdentityUnregistered()
+{
+    QObject *ident = sender();
+    if (!ident) {
+        return;
+    }
+
+    for (int i = 0; i < m_registeredIdentities.size(); ++i) {
+        if (m_registeredIdentities[i]->ident == ident) {
+            m_registeredIdentities[i]->conn.unregisterObject(ident->objectName());
+            delete m_registeredIdentities.takeAt(i);
+            return;
+        }
+    }
+}
+
 void SignonDaemonAdaptor::onIdentityAccessReplyFinished()
 {
     SignOn::AccessReply *reply = qobject_cast<SignOn::AccessReply*>(sender());
diff --git a/src/signond/signondaemonadaptor.h b/src/signond/signondaemonadaptor.h
index db8d875f..1c20cac3 100644
--- a/src/signond/signondaemonadaptor.h
+++ b/src/signond/signondaemonadaptor.h
@@ -34,6 +34,7 @@
 namespace SignonDaemonNS {
 
 typedef QList<QVariantMap> MapList;
+class RegisteredIdentity;
 
 class SignonDaemonAdaptor: public QDBusAbstractAdaptor
 {
@@ -74,10 +75,12 @@ private:
                                    QObject *object);
 
 private Q_SLOTS:
+    void onIdentityUnregistered();
     void onIdentityAccessReplyFinished();
     void onAuthSessionAccessReplyFinished();
 
 private:
+    QList<RegisteredIdentity*> m_registeredIdentities;
     SignonDaemon *m_parent;
 }; //class SignonDaemonAdaptor
 
diff --git a/src/signond/signonidentity.cpp b/src/signond/signonidentity.cpp
index ce1ecfb0..a143c223 100644
--- a/src/signond/signonidentity.cpp
+++ b/src/signond/signonidentity.cpp
@@ -84,7 +84,8 @@ private:
 SignonIdentity::SignonIdentity(quint32 id, int timeout,
                                SignonDaemon *parent):
     SignonDisposable(timeout, parent),
-    m_pInfo(NULL)
+    m_pInfo(NULL),
+    m_destroyed(false)
 {
     m_id = id;
 
@@ -112,7 +113,10 @@ SignonIdentity::SignonIdentity(quint32 id, int timeout,
 
 SignonIdentity::~SignonIdentity()
 {
-    emit unregistered();
+    if (!m_destroyed) {
+        m_destroyed = true;
+        Q_EMIT unregistered();
+    }
 
     delete m_signonui;
     delete m_pInfo;
@@ -125,9 +129,8 @@ SignonIdentity *SignonIdentity::createIdentity(quint32 id, SignonDaemon *parent)
 
 void SignonIdentity::destroy()
 {
-    /* Emitting the destroyed signal makes QDBusConnection unregister the
-     * object */
-    Q_EMIT destroyed();
+    m_destroyed = true;
+    Q_EMIT unregistered();
     deleteLater();
 }
 
diff --git a/src/signond/signonidentity.h b/src/signond/signonidentity.h
index 9ec9be4e..f6321f30 100644
--- a/src/signond/signonidentity.h
+++ b/src/signond/signonidentity.h
@@ -96,6 +96,7 @@ private:
     quint32 m_id;
     SignonUiAdaptor *m_signonui;
     SignonIdentityInfo *m_pInfo;
+    bool m_destroyed;
 }; //class SignonDaemon
 
 } //namespace SignonDaemonNS
-- 
2.26.2