And move the creation of connections to the thread

Now we know that all timers and socket notifiers get created only in the
QDBusConnectionManager thread.

Incidentally, this reduced code duplication.

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

6 files changed, 142 insertions, 95 deletions

diff --git a/src/dbus/qdbusconnection.cpp b/src/dbus/qdbusconnection.cpp
index 8ebdf4c66e..a5d674bb5a 100644
--- a/src/dbus/qdbusconnection.cpp
+++ b/src/dbus/qdbusconnection.cpp
@@ -59,6 +59,23 @@ QT_BEGIN_NAMESPACE
 
 Q_GLOBAL_STATIC(QDBusConnectionManager, _q_manager)
 
+struct QDBusConnectionManager::ConnectionRequestData
+{
+    enum RequestType {
+        ConnectToStandardBus,
+        ConnectToBusByAddress,
+        ConnectToPeerByAddress
+    } type;
+
+    union {
+        QDBusConnection::BusType busType;
+        const QString *busAddress;
+    };
+    const QString *name;
+
+    QDBusConnectionPrivate *result;
+};
+
 QDBusConnectionPrivate *QDBusConnectionManager::connection(const QString &name) const
 {
     return connectionHash.value(name, 0);
@@ -80,7 +97,11 @@ void QDBusConnectionManager::removeConnection(const QString &name)
 
 QDBusConnectionManager::QDBusConnectionManager()
 {
-    moveToThread(Q_NULLPTR);    // we don't handle events
+    connect(this, &QDBusConnectionManager::connectionRequested,
+            this, &QDBusConnectionManager::executeConnectionRequest, Qt::BlockingQueuedConnection);
+    connect(this, &QDBusConnectionManager::serverRequested,
+            this, &QDBusConnectionManager::createServer, Qt::BlockingQueuedConnection);
+    moveToThread(this);         // ugly, don't do this in other projects
     start();
 }
 
@@ -123,6 +144,103 @@ void QDBusConnectionManager::run()
         }
     }
     connectionHash.clear();
+
+    // allow deletion from any thread without warning
+    moveToThread(Q_NULLPTR);
+}
+
+QDBusConnectionPrivate *QDBusConnectionManager::connectToBus(QDBusConnection::BusType type, const QString &name)
+{
+    ConnectionRequestData data;
+    data.type = ConnectionRequestData::ConnectToStandardBus;
+    data.busType = type;
+    data.name = &name;
+
+    emit connectionRequested(&data);
+    return data.result;
+}
+
+QDBusConnectionPrivate *QDBusConnectionManager::connectToBus(const QString &address, const QString &name)
+{
+    ConnectionRequestData data;
+    data.type = ConnectionRequestData::ConnectToBusByAddress;
+    data.busAddress = &address;
+    data.name = &name;
+
+    emit connectionRequested(&data);
+    return data.result;
+}
+
+QDBusConnectionPrivate *QDBusConnectionManager::connectToPeer(const QString &address, const QString &name)
+{
+    ConnectionRequestData data;
+    data.type = ConnectionRequestData::ConnectToPeerByAddress;
+    data.busAddress = &address;
+    data.name = &name;
+
+    emit connectionRequested(&data);
+    return data.result;
+}
+
+void QDBusConnectionManager::executeConnectionRequest(QDBusConnectionManager::ConnectionRequestData *data)
+{
+    QMutexLocker locker(&mutex);
+    const QString &name = *data->name;
+    QDBusConnectionPrivate *&d = data->result;
+
+    // check if the connection exists by name
+    d = connection(name);
+    if (d || name.isEmpty())
+        return;
+
+    d = new QDBusConnectionPrivate;
+    DBusConnection *c = 0;
+    QDBusErrorInternal error;
+    switch (data->type) {
+    case ConnectionRequestData::ConnectToStandardBus:
+        switch (data->busType) {
+        case QDBusConnection::SystemBus:
+            c = q_dbus_bus_get_private(DBUS_BUS_SYSTEM, error);
+            break;
+        case QDBusConnection::SessionBus:
+            c = q_dbus_bus_get_private(DBUS_BUS_SESSION, error);
+            break;
+        case QDBusConnection::ActivationBus:
+            c = q_dbus_bus_get_private(DBUS_BUS_STARTER, error);
+            break;
+        }
+        break;
+
+    case ConnectionRequestData::ConnectToBusByAddress:
+    case ConnectionRequestData::ConnectToPeerByAddress:
+        c = q_dbus_connection_open_private(data->busAddress->toUtf8().constData(), error);
+        if (c && data->type == ConnectionRequestData::ConnectToBusByAddress) {
+            // register on the bus
+            if (!q_dbus_bus_register(c, error)) {
+                q_dbus_connection_unref(c);
+                c = 0;
+            }
+        }
+        break;
+    }
+
+    setConnection(name, d);
+    if (data->type == ConnectionRequestData::ConnectToPeerByAddress) {
+        d->setPeer(c, error);
+    } else {
+        // create the bus service
+        // will lock in QDBusConnectionPrivate::connectRelay()
+        d->setConnection(c, error);
+        d->createBusService();
+    }
+}
+
+void QDBusConnectionManager::createServer(const QString &address, void *server)
+{
+    QDBusErrorInternal error;
+    QDBusConnectionPrivate *d = new QDBusConnectionPrivate;
+    d->setServer(static_cast<QDBusServer *>(server),
+                 q_dbus_server_listen(address.toUtf8().constData(), error), error);
 }
 
 /*!
@@ -318,39 +436,7 @@ QDBusConnection QDBusConnection::connectToBus(BusType type, const QString &name)
         QDBusConnectionPrivate *d = 0;
         return QDBusConnection(d);
     }
-
-    QMutexLocker locker(&_q_manager()->mutex);
-
-    QDBusConnectionPrivate *d = _q_manager()->connection(name);
-    if (d || name.isEmpty())
-        return QDBusConnection(d);
-
-    d = new QDBusConnectionPrivate;
-    DBusConnection *c = 0;
-    QDBusErrorInternal error;
-    switch (type) {
-        case SystemBus:
-            c = q_dbus_bus_get_private(DBUS_BUS_SYSTEM, error);
-            break;
-        case SessionBus:
-            c = q_dbus_bus_get_private(DBUS_BUS_SESSION, error);
-            break;
-        case ActivationBus:
-            c = q_dbus_bus_get_private(DBUS_BUS_STARTER, error);
-            break;
-    }
-    d->setConnection(c, error); //setConnection does the error handling for us
-
-    _q_manager()->setConnection(name, d);
-
-    QDBusConnection retval(d);
-
-    // create the bus service
-    // will lock in QDBusConnectionPrivate::connectRelay()
-    d->setBusService(retval);
-    d->moveToThread(_q_manager());
-
-    return retval;
+    return QDBusConnection(_q_manager()->connectToBus(type, name));
 }
 
 /*!
@@ -364,34 +450,7 @@ QDBusConnection QDBusConnection::connectToBus(const QString &address,
         QDBusConnectionPrivate *d = 0;
         return QDBusConnection(d);
     }
-
-    QMutexLocker locker(&_q_manager()->mutex);
-
-    QDBusConnectionPrivate *d = _q_manager()->connection(name);
-    if (d || name.isEmpty())
-        return QDBusConnection(d);
-
-    d = new QDBusConnectionPrivate;
-    // setConnection does the error handling for us
-    QDBusErrorInternal error;
-    DBusConnection *c = q_dbus_connection_open_private(address.toUtf8().constData(), error);
-    if (c) {
-        if (!q_dbus_bus_register(c, error)) {
-            q_dbus_connection_unref(c);
-            c = 0;
-        }
-    }
-    d->setConnection(c, error);
-    _q_manager()->setConnection(name, d);
-
-    QDBusConnection retval(d);
-
-    // create the bus service
-    // will lock in QDBusConnectionPrivate::connectRelay()
-    d->setBusService(retval);
-    d->moveToThread(_q_manager());
-
-    return retval;
+    return QDBusConnection(_q_manager()->connectToBus(address, name));
 }
 /*!
     \since 4.8
@@ -406,25 +465,7 @@ QDBusConnection QDBusConnection::connectToPeer(const QString &address,
         QDBusConnectionPrivate *d = 0;
         return QDBusConnection(d);
     }
-
-    QMutexLocker locker(&_q_manager()->mutex);
-
-    QDBusConnectionPrivate *d = _q_manager()->connection(name);
-    if (d || name.isEmpty())
-        return QDBusConnection(d);
-
-    d = new QDBusConnectionPrivate;
-    // setPeer does the error handling for us
-    QDBusErrorInternal error;
-    DBusConnection *c = q_dbus_connection_open_private(address.toUtf8().constData(), error);
-
-    d->setPeer(c, error);
-    _q_manager()->setConnection(name, d);
-    d->moveToThread(_q_manager());
-
-    QDBusConnection retval(d);
-
-    return retval;
+    return QDBusConnection(_q_manager()->connectToPeer(address, name));
 }
 
 /*!
@@ -1130,9 +1171,10 @@ QDBusConnection QDBusConnection::sender()
 /*!
   \internal
 */
-void QDBusConnectionPrivate::setBusService(const QDBusConnection &connection)
+void QDBusConnectionPrivate::createBusService()
 {
     Q_ASSERT(mode == ClientMode);
+    QDBusConnection connection(this);
     busService = new QDBusConnectionInterface(connection, this);
     ref.deref(); // busService has increased the refcounting to us
                  // avoid cyclic refcounting
diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h
index f653c427e7..765e4281fd 100644
--- a/src/dbus/qdbusconnection_p.h
+++ b/src/dbus/qdbusconnection_p.h
@@ -190,7 +190,7 @@ public:
     explicit QDBusConnectionPrivate(QObject *parent = 0);
     ~QDBusConnectionPrivate();
 
-    void setBusService(const QDBusConnection &connection);
+    void createBusService();
     void setPeer(DBusConnection *connection, const QDBusErrorInternal &error);
     void setConnection(DBusConnection *connection, const QDBusErrorInternal &error);
     void setServer(QDBusServer *object, DBusServer *server, const QDBusErrorInternal &error);
diff --git a/src/dbus/qdbusconnectionmanager_p.h b/src/dbus/qdbusconnectionmanager_p.h
index c3c7999699..4527c562f8 100644
--- a/src/dbus/qdbusconnectionmanager_p.h
+++ b/src/dbus/qdbusconnectionmanager_p.h
@@ -57,6 +57,7 @@ QT_BEGIN_NAMESPACE
 class QDBusConnectionManager : public QDaemonThread
 {
     Q_OBJECT
+    struct ConnectionRequestData;
 public:
     QDBusConnectionManager();
     ~QDBusConnectionManager();
@@ -65,13 +66,23 @@ public:
     QDBusConnectionPrivate *connection(const QString &name) const;
     void removeConnection(const QString &name);
     void setConnection(const QString &name, QDBusConnectionPrivate *c);
+    QDBusConnectionPrivate *connectToBus(QDBusConnection::BusType type, const QString &name);
+    QDBusConnectionPrivate *connectToBus(const QString &address, const QString &name);
+    QDBusConnectionPrivate *connectToPeer(const QString &address, const QString &name);
 
     mutable QMutex mutex;
 
+signals:
+    void connectionRequested(ConnectionRequestData *);
+    void serverRequested(const QString &address, void *server);
+
 protected:
     void run() Q_DECL_OVERRIDE;
 
 private:
+    void executeConnectionRequest(ConnectionRequestData *data);
+    void createServer(const QString &address, void *server);
+
     QHash<QString, QDBusConnectionPrivate *> connectionHash;
 
     mutable QMutex senderMutex;
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index d5eaf39021..9b0a51107a 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1644,6 +1644,7 @@ void QDBusConnectionPrivate::setServer(QDBusServer *object, DBusServer *s, const
 {
     mode = ServerMode;
     serverObject = object;
+    object->d = this;
     if (!s) {
         handleError(error);
         return;
diff --git a/src/dbus/qdbusserver.cpp b/src/dbus/qdbusserver.cpp
index a0b2c8283c..6c1b4c10ef 100644
--- a/src/dbus/qdbusserver.cpp
+++ b/src/dbus/qdbusserver.cpp
@@ -63,14 +63,10 @@ QDBusServer::QDBusServer(const QString &address, QObject *parent)
         d = 0;
         return;
     }
-    d = new QDBusConnectionPrivate;
 
+    emit QDBusConnectionManager::instance()->serverRequested(address, this);
     QObject::connect(d, SIGNAL(newServerConnection(QDBusConnectionPrivate*)),
                      this, SLOT(_q_newConnection(QDBusConnectionPrivate*)), Qt::QueuedConnection);
-
-    QDBusErrorInternal error;
-    d->setServer(this, q_dbus_server_listen(address.toUtf8().constData(), error), error);
-    d->moveToThread(QDBusConnectionManager::instance());
 }
 
 /*!
@@ -83,23 +79,19 @@ QDBusServer::QDBusServer(QObject *parent)
 {
 #ifdef Q_OS_UNIX
     // Use Unix sockets on Unix systems only
-    static const char address[] = "unix:tmpdir=/tmp";
+    const QString address = QStringLiteral("unix:tmpdir=/tmp");
 #else
-    static const char address[] = "tcp:";
+    const QString address = QStringLiteral("tcp:");
 #endif
 
     if (!qdbus_loadLibDBus()) {
         d = 0;
         return;
     }
-    d = new QDBusConnectionPrivate;
 
+    emit QDBusConnectionManager::instance()->serverRequested(address, this);
     QObject::connect(d, SIGNAL(newServerConnection(QDBusConnectionPrivate*)),
                      this, SLOT(_q_newConnection(QDBusConnectionPrivate*)), Qt::QueuedConnection);
-
-    QDBusErrorInternal error;
-    d->setServer(this, q_dbus_server_listen(address, error), error);
-    d->moveToThread(QDBusConnectionManager::instance());
 }
 
 /*!
diff --git a/src/dbus/qdbusserver.h b/src/dbus/qdbusserver.h
index 2c66472a4d..3cb2ec8c50 100644
--- a/src/dbus/qdbusserver.h
+++ b/src/dbus/qdbusserver.h
@@ -69,6 +69,7 @@ private:
     Q_DISABLE_COPY(QDBusServer)
     Q_PRIVATE_SLOT(d, void _q_newConnection(QDBusConnectionPrivate*))
     QDBusConnectionPrivate *d;
+    friend class QDBusConnectionPrivate;
 };
 
 QT_END_NAMESPACE