From 83da3e5edaf568b4a8c6aec8e0750fa45c726a27 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 28 Dec 2015 13:23:28 -0200 Subject: Change a QList of pointers to QVector QList of pointers is optimum, but QVector should provide the same performance (we aren't using the beginning-of-list feature that QList has and QVector doesn't). But since we're using QVector elsewhere, this should be better. Change-Id: I39cc61d0d59846ab8c23ffff14241c6715e2eb00 Reviewed-by: Marc Mutz --- src/dbus/qdbusconnection_p.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/dbus') diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 91824c5c79..0371f5ece0 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -174,7 +174,7 @@ public: typedef QMultiHash SignalHookHash; typedef QHash MetaObjectHash; typedef QHash MatchRefCountHash; - typedef QList PendingCallList; + typedef QVector PendingCallList; struct WatchedServiceData { WatchedServiceData() : refcount(0) {} -- cgit v1.2.3 From fd3ea7004dd9c1c5bd6e18a39fb02b230ef603a4 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 28 Dec 2015 13:14:16 -0200 Subject: Remove unused member variable QDBusConnectionPrivate::timeoutsPendingAdd They're never pending, since we add them immediately since commit 186d8814407ccb3e221537d9797172c37127bc51. Change-Id: I39cc61d0d59846ab8c23ffff14241be6785ad5a0 Reviewed-by: Lorn Potter --- src/dbus/qdbusconnection_p.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/dbus') diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 0371f5ece0..2df7a49966 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -169,7 +169,6 @@ public: // typedefs typedef QMultiHash WatcherHash; typedef QHash TimeoutHash; - typedef QVector > PendingTimeoutList; typedef QMultiHash SignalHookHash; typedef QHash MetaObjectHash; @@ -309,7 +308,6 @@ public: }; WatcherHash watchers; TimeoutHash timeouts; - PendingTimeoutList timeoutsPendingAdd; // the master lock protects our own internal state QReadWriteLock lock; -- cgit v1.2.3 From 8d195c0d57ea241183d23c45f855be1b126d8ee7 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 28 Dec 2015 13:38:08 -0200 Subject: Add a default argument to QDBusPendingCallWatcher::finished signal So we can do connect(&watcher, SIGNAL(finished()), receiver, SLOT(foo())); Change-Id: I39cc61d0d59846ab8c23ffff14241d33fecf2d53 Reviewed-by: Lorn Potter --- src/dbus/qdbuspendingcall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/dbus') diff --git a/src/dbus/qdbuspendingcall.h b/src/dbus/qdbuspendingcall.h index 3bcacffd22..d7d68825be 100644 --- a/src/dbus/qdbuspendingcall.h +++ b/src/dbus/qdbuspendingcall.h @@ -106,7 +106,7 @@ public: void waitForFinished(); // non-virtual override Q_SIGNALS: - void finished(QDBusPendingCallWatcher *self); + void finished(QDBusPendingCallWatcher *self = Q_NULLPTR); private: Q_DECLARE_PRIVATE(QDBusPendingCallWatcher) -- cgit v1.2.3 From 1f6fa1f37a14742ddf53c753ce52d9dc048cd1dc Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sun, 27 Dec 2015 11:15:24 -0200 Subject: 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 --- src/dbus/qdbusconnection.cpp | 45 ++++++++++++++++++++++++++++++++++--- src/dbus/qdbusconnection_p.h | 4 ++++ src/dbus/qdbusconnectionmanager_p.h | 2 +- src/dbus/qdbusintegrator.cpp | 33 +++++++++++++++++++++++++-- src/dbus/qdbusthreaddebug_p.h | 2 +- 5 files changed, 79 insertions(+), 7 deletions(-) (limited to 'src/dbus') 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 #include #include +#include #include #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 diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 2df7a49966..f030a3ff8c 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -169,6 +169,7 @@ public: // typedefs typedef QMultiHash WatcherHash; typedef QHash TimeoutHash; + typedef QVector PendingMessageList; typedef QMultiHash SignalHookHash; typedef QHash MetaObjectHash; @@ -191,6 +192,7 @@ public: ~QDBusConnectionPrivate(); void createBusService(); + void setDispatchEnabled(bool enable); void setPeer(DBusConnection *connection, const QDBusErrorInternal &error); void setConnection(DBusConnection *connection, const QDBusErrorInternal &error); void setServer(QDBusServer *object, DBusServer *server, const QDBusErrorInternal &error); @@ -308,6 +310,7 @@ public: }; WatcherHash watchers; TimeoutHash timeouts; + PendingMessageList pendingMessages; // the master lock protects our own internal state QReadWriteLock lock; @@ -322,6 +325,7 @@ public: PendingCallList pendingCalls; bool anonymousAuthenticationAllowed; + bool dispatchEnabled; // protected by the dispatch lock, not the main lock public: // static methods diff --git a/src/dbus/qdbusconnectionmanager_p.h b/src/dbus/qdbusconnectionmanager_p.h index 3f815fdcd7..c0ab48e4ee 100644 --- a/src/dbus/qdbusconnectionmanager_p.h +++ b/src/dbus/qdbusconnectionmanager_p.h @@ -67,7 +67,7 @@ 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(QDBusConnection::BusType type, const QString &name, bool suspendedDelivery); QDBusConnectionPrivate *connectToBus(const QString &address, const QString &name); QDBusConnectionPrivate *connectToPeer(const QString &address, const QString &name); diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index c465706913..f6221d51b6 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -496,6 +496,11 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg) if (!ref.load()) return false; + if (!dispatchEnabled && !QDBusMessagePrivate::isLocal(amsg)) { + // queue messages only, we'll handle them later + pendingMessages << amsg; + return amsg.type() == QDBusMessage::MethodCallMessage; + } switch (amsg.type()) { case QDBusMessage::SignalMessage: @@ -690,6 +695,20 @@ static int findSlot(const QMetaObject *mo, const QByteArray &name, int flags, return -1; } +/*! + \internal + Enables or disables the delivery of incoming method calls and signals. If + \a enable is true, this will also cause any queued, pending messages to be + delivered. + */ +void QDBusConnectionPrivate::setDispatchEnabled(bool enable) +{ + QDBusDispatchLocker locker(SetDispatchEnabledAction, this); + dispatchEnabled = enable; + if (enable) + emit dispatchStatusChanged(); +} + static QDBusCallDeliveryEvent * const DIRECT_DELIVERY = (QDBusCallDeliveryEvent *)1; QDBusCallDeliveryEvent* QDBusConnectionPrivate::prepareReply(QDBusConnectionPrivate *target, @@ -946,7 +965,8 @@ QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p) : QObject(p), ref(1), capabilities(0), mode(InvalidMode), busService(0), dispatchLock(QMutex::Recursive), connection(0), rootNode(QString(QLatin1Char('/'))), - anonymousAuthenticationAllowed(false) + anonymousAuthenticationAllowed(false), + dispatchEnabled(true) { static const bool threads = q_dbus_threads_init_default(); if (::isDebugging == -1) @@ -1066,8 +1086,17 @@ void QDBusConnectionPrivate::timerEvent(QTimerEvent *e) void QDBusConnectionPrivate::doDispatch() { QDBusDispatchLocker locker(DoDispatchAction, this); - if (mode == ClientMode || mode == PeerMode) + 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(); + PendingMessageList::Iterator end = pendingMessages.end(); + for ( ; it != end; ++it) + handleMessage(qMove(*it)); + pendingMessages.clear(); + } + } } void QDBusConnectionPrivate::socketRead(int fd) diff --git a/src/dbus/qdbusthreaddebug_p.h b/src/dbus/qdbusthreaddebug_p.h index eace25478d..420f062615 100644 --- a/src/dbus/qdbusthreaddebug_p.h +++ b/src/dbus/qdbusthreaddebug_p.h @@ -83,7 +83,7 @@ enum ThreadAction { HandleObjectCallPostEventAction = 22, HandleObjectCallSemaphoreAction = 23, DoDispatchAction = 24, - // unused: 25, + SetDispatchEnabledAction = 25, MessageResultReceivedAction = 26, ActivateSignalAction = 27, PendingCallBlockAction = 28, -- cgit v1.2.3 From 618e2cc081e09d9222418bd933876224675a7530 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Wed, 18 Feb 2015 02:13:49 +0100 Subject: dbus: Add method serial() and replySerial() to class DBusMessage. This patch includes setup of class member 'msg' in QDBusMessagePrivate::toDBusMessage() to be able to get the serial after message sending. Testcases for comparing the 'reply serial to' with the 'serial' are included. Task-number: QTBUG-44490 Change-Id: Iae7c48f5b0c70a6c5ae500904072b38b46dfd876 Reviewed-by: Thiago Macieira --- src/dbus/qdbus_symbols_p.h | 2 ++ src/dbus/qdbusmessage.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ src/dbus/qdbusmessage.h | 2 ++ src/dbus/qdbusmessage_p.h | 2 ++ 4 files changed, 47 insertions(+) (limited to 'src/dbus') diff --git a/src/dbus/qdbus_symbols_p.h b/src/dbus/qdbus_symbols_p.h index 1991a462e9..67680f6b82 100644 --- a/src/dbus/qdbus_symbols_p.h +++ b/src/dbus/qdbus_symbols_p.h @@ -286,6 +286,8 @@ DEFINEFUNC(const char* , dbus_message_get_sender, (DBusMessage *message), (message), return) DEFINEFUNC(dbus_uint32_t , dbus_message_get_serial, (DBusMessage *message), (message), return) +DEFINEFUNC(dbus_uint32_t , dbus_message_get_reply_serial, (DBusMessage *message), + (message), return) DEFINEFUNC(const char* , dbus_message_get_signature, (DBusMessage *message), (message), return) DEFINEFUNC(int , dbus_message_get_type, (DBusMessage *message), diff --git a/src/dbus/qdbusmessage.cpp b/src/dbus/qdbusmessage.cpp index 32b7787514..e20c851d6c 100644 --- a/src/dbus/qdbusmessage.cpp +++ b/src/dbus/qdbusmessage.cpp @@ -188,7 +188,12 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB // check if everything is ok if (marshaller.ok) + { + QDBusMessage *m = (QDBusMessage*)&message; + q_dbus_message_ref(msg); + m->d_ptr->msg = msg; return msg; + } // not ok; q_dbus_message_unref(msg); @@ -317,6 +322,16 @@ QDBusMessage QDBusMessagePrivate::makeLocalReply(const QDBusConnectionPrivate &c return QDBusMessage(); // failed } +uint QDBusMessagePrivate::serial() +{ + return msg ? q_dbus_message_get_serial(msg) : reply ? q_dbus_message_get_serial(reply) : 0; +} + +uint QDBusMessagePrivate::replySerial() +{ + return msg ? q_dbus_message_get_reply_serial(msg) : reply ? q_dbus_message_get_reply_serial(reply) : 0; +} + /*! \class QDBusMessage \inmodule QtDBus @@ -632,6 +647,32 @@ QString QDBusMessage::signature() const return d_ptr->signature; } +/*! + Returns the serial of the message or 0 if undefined. + + The serial number is a unique identifier of a message coming from a + given connection. + + The serial is set to a non zero value after the message has been sent + over a D-Bus connection. +*/ +uint QDBusMessage::serial() const +{ + return d_ptr->serial(); +} + +/*! + Returns the serial of the message this is a reply to or 0 if undefined. + + The serial number is a unique identifier of a message coming from a + given connection and D-Bus messages of 'method return' or 'error' type + use them to match the reply to the method call message. +*/ +uint QDBusMessage::replySerial() const +{ + return d_ptr->replySerial(); +} + /*! Returns the flag that indicates if this message should see a reply or not. This is only meaningful for \l {MethodCallMessage}{method diff --git a/src/dbus/qdbusmessage.h b/src/dbus/qdbusmessage.h index e85d600080..f6538bd2cf 100644 --- a/src/dbus/qdbusmessage.h +++ b/src/dbus/qdbusmessage.h @@ -104,6 +104,8 @@ public: QString errorMessage() const; MessageType type() const; QString signature() const; + uint serial() const; + uint replySerial() const; bool isReplyRequired() const; diff --git a/src/dbus/qdbusmessage_p.h b/src/dbus/qdbusmessage_p.h index 5abd490502..0ad9924cac 100644 --- a/src/dbus/qdbusmessage_p.h +++ b/src/dbus/qdbusmessage_p.h @@ -93,6 +93,8 @@ public: const QDBusMessage &asSent); static QDBusMessage makeLocalReply(const QDBusConnectionPrivate &conn, const QDBusMessage &asSent); + uint serial(); + uint replySerial(); }; QT_END_NAMESPACE -- cgit v1.2.3 From d3fe4f066f70bc8e4aef06b963444ecdbc3dd00f Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Wed, 18 Feb 2015 02:14:39 +0100 Subject: dbus: Print out 'serial' and 'serial reply to' with DBusMessage operator<<. The reply serial is displayed for method call returns and errors, while the serial is displayed for all message types. To see a message serial it is required to dump messages after sending, not before. Task-number: QTBUG-44490 Change-Id: I859f50d739ed059d5b2dfe1a2efdf04b906891a7 Reviewed-by: Thiago Macieira --- src/dbus/qdbusintegrator.cpp | 4 ++-- src/dbus/qdbusmessage.cpp | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) (limited to 'src/dbus') diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index f6221d51b6..0c0109b6b6 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -1819,8 +1819,8 @@ bool QDBusConnectionPrivate::send(const QDBusMessage& message) } q_dbus_message_set_no_reply(msg, true); // the reply would not be delivered to anything - qDBusDebug() << this << "sending message (no reply):" << message; emit messageNeedsSending(Q_NULLPTR, msg); + qDBusDebug() << this << "sending message (no reply):" << message; return true; } @@ -2019,8 +2019,8 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM lastError = error; processFinishedCall(pcall); } else { - qDBusDebug() << this << "sending message:" << message; emit messageNeedsSending(pcall, msg, timeout); + qDBusDebug() << this << "sending message:" << message; } return pcall; } diff --git a/src/dbus/qdbusmessage.cpp b/src/dbus/qdbusmessage.cpp index e20c851d6c..078442f3f1 100644 --- a/src/dbus/qdbusmessage.cpp +++ b/src/dbus/qdbusmessage.cpp @@ -861,10 +861,16 @@ QDebug operator<<(QDebug dbg, const QDBusMessage &msg) msg.type() == QDBusMessage::SignalMessage) dbg.nospace() << ", path=" << msg.path() << ", interface=" << msg.interface() - << ", member=" << msg.member(); + << ", member=" << msg.member() + << ", serial=" << msg.serial(); if (msg.type() == QDBusMessage::ErrorMessage) dbg.nospace() << ", error name=" << msg.errorName() - << ", error message=" << msg.errorMessage(); + << ", error message=" << msg.errorMessage() + << ", serial=" << msg.serial() + << ", reply serial=" << msg.replySerial(); + else if (msg.type() == QDBusMessage::ReplyMessage) + dbg.nospace() << ", serial=" << msg.serial() + << ", reply serial=" << msg.replySerial(); dbg.nospace() << ", signature=" << msg.signature() << ", contents=("; debugVariantList(dbg, msg.arguments()); -- cgit v1.2.3