QDBusConnectionPrivate: Fix race in sendWithReply()

The message processing may finish before watcherHelper is
setup. Use locking and an additional check for message
processing completion to avoid the race condition.

Move the code to new function inside QDBusPendingCallPrivate,
close to where the waiting without GUI is done.

Add assertions that check that watcherHelper is not overwritten.

Change-Id: I24e54598135edf293c41b3a80369b0a3b46f775c
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/dbus/qdbusintegrator.cpp

@@ -2056,20 +2056,10 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message,
     QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, nullptr, nullptr, nullptr, timeout);
     Q_ASSERT(pcall);
 
-    if (pcall->replyMessage.type() == QDBusMessage::InvalidMessage) {
+    if (mode == QDBus::BlockWithGui)
-        // need to wait for the reply
+        pcall->waitForFinishedWithGui();
-        if (mode == QDBus::BlockWithGui) {
+    else
-            pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
-            QEventLoop loop;
-            loop.connect(pcall->watcherHelper, &QDBusPendingCallWatcherHelper::reply, &loop, &QEventLoop::quit);
-            loop.connect(pcall->watcherHelper, &QDBusPendingCallWatcherHelper::error, &loop, &QEventLoop::quit);
-
-            // enter the event loop and wait for a reply
-            loop.exec(QEventLoop::ExcludeUserInputEvents | QEventLoop::WaitForMoreEvents);
-        } else {
         pcall->waitForFinished();
-        }
-    }
 
     QDBusMessage reply = pcall->replyMessage;
     lastError = QDBusError(reply);      // set or clear error
@@ -2130,6 +2120,7 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
         pcall->setReplyCallback(receiver, returnMethod);
 
     if (errorMethod) {
+        Q_ASSERT(!pcall->watcherHelper);
         pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
         connect(pcall->watcherHelper, SIGNAL(error(QDBusError,QDBusMessage)), receiver, errorMethod,
                 Qt::QueuedConnection);

src/dbus/qdbuspendingcall.cpp

@@ -206,6 +206,26 @@ void QDBusPendingCallPrivate::waitForFinished()
     waitForFinishedCondition.wait(&mutex);
 }
 
+void QDBusPendingCallPrivate::waitForFinishedWithGui()
+{
+    QEventLoop loop;
+
+    {
+        const auto locker = qt_scoped_lock(mutex);
+        if (replyMessage.type() != QDBusMessage::InvalidMessage)
+            return; // already finished
+
+        Q_ASSERT(!watcherHelper);
+        watcherHelper = new QDBusPendingCallWatcherHelper;
+        loop.connect(watcherHelper, &QDBusPendingCallWatcherHelper::reply, &loop,
+                     &QEventLoop::quit);
+        loop.connect(watcherHelper, &QDBusPendingCallWatcherHelper::error, &loop,
+                     &QEventLoop::quit);
+    }
+
+    loop.exec(QEventLoop::ExcludeUserInputEvents | QEventLoop::WaitForMoreEvents);
+}
+
 /*!
     Creates a copy of the \a other pending asynchronous call. Note
     that both objects will refer to the same pending call.

src/dbus/qdbuspendingcall_p.h

@@ -68,6 +68,7 @@ public:
     ~QDBusPendingCallPrivate();
     bool setReplyCallback(QObject *target, const char *member);
     void waitForFinished();
+    void waitForFinishedWithGui();
     void setMetaTypes(int count, const QMetaType *types);
     void checkReceivedSignature();
 };