Remove the DBusMessage* members from QDBusMessagePrivate

We don't need to keep the entire original message in QDBusMessagePrivate
after we've demarshalled it, nor do we need to keep a link to the
message we're replying to. In order to reply properly to a message, we
only need two pieces of information: the serial of the method call being
replied to and the service that originated the call.

Fixes: QTBUG-69919
Change-Id: Ic5d393bfd36e48a193fcffff13b7375db459e619
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>

3 files changed, 49 insertions, 37 deletions

diff --git a/src/dbus/qdbusmessage.cpp b/src/dbus/qdbusmessage.cpp
index 6372fe0496..a43a86df22 100644
--- a/src/dbus/qdbusmessage.cpp
+++ b/src/dbus/qdbusmessage.cpp
@@ -36,22 +36,37 @@ static inline const char *data(const QByteArray &arr)
 }
 
 QDBusMessagePrivate::QDBusMessagePrivate()
-    : msg(nullptr), reply(nullptr), localReply(nullptr), ref(1), type(QDBusMessage::InvalidMessage),
-      delayedReply(false), localMessage(false),
-      parametersValidated(false), autoStartService(true),
-      interactiveAuthorizationAllowed(false)
+    : localReply(nullptr), ref(1), type(QDBusMessage::InvalidMessage),
+      delayedReply(false), parametersValidated(false),
+      localMessage(false), autoStartService(true),
+      interactiveAuthorizationAllowed(false), isReplyRequired(false)
 {
 }
 
 QDBusMessagePrivate::~QDBusMessagePrivate()
 {
-    if (msg)
-        q_dbus_message_unref(msg);
-    if (reply)
-        q_dbus_message_unref(reply);
     delete localReply;
 }
 
+void QDBusMessagePrivate::createResponseLink(const QDBusMessagePrivate *call)
+{
+    if (Q_UNLIKELY(call->type != QDBusMessage::MethodCallMessage)) {
+        qWarning("QDBusMessage: replying to a message that isn't a method call");
+        return;
+    }
+
+    if (call->localMessage) {
+        localMessage = true;
+        call->localReply = new QDBusMessage(*this); // keep an internal copy
+    } else {
+        serial = call->serial;
+        service = call->service;
+    }
+
+    // the reply must have a serial or be a local-loop optimization
+    Q_ASSERT(serial || localMessage);
+}
+
 /*!
     \since 4.3
      Returns the human-readable message associated with the error that was received.
@@ -113,8 +128,8 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB
     case QDBusMessage::ReplyMessage:
         msg = q_dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
         if (!d_ptr->localMessage) {
-            q_dbus_message_set_destination(msg, q_dbus_message_get_sender(d_ptr->reply));
-            q_dbus_message_set_reply_serial(msg, q_dbus_message_get_serial(d_ptr->reply));
+            q_dbus_message_set_destination(msg, data(d_ptr->service.toUtf8()));
+            q_dbus_message_set_reply_serial(msg, d_ptr->serial);
         }
         break;
     case QDBusMessage::ErrorMessage:
@@ -126,8 +141,8 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB
         msg = q_dbus_message_new(DBUS_MESSAGE_TYPE_ERROR);
         q_dbus_message_set_error_name(msg, d_ptr->name.toUtf8());
         if (!d_ptr->localMessage) {
-            q_dbus_message_set_destination(msg, q_dbus_message_get_sender(d_ptr->reply));
-            q_dbus_message_set_reply_serial(msg, q_dbus_message_get_serial(d_ptr->reply));
+            q_dbus_message_set_destination(msg, data(d_ptr->service.toUtf8()));
+            q_dbus_message_set_reply_serial(msg, d_ptr->serial);
         }
         break;
     case QDBusMessage::SignalMessage:
@@ -204,6 +219,7 @@ QDBusMessage QDBusMessagePrivate::fromDBusMessage(DBusMessage *dmsg, QDBusConnec
         return message;
 
     message.d_ptr->type = QDBusMessage::MessageType(q_dbus_message_get_type(dmsg));
+    message.d_ptr->serial = q_dbus_message_get_serial(dmsg);
     message.d_ptr->path = QString::fromUtf8(q_dbus_message_get_path(dmsg));
     message.d_ptr->interface = QString::fromUtf8(q_dbus_message_get_interface(dmsg));
     message.d_ptr->name = message.d_ptr->type == DBUS_MESSAGE_TYPE_ERROR ?
@@ -212,7 +228,7 @@ QDBusMessage QDBusMessagePrivate::fromDBusMessage(DBusMessage *dmsg, QDBusConnec
     message.d_ptr->service = QString::fromUtf8(q_dbus_message_get_sender(dmsg));
     message.d_ptr->signature = QString::fromUtf8(q_dbus_message_get_signature(dmsg));
     message.d_ptr->interactiveAuthorizationAllowed = q_dbus_message_get_allow_interactive_authorization(dmsg);
-    message.d_ptr->msg = q_dbus_message_ref(dmsg);
+    message.d_ptr->isReplyRequired = !q_dbus_message_get_no_reply(dmsg);
 
     QDBusDemarshaller demarshaller(capabilities);
     demarshaller.message = q_dbus_message_ref(dmsg);
@@ -402,6 +418,7 @@ QDBusMessage QDBusMessage::createMethodCall(const QString &service, const QStrin
     message.d_ptr->path = path;
     message.d_ptr->interface = interface;
     message.d_ptr->name = method;
+    message.d_ptr->isReplyRequired = true;
 
     return message;
 }
@@ -444,15 +461,7 @@ QDBusMessage QDBusMessage::createReply(const QVariantList &arguments) const
     QDBusMessage reply;
     reply.setArguments(arguments);
     reply.d_ptr->type = ReplyMessage;
-    if (d_ptr->msg)
-        reply.d_ptr->reply = q_dbus_message_ref(d_ptr->msg);
-    if (d_ptr->localMessage) {
-        reply.d_ptr->localMessage = true;
-        d_ptr->localReply = new QDBusMessage(reply); // keep an internal copy
-    }
-
-    // the reply must have a msg or be a local-loop optimization
-    Q_ASSERT(reply.d_ptr->reply || reply.d_ptr->localMessage);
+    reply.d_ptr->createResponseLink(d_ptr);
     return reply;
 }
 
@@ -463,15 +472,7 @@ QDBusMessage QDBusMessage::createReply(const QVariantList &arguments) const
 QDBusMessage QDBusMessage::createErrorReply(const QString &name, const QString &msg) const
 {
     QDBusMessage reply = QDBusMessage::createError(name, msg);
-    if (d_ptr->msg)
-        reply.d_ptr->reply = q_dbus_message_ref(d_ptr->msg);
-    if (d_ptr->localMessage) {
-        reply.d_ptr->localMessage = true;
-        d_ptr->localReply = new QDBusMessage(reply); // keep an internal copy
-    }
-
-    // the reply must have a msg or be a local-loop optimization
-    Q_ASSERT(reply.d_ptr->reply || reply.d_ptr->localMessage);
+    reply.d_ptr->createResponseLink(d_ptr);
     return reply;
 }
 
@@ -555,6 +556,8 @@ QDBusMessage &QDBusMessage::operator=(const QDBusMessage &other)
 */
 QString QDBusMessage::service() const
 {
+    if (d_ptr->type == ErrorMessage || d_ptr->type == ReplyMessage)
+        return QString();   // d_ptr->service holds the destination
     return d_ptr->service;
 }
 
@@ -617,9 +620,9 @@ bool QDBusMessage::isReplyRequired() const
     if (d_ptr->type != QDBusMessage::MethodCallMessage)
         return false;
 
-    if (!d_ptr->msg)
-        return d_ptr->localMessage; // if it's a local message, reply is required
-    return !q_dbus_message_get_no_reply(d_ptr->msg);
+    if (d_ptr->localMessage) // if it's a local message, reply is required
+        return true;
+    return d_ptr->isReplyRequired;
 }
 
 /*!
@@ -759,6 +762,12 @@ QDBusMessage &QDBusMessage::operator<<(const QVariant &arg)
     return *this;
 }
 
+QDBusMessage::QDBusMessage(QDBusMessagePrivate &dd)
+    : d_ptr(&dd)
+{
+    d_ptr->ref.ref();
+}
+
 /*!
     Returns the message type.
 */
diff --git a/src/dbus/qdbusmessage.h b/src/dbus/qdbusmessage.h
index 644a1170e3..819f786487 100644
--- a/src/dbus/qdbusmessage.h
+++ b/src/dbus/qdbusmessage.h
@@ -84,6 +84,7 @@ public:
     QDBusMessage &operator<<(const QVariant &arg);
 
 private:
+    QDBusMessage(QDBusMessagePrivate &dd);
     friend class QDBusMessagePrivate;
     QDBusMessagePrivate *d_ptr;
 };
diff --git a/src/dbus/qdbusmessage_p.h b/src/dbus/qdbusmessage_p.h
index cca1b4bd19..88ba78025e 100644
--- a/src/dbus/qdbusmessage_p.h
+++ b/src/dbus/qdbusmessage_p.h
@@ -39,20 +39,22 @@ public:
 
     // the following parameters are "const": they are not changed after the constructors
     // the parametersValidated member below controls whether they've been validated already
+    // (service is also used to store the destination of reply-type messages)
     QString service, path, interface, name, message, signature;
 
-    DBusMessage *msg;
-    DBusMessage *reply;
     mutable QDBusMessage *localReply;
     QAtomicInt ref;
     QDBusMessage::MessageType type;
+    uint32_t serial; // if type == MethodCall; the incoming serial; if type == Reply or Error, the serial we're replying to
 
     mutable uint delayedReply : 1;
-    uint localMessage : 1;
     mutable uint parametersValidated : 1;
+    uint localMessage : 1;
     uint autoStartService : 1;
     uint interactiveAuthorizationAllowed : 1;
+    uint isReplyRequired : 1;
 
+    void createResponseLink(const QDBusMessagePrivate *call);
     static void setParametersValidated(QDBusMessage &msg, bool enable)
     { msg.d_ptr->parametersValidated = enable; }