From 6a2bdc4ee2dc49b5d89d09a1f255a7a0e2f18acf Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 28 Oct 2014 17:23:09 -0700 Subject: Always lock the DBus dispatcher before dbus_connection_send* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We lock it before dbus_connection_send_with_reply (the async version) in QDBusConnectionPrivate::sendWithReplyAsync. We weren't locking it before send_with_reply_and_block and we apparently should. The locking around the dbus_connection_send function might not be necessary, but let's do it to be safe. The lock now needs to be recursive because we may be inside QDBusConnectionPrivate::doDispatch. Task-number: QTBUG-42189 Change-Id: I7b6b350909359817ea8b3f9c693bced042c9779a Reviewed-by: Jędrzej Nowacki Reviewed-by: Frederik Gladhorn --- src/dbus/qdbusintegrator.cpp | 19 +++++++++++++++---- src/dbus/qdbusthreaddebug_p.h | 3 +++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index 499e9dbd82..b2f6635d1b 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -1017,7 +1017,7 @@ extern bool qDBusInitThreads(); QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p) : QObject(p), ref(1), capabilities(0), mode(InvalidMode), connection(0), server(0), busService(0), - watchAndTimeoutLock(QMutex::Recursive), + watchAndTimeoutLock(QMutex::Recursive), dispatchLock(QMutex::Recursive), rootNode(QString(QLatin1Char('/'))), anonymousAuthenticationAllowed(false) { @@ -1266,7 +1266,10 @@ void QDBusConnectionPrivate::relaySignal(QObject *obj, const QMetaObject *mo, in //qDBusDebug() << "Emitting signal" << message; //qDBusDebug() << "for paths:"; q_dbus_message_set_no_reply(msg, true); // the reply would not be delivered to anything - huntAndEmit(connection, msg, obj, rootNode, isScriptable, isAdaptor); + { + QDBusDispatchLocker locker(HuntAndEmitAction, this); + huntAndEmit(connection, msg, obj, rootNode, isScriptable, isAdaptor); + } q_dbus_message_unref(msg); } @@ -1923,7 +1926,11 @@ int QDBusConnectionPrivate::send(const QDBusMessage& message) qDBusDebug() << this << "sending message (no reply):" << message; checkThread(); - bool isOk = q_dbus_connection_send(connection, msg, 0); + bool isOk; + { + QDBusDispatchLocker locker(SendMessageAction, this); + isOk = q_dbus_connection_send(connection, msg, 0); + } int serial = 0; if (isOk) serial = q_dbus_message_get_serial(msg); @@ -1955,7 +1962,11 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message, qDBusDebug() << this << "sending message (blocking):" << message; QDBusErrorInternal error; - DBusMessage *reply = q_dbus_connection_send_with_reply_and_block(connection, msg, timeout, error); + DBusMessage *reply; + { + QDBusDispatchLocker locker(SendWithReplyAndBlockAction, this); + reply = q_dbus_connection_send_with_reply_and_block(connection, msg, timeout, error); + } q_dbus_message_unref(msg); diff --git a/src/dbus/qdbusthreaddebug_p.h b/src/dbus/qdbusthreaddebug_p.h index f9039ef3cd..dcde99169c 100644 --- a/src/dbus/qdbusthreaddebug_p.h +++ b/src/dbus/qdbusthreaddebug_p.h @@ -94,6 +94,9 @@ enum ThreadAction { MessageResultReceivedAction = 26, ActivateSignalAction = 27, PendingCallBlockAction = 28, + SendMessageAction = 29, + SendWithReplyAndBlockAction = 30, + HuntAndEmitAction = 31, AddTimeoutAction = 50, RealAddTimeoutAction = 51, -- cgit v1.2.3