Suspend processing of some messages in the default busses by default

To retain a bit compatibility with applications developed in the last 9
years that expect that QDBusConnections won't process their events until
the event loop runs, we now suspend the handling of incoming messages
in the two default buses (and only in them) and resume when the event
loop starts. This is required because the new threaded QtDBus would
otherwise process incoming messages that the application didn't expect
it to.

For example, if the application first acquires names on the bus and only
after that registers objects with QtDBus, there's a small window in
which the name is acquired and visible to other applications, but no
objects are registered yet. Calls to those objects may be received,
would then be processed in the QDBusConnectionManager thread and fail.

The work around is to disable the actual handling of method calls and
signals in QDBusConnectionPrivate::handleMessage. Instead, those
messages are queued until later.

Due to the way that libdbus-1 works, outgoing method calls that are
waiting for replies are not affected, since their processing does not
happen in handleMessage().

[ChangeLog][Important Behavior Changes] QtDBus now uses threads to
implement processing of incoming and outgoing messages. This solves a
number of thread safety issues and fixes an architectural problem that
would cause all processing to stop if a particular thread (usually the
main thread) were blocked in any operation. On the flip side, application
developers need to know that modifications to a QDBusConnection may be
visible immediately on the connection, so they should be done in an
order that won't allow for incomplete states to be observed (for
example, first register all objects, then acquire service names).

Change-Id: I39cc61d0d59846ab8c23ffff1423c6d555f6ee0a
Reviewed-by: David Faure <david.faure@kdab.com>

1 files changed, 42 insertions, 3 deletions

diff --git a/src/dbus/qdbusconnection.cpp b/src/dbus/qdbusconnection.cpp
index e9196173ad..0f2d799b92 100644
--- a/src/dbus/qdbusconnection.cpp
+++ b/src/dbus/qdbusconnection.cpp
@@ -38,6 +38,7 @@
 #include <qdebug.h>
 #include <qcoreapplication.h>
 #include <qstringlist.h>
+#include <qtimer.h>
 #include <qthread.h>
 
 #include "qdbusconnectioninterface.h"
@@ -59,6 +60,24 @@ QT_BEGIN_NAMESPACE
 
 Q_GLOBAL_STATIC(QDBusConnectionManager, _q_manager)
 
+// can be replaced with a lambda in Qt 5.7
+class QDBusConnectionDispatchEnabler : public QObject
+{
+    Q_OBJECT
+    QDBusConnectionPrivate *con;
+public:
+    QDBusConnectionDispatchEnabler(QDBusConnectionPrivate *con) : con(con) {}
+
+public slots:
+    void execute()
+    {
+        con->setDispatchEnabled(true);
+        if (!con->ref.deref())
+            con->deleteLater();
+        deleteLater();
+    }
+};
+
 struct QDBusConnectionManager::ConnectionRequestData
 {
     enum RequestType {
@@ -74,6 +93,8 @@ struct QDBusConnectionManager::ConnectionRequestData
     const QString *name;
 
     QDBusConnectionPrivate *result;
+
+    bool suspendedDelivery;
 };
 
 QDBusConnectionPrivate *QDBusConnectionManager::busConnection(QDBusConnection::BusType type)
@@ -84,6 +105,10 @@ QDBusConnectionPrivate *QDBusConnectionManager::busConnection(QDBusConnection::B
     if (!qdbus_loadLibDBus())
         return 0;
 
+    // we'll start in suspended delivery mode if we're in the main thread
+    // (the event loop will resume delivery)
+    bool suspendedDelivery = qApp && qApp->thread() == QThread::currentThread();
+
     QMutexLocker lock(&defaultBusMutex);
     if (defaultBuses[type])
         return defaultBuses[type];
@@ -91,7 +116,7 @@ QDBusConnectionPrivate *QDBusConnectionManager::busConnection(QDBusConnection::B
     QString name = QStringLiteral("qt_default_session_bus");
     if (type == QDBusConnection::SystemBus)
         name = QStringLiteral("qt_default_system_bus");
-    return defaultBuses[type] = connectToBus(type, name);
+    return defaultBuses[type] = connectToBus(type, name, suspendedDelivery);
 }
 
 QDBusConnectionPrivate *QDBusConnectionManager::connection(const QString &name) const
@@ -169,14 +194,22 @@ void QDBusConnectionManager::run()
     moveToThread(Q_NULLPTR);
 }
 
-QDBusConnectionPrivate *QDBusConnectionManager::connectToBus(QDBusConnection::BusType type, const QString &name)
+QDBusConnectionPrivate *QDBusConnectionManager::connectToBus(QDBusConnection::BusType type, const QString &name,
+                                                             bool suspendedDelivery)
 {
     ConnectionRequestData data;
     data.type = ConnectionRequestData::ConnectToStandardBus;
     data.busType = type;
     data.name = &name;
+    data.suspendedDelivery = suspendedDelivery;
 
     emit connectionRequested(&data);
+    if (suspendedDelivery) {
+        data.result->ref.ref();
+        QDBusConnectionDispatchEnabler *o = new QDBusConnectionDispatchEnabler(data.result);
+        QTimer::singleShot(0, o, SLOT(execute()));
+        o->moveToThread(qApp->thread());    // qApp was checked in the caller
+    }
     return data.result;
 }
 
@@ -186,6 +219,7 @@ QDBusConnectionPrivate *QDBusConnectionManager::connectToBus(const QString &addr
     data.type = ConnectionRequestData::ConnectToBusByAddress;
     data.busAddress = &address;
     data.name = &name;
+    data.suspendedDelivery = false;
 
     emit connectionRequested(&data);
     return data.result;
@@ -197,6 +231,7 @@ QDBusConnectionPrivate *QDBusConnectionManager::connectToPeer(const QString &add
     data.type = ConnectionRequestData::ConnectToPeerByAddress;
     data.busAddress = &address;
     data.name = &name;
+    data.suspendedDelivery = false;
 
     emit connectionRequested(&data);
     return data.result;
@@ -252,6 +287,8 @@ void QDBusConnectionManager::executeConnectionRequest(QDBusConnectionManager::Co
         // will lock in QDBusConnectionPrivate::connectRelay()
         d->setConnection(c, error);
         d->createBusService();
+        if (data->suspendedDelivery)
+            d->setDispatchEnabled(false);
     }
 }
 
@@ -456,7 +493,7 @@ QDBusConnection QDBusConnection::connectToBus(BusType type, const QString &name)
         QDBusConnectionPrivate *d = 0;
         return QDBusConnection(d);
     }
-    return QDBusConnection(_q_manager()->connectToBus(type, name));
+    return QDBusConnection(_q_manager()->connectToBus(type, name, false));
 }
 
 /*!
@@ -1232,4 +1269,6 @@ QByteArray QDBusConnection::localMachineId()
 
 QT_END_NAMESPACE
 
+#include "qdbusconnection.moc"
+
 #endif // QT_NO_DBUS