Change QDBusPendingCallPrivate to full reference counting for deletion.

Fixes race between QDBusConnectionPrivate::processFinishedCall()
releasing the mutex before emitting signals (using various members of
QDBusPendingCallPrivate) and deletion of the QDBusPendingCallPrivate
object through QDBusPendingCall::d's destructor (a member of type
QExplicitlySharedDataPointer<QDBusPendingCallPrivate>) leeds to
segmentation fault with CrashTest example on slow/single core
arm cpu).

Task-number: QTBUG-27809
Change-Id: I3590d74d1cfa5816ede764b50b83a7008ec780ff
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/dbus/qdbusconnection.cpp

@@ -54,6 +54,7 @@
 #include "qdbusinterface_p.h"
 #include "qdbusutil_p.h"
 #include "qdbusconnectionmanager_p.h"
+#include "qdbuspendingcall_p.h"
 
 #include "qdbusthreaddebug_p.h"
 
@@ -611,7 +612,7 @@ QDBusPendingCall QDBusConnection::asyncCall(const QDBusMessage &message, int tim
         return QDBusPendingCall(0); // null pointer -> disconnected
     }
 
-    QDBusPendingCallPrivate *priv = d->sendWithReplyAsync(message, timeout);
+    QDBusPendingCallPrivate *priv = d->sendWithReplyAsync(message, 0, 0, 0, timeout);
     return QDBusPendingCall(priv);
 }
 

src/dbus/qdbusconnection_p.h

@@ -199,9 +199,8 @@ public:
     int send(const QDBusMessage &message);
     QDBusMessage sendWithReply(const QDBusMessage &message, int mode, int timeout = -1);
     QDBusMessage sendWithReplyLocal(const QDBusMessage &message);
-    QDBusPendingCallPrivate *sendWithReplyAsync(const QDBusMessage &message, int timeout = -1);
+    QDBusPendingCallPrivate *sendWithReplyAsync(const QDBusMessage &message, QObject *receiver,
-    int sendWithReplyAsync(const QDBusMessage &message, QObject *receiver,
+                                                const char *returnMethod, const char *errorMethod,int timeout = -1);
-                           const char *returnMethod, const char *errorMethod, int timeout = -1);
     bool connectSignal(const QString &service, const QString &path, const QString& interface,
                        const QString &name, const QStringList &argumentMatch, const QString &signature,
                        QObject *receiver, const char *slot);

src/dbus/qdbusintegrator.cpp

@@ -1901,6 +1901,9 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
 
     if (msg.type() == QDBusMessage::ErrorMessage)
         emit connection->callWithCallbackFailed(QDBusError(msg), call->sentMessage);
+
+    if (!call->ref.deref())
+        delete call;
 }
 
 int QDBusConnectionPrivate::send(const QDBusMessage& message)
@@ -1983,7 +1986,7 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message,
 
         return amsg;
     } else { // use the event loop
-        QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, timeout);
+        QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, 0, 0, 0, timeout);
         Q_ASSERT(pcall);
 
         if (pcall->replyMessage.type() == QDBusMessage::InvalidMessage) {
@@ -1999,6 +2002,10 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message,
         QDBusMessage reply = pcall->replyMessage;
         lastError = QDBusError(reply);      // set or clear error
 
+        bool r = pcall->ref.deref();
+        Q_ASSERT(!r);
+        Q_UNUSED(r);
+
         delete pcall;
         return reply;
     }
@@ -2038,19 +2045,55 @@ QDBusMessage QDBusConnectionPrivate::sendWithReplyLocal(const QDBusMessage &mess
 }
 
 QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusMessage &message,
-                                                                    int timeout)
+                                                                    QObject *receiver, const char *returnMethod,
+                                                                    const char *errorMethod, int timeout)
 {
     if (isServiceRegisteredByThread(message.service())) {
         // special case for local calls
         QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate(message, this);
         pcall->replyMessage = sendWithReplyLocal(message);
+        if (receiver && returnMethod)
+            pcall->setReplyCallback(receiver, returnMethod);
 
+        if (errorMethod) {
+            pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
+            connect(pcall->watcherHelper, SIGNAL(error(QDBusError,QDBusMessage)), receiver, errorMethod,
+                    Qt::QueuedConnection);
+            pcall->watcherHelper->moveToThread(thread());
+        }
+
+        if ((receiver && returnMethod) || errorMethod) {
+           // no one waiting, will delete pcall in processFinishedCall()
+           pcall->ref.store(1);
+        } else {
+           // set double ref to prevent race between processFinishedCall() and ref counting
+           // by QDBusPendingCall::QExplicitlySharedDataPointer<QDBusPendingCallPrivate>
+           pcall->ref.store(2);
+        }
+        processFinishedCall(pcall);
         return pcall;
     }
 
     checkThread();
     QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate(message, this);
-    pcall->ref.store(0);
+    if (receiver && returnMethod)
+        pcall->setReplyCallback(receiver, returnMethod);
+
+    if (errorMethod) {
+        pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
+        connect(pcall->watcherHelper, SIGNAL(error(QDBusError,QDBusMessage)), receiver, errorMethod,
+                Qt::QueuedConnection);
+        pcall->watcherHelper->moveToThread(thread());
+    }
+
+    if ((receiver && returnMethod) || errorMethod) {
+       // no one waiting, will delete pcall in processFinishedCall()
+       pcall->ref.store(1);
+    } else {
+       // set double ref to prevent race between processFinishedCall() and ref counting
+       // by QDBusPendingCall::QExplicitlySharedDataPointer<QDBusPendingCallPrivate>
+       pcall->ref.store(2);
+    }
 
     QDBusError error;
     DBusMessage *msg = QDBusMessagePrivate::toDBusMessage(message, capabilities, &error);
@@ -2061,6 +2104,7 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
                  qPrintable(error.message()));
         pcall->replyMessage = QDBusMessage::createError(error);
         lastError = error;
+        processFinishedCall(pcall);
         return pcall;
     }
 
@@ -2086,45 +2130,10 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
 
     q_dbus_message_unref(msg);
     pcall->replyMessage = QDBusMessage::createError(error);
+    processFinishedCall(pcall);
     return pcall;
 }
 
-int QDBusConnectionPrivate::sendWithReplyAsync(const QDBusMessage &message, QObject *receiver,
-                                               const char *returnMethod, const char *errorMethod,
-                                               int timeout)
-{
-    QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, timeout);
-    Q_ASSERT(pcall);
-
-    // has it already finished with success (dispatched locally)?
-    if (pcall->replyMessage.type() == QDBusMessage::ReplyMessage) {
-        pcall->setReplyCallback(receiver, returnMethod);
-        processFinishedCall(pcall);
-        delete pcall;
-        return 1;
-    }
-
-    // either it hasn't finished or it has finished with error
-    if (errorMethod) {
-        pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
-        connect(pcall->watcherHelper, SIGNAL(error(QDBusError,QDBusMessage)), receiver, errorMethod,
-                Qt::QueuedConnection);
-        pcall->watcherHelper->moveToThread(thread());
-    }
-
-    // has it already finished and is an error reply message?
-    if (pcall->replyMessage.type() == QDBusMessage::ErrorMessage) {
-        processFinishedCall(pcall);
-        delete pcall;
-        return 1;
-    }
-
-    pcall->ref.ref();
-    pcall->setReplyCallback(receiver, returnMethod);
-
-    return 1;
-}
-
 bool QDBusConnectionPrivate::connectSignal(const QString &service,
                                            const QString &path, const QString &interface, const QString &name,
                                            const QStringList &argumentMatch, const QString &signature,

src/dbus/qdbuspendingcall.cpp

@@ -256,6 +256,11 @@ QDBusPendingCall::QDBusPendingCall(const QDBusPendingCall &other)
 QDBusPendingCall::QDBusPendingCall(QDBusPendingCallPrivate *dd)
     : d(dd)
 {
+    if (dd) {
+        bool r = dd->ref.deref();
+        Q_ASSERT(r);
+        Q_UNUSED(r);
+    }
 }
 
 /*!
@@ -469,6 +474,7 @@ QDBusPendingCall QDBusPendingCall::fromCompletedCall(const QDBusMessage &msg)
         msg.type() == QDBusMessage::ReplyMessage) {
         d = new QDBusPendingCallPrivate(QDBusMessage(), 0);
         d->replyMessage = msg;
+        d->ref.store(1);
     }
 
     return QDBusPendingCall(d);