QDBusConnectionPrivate: Fix handling of queued messages

Handle any queued messages before attempting to dispatch
any newly received messages. This ensures that messages are
processed in the order they were sent.

Add a regression test for this bug using code adapted
from the bug report by Pascal Weisser. Because of the
nature of the bug, this new test does not always fail
even when compiled with affected versions of Qt though.

Fixes: QTBUG-105457
Pick-to: 6.2 6.5
Change-Id: I2725f3450ad537d63d6660e21645ac2c578e1768
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/dbus/qdbusintegrator.cpp

@@ -1173,7 +1173,6 @@ void QDBusConnectionPrivate::timerEvent(QTimerEvent *e)
 void QDBusConnectionPrivate::doDispatch()
 {
     if (mode == ClientMode || mode == PeerMode) {
-        while (q_dbus_connection_dispatch(connection) == DBUS_DISPATCH_DATA_REMAINS) ;
         if (dispatchEnabled && !pendingMessages.isEmpty()) {
             // dispatch previously queued messages
             PendingMessageList::Iterator it = pendingMessages.begin();
@@ -1184,6 +1183,7 @@ void QDBusConnectionPrivate::doDispatch()
             }
             pendingMessages.clear();
         }
+        while (q_dbus_connection_dispatch(connection) == DBUS_DISPATCH_DATA_REMAINS) ;
     }
 }
 

tests/auto/dbus/CMakeLists.txt

@@ -6,6 +6,7 @@ add_subdirectory(qdbusconnection)
 add_subdirectory(qdbusconnection_no_app)
 add_subdirectory(qdbusconnection_no_bus)
 add_subdirectory(qdbusconnection_no_libdbus)
+add_subdirectory(qdbusconnection_signalorder)
 add_subdirectory(qdbusconnection_spyhook)
 add_subdirectory(qdbuscontext)
 add_subdirectory(qdbuslocalcalls)

tests/auto/dbus/qdbusconnection_signalorder/CMakeLists.txt

@@ -0,0 +1,13 @@
+# Copyright (C) 2023 The Qt Company Ltd.
+# SPDX-License-Identifier: BSD-3-Clause
+
+#####################################################################
+## tst_qdbusconnection_signalorder Test:
+#####################################################################
+
+qt_internal_add_test(tst_qdbusconnection_signalorder
+    SOURCES
+        tst_qdbusconnection_signalorder.cpp
+    LIBRARIES
+        Qt::DBus
+)

tests/auto/dbus/qdbusconnection_signalorder/tst_qdbusconnection_signalorder.cpp

@@ -0,0 +1,103 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include <QCoreApplication>
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QObject>
+#include <QTest>
+#include <QTimer>
+
+using namespace Qt::StringLiterals;
+
+static constexpr int MAX_TEST_DURATION_MS = 10000;
+static constexpr int NUM_MESSAGES = 20;
+
+class SignalReceiver : public QObject
+{
+    Q_OBJECT
+public:
+    explicit SignalReceiver(QDBusConnection &connection, const QString &serviceName);
+
+    int nextValue() const { return m_nextValue; }
+    bool inOrder() const { return m_inOrder; }
+
+Q_SIGNALS:
+    void testSignal(int);
+    void done();
+
+private Q_SLOTS:
+    void testSlot(int number);
+
+private:
+    int m_nextValue = 0;
+    bool m_inOrder = true;
+};
+
+SignalReceiver::SignalReceiver(QDBusConnection &connection, const QString &serviceName)
+{
+    connection.connect(serviceName, "/", {}, "testSignal", this, SLOT(testSlot(int)));
+}
+
+void SignalReceiver::testSlot(int number)
+{
+    if (m_nextValue != number) {
+        m_inOrder = false;
+        qWarning("Message out of sequence, expected: %d, received: %d", m_nextValue, number);
+    }
+
+    m_nextValue++;
+
+    if (m_nextValue == NUM_MESSAGES) {
+        Q_EMIT done();
+    }
+}
+
+class tst_QDBusConnection_SignalOrder : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void signalOrder();
+};
+
+// This is a regression test for QTBUG-105457. The bug is a race condition,
+// so it cannot be reliably triggered at each test execution.
+void tst_QDBusConnection_SignalOrder::signalOrder()
+{
+    int argc = 1;
+    static char appName[] = "tst_qdbusconnection_signalorder";
+    char *argv[] = { appName, 0 };
+    QCoreApplication app(argc, argv);
+
+    const QString serviceName =
+            u"org.qtproject.tst_dbusconnection_signalorder_%1"_s.arg(app.applicationPid());
+
+    auto connection = QDBusConnection::sessionBus();
+
+    QVERIFY(connection.isConnected());
+    QVERIFY(connection.registerService(serviceName));
+
+    // Limit the test execution time in case if something goes wrong inside
+    // the signal receiver.
+    QTimer::singleShot(MAX_TEST_DURATION_MS, &app, &QCoreApplication::quit);
+
+    SignalReceiver signalReceiver(connection, serviceName);
+    connect(&signalReceiver, &SignalReceiver::done, &app, &QCoreApplication::quit);
+
+    QVERIFY(connection.registerObject("/", &signalReceiver, QDBusConnection::ExportAllSlots));
+
+    for (int i = 0; i < NUM_MESSAGES; i++) {
+        auto testSignal = QDBusMessage::createSignal("/", serviceName, "testSignal");
+        testSignal << i;
+        QVERIFY(connection.send(testSignal));
+    }
+
+    app.exec();
+
+    QVERIFY(signalReceiver.inOrder());
+    QCOMPARE(signalReceiver.nextValue(), NUM_MESSAGES);
+}
+
+QTEST_APPLESS_MAIN(tst_QDBusConnection_SignalOrder)
+
+#include "tst_qdbusconnection_signalorder.moc"