tst_QDBusAbstractInterface: fix racy test failing every so often

The explanation is in the code comment. Ever since QDBusConnections
began being processed in a separate thread, we were relying on the fact
that the main thread didn't begin processing its event queue until the
second event got posted (the event loop only exits after it has finished
processing all pending events). We had a race between the main thread
starting its processing, at which point it decides which is the last
event it will process, and the QDBusConnectionManager thread posting the
second event.

This is very fragile code, since it depends on the behavior of
QDBusConnectionPrivate (how it stores the signal relays in a hash) and
that of QHash with duplicate keys. This only works because the hash
key between the two connections is the same (it's only dependent on the
method name and interface name). If we ever begin using something that
isn't the same between "control" and "p", then with QHash's randomness,
we'll be racy again.

Change-Id: I42e7ef1a481840699a8dffff1406c3a4674ec3a6
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

1 files changed, 8 insertions, 2 deletions

diff --git a/tests/auto/dbus/qdbusabstractinterface/tst_qdbusabstractinterface.cpp b/tests/auto/dbus/qdbusabstractinterface/tst_qdbusabstractinterface.cpp
index 403e500d8c..cd23f2cb47 100644
--- a/tests/auto/dbus/qdbusabstractinterface/tst_qdbusabstractinterface.cpp
+++ b/tests/auto/dbus/qdbusabstractinterface/tst_qdbusabstractinterface.cpp
@@ -1012,8 +1012,14 @@ void tst_QDBusAbstractInterface::followSignal()
     QVERIFY(!con.interface()->isServiceRegistered(serviceToFollow));
     Pinger control = getPinger("");
 
-    // we need to connect the signal somewhere in order for D-Bus to enable the rules
-    QTestEventLoop::instance().connect(p.data(), SIGNAL(voidSignal()), SLOT(exitLoop()));
+    // connect our test signal
+    // FRAGILE CODE AHEAD:
+    // Connection order is important: we connect the control first because that
+    // needs to be delivered last, to ensure that we don't exitLoop() before
+    // the signal delivery to QSignalSpy is posted to the current thread. That
+    // happens because QDBusConnectionPrivate runs in a separate thread and
+    // uses a QMultiHash and insertMulti prepends to the list of items with the
+    // same key.
     QTestEventLoop::instance().connect(control.data(), SIGNAL(voidSignal()), SLOT(exitLoop()));
     QSignalSpy s(p.data(), SIGNAL(voidSignal()));