From 3c94288fd993ba61c7271be30b36e1c95eb19f25 Mon Sep 17 00:00:00 2001
From: Alessandro Astone <ales.astone@gmail.com>
Date: Fri, 21 Jul 2023 21:44:26 +0200
Subject: [PATCH] Do not destroy the transaction before it has replied

Destroy might be signaled before the dbus call has replied.

In destroy() we emit a `finished` event with code ExitUnknown, but
if the transaction returns an error we should have sent ExitFailed.

Furthermore, if we destroy the transaction object before
QDBusPendingCallWatcher fires, we might lose that event.
---
 src/transactionprivate.cpp | 18 +++++++++++++++---
 src/transactionprivate.h   |  2 ++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/transactionprivate.cpp b/src/transactionprivate.cpp
index b4b44b7..5e219a1 100644
--- a/src/transactionprivate.cpp
+++ b/src/transactionprivate.cpp
@@ -188,14 +188,18 @@ void TransactionPrivate::runQueuedTransaction()
         return;
     }
 
-    if (reply.isFinished() && reply.isError()) {
-        q->errorCode(Transaction::ErrorInternalError, reply.error().message());
-        finished(Transaction::ExitFailed, 0);
+    if (reply.isFinished()) {
+        receivedReply = true;
+        if (reply.isError()) {
+            q->errorCode(Transaction::ErrorInternalError, reply.error().message());
+            finished(Transaction::ExitFailed, 0);
+        }
         return;
     }
     auto watcher = new QDBusPendingCallWatcher(reply, q);
     q->connect(watcher, &QDBusPendingCallWatcher::finished,
                q, [this, q] (QDBusPendingCallWatcher *call) {
+        receivedReply = true;
         QDBusPendingReply<> reply = *call;
         if (reply.isError()) {
             QDBusError error = reply.error();
@@ -204,6 +208,8 @@ void TransactionPrivate::runQueuedTransaction()
             q->errorCode(transactionError, error.message());
             finished(Transaction::ExitFailed, 0);
             destroy();
+        } else if (destroyOnReply) {
+            destroy();
         }
         call->deleteLater();
     });
@@ -248,6 +254,12 @@ void TransactionPrivate::finished(uint exitCode, uint runtime)
 void TransactionPrivate::destroy()
 {
     Q_Q(Transaction);
+
+    if (!receivedReply) {
+        destroyOnReply = true;
+        return;
+    }
+
     if (p) {
        delete p;
        p = nullptr;
diff --git a/src/transactionprivate.h b/src/transactionprivate.h
index 69217a8..bbdf87d 100644
--- a/src/transactionprivate.h
+++ b/src/transactionprivate.h
@@ -82,6 +82,8 @@ class TransactionPrivate
     uint uid = 0;
     QString senderName;
     bool sentFinished = false;
+    bool receivedReply = false;
+    bool destroyOnReply = false;
     bool allowCancel = false;
     bool callerActive = false;