126 lines
5.7 KiB
Diff
126 lines
5.7 KiB
Diff
From 11c5e716b08b6b3c5a7c9fce96b0cde8624ec869 Mon Sep 17 00:00:00 2001
|
|
From: Thiago Macieira <thiago.macieira@intel.com>
|
|
Date: Tue, 15 Mar 2016 11:00:20 -0700
|
|
Subject: [PATCH] Fix QtDBus deadlock inside kded/kiod
|
|
|
|
Whenever a message spy was installed, we failed to actually process
|
|
looped-back messages by queueing them for processing by the spy. That
|
|
had as a consequence that the caller got an error reply. Worse, since
|
|
the message had been queued, QtDBus would attempt to deliver it later.
|
|
Since that message had isLocal==true, bad things happened inside the
|
|
manager thread.
|
|
|
|
The correct solution is not to queue the message for the filter. If the
|
|
message is local, then simply deliver directly, as we're still in the
|
|
user's thread. This used to be the behavior in Qt 5.5.
|
|
|
|
Task-number: QTBUG-51676
|
|
Change-Id: I1dc112894cde7121e8ce302ae51b438ade1ff612
|
|
---
|
|
src/dbus/qdbusintegrator.cpp | 42 ++++++++++++++++++++++++++++++++----------
|
|
src/dbus/qdbusintegrator_p.h | 1 +
|
|
2 files changed, 33 insertions(+), 10 deletions(-)
|
|
|
|
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
|
|
index cd44861..478a2c4 100644
|
|
--- a/src/dbus/qdbusintegrator.cpp
|
|
+++ b/src/dbus/qdbusintegrator.cpp
|
|
@@ -481,6 +481,11 @@ QDBusSpyCallEvent::~QDBusSpyCallEvent()
|
|
|
|
void QDBusSpyCallEvent::placeMetaCall(QObject *)
|
|
{
|
|
+ invokeSpyHooks(msg, hooks, hookCount);
|
|
+}
|
|
+
|
|
+inline void QDBusSpyCallEvent::invokeSpyHooks(const QDBusMessage &msg, const Hook *hooks, int hookCount)
|
|
+{
|
|
// call the spy hook list
|
|
for (int i = 0; i < hookCount; ++i)
|
|
hooks[i](msg);
|
|
@@ -509,7 +514,12 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
|
|
{
|
|
if (!ref.load())
|
|
return false;
|
|
- if (!dispatchEnabled && !QDBusMessagePrivate::isLocal(amsg)) {
|
|
+
|
|
+ // local message are always delivered, regardless of filtering
|
|
+ // or whether the dispatcher is enabled
|
|
+ bool isLocal = QDBusMessagePrivate::isLocal(amsg);
|
|
+
|
|
+ if (!dispatchEnabled && !isLocal) {
|
|
// queue messages only, we'll handle them later
|
|
qDBusDebug() << this << "delivery is suspended";
|
|
pendingMessages << amsg;
|
|
@@ -523,13 +533,23 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
|
|
// let them see the signal too
|
|
return false;
|
|
case QDBusMessage::MethodCallMessage:
|
|
- // run it through the spy filters (if any) before the regular processing
|
|
+ // run it through the spy filters (if any) before the regular processing:
|
|
+ // a) if it's a local message, we're in the caller's thread, so invoke the filter directly
|
|
+ // b) if it's an external message, post to the main thread
|
|
if (Q_UNLIKELY(qDBusSpyHookList.exists()) && qApp) {
|
|
const QDBusSpyHookList &list = *qDBusSpyHookList;
|
|
- qDBusDebug() << this << "invoking message spies";
|
|
- QCoreApplication::postEvent(qApp, new QDBusSpyCallEvent(this, QDBusConnection(this),
|
|
- amsg, list.constData(), list.size()));
|
|
- return true;
|
|
+ if (isLocal) {
|
|
+ Q_ASSERT(QThread::currentThread() != thread());
|
|
+ qDBusDebug() << this << "invoking message spies directly";
|
|
+ QDBusSpyCallEvent::invokeSpyHooks(amsg, list.constData(), list.size());
|
|
+ } else {
|
|
+ qDBusDebug() << this << "invoking message spies via event";
|
|
+ QCoreApplication::postEvent(qApp, new QDBusSpyCallEvent(this, QDBusConnection(this),
|
|
+ amsg, list.constData(), list.size()));
|
|
+
|
|
+ // we'll be called back, so return
|
|
+ return true;
|
|
+ }
|
|
}
|
|
|
|
handleObjectCall(amsg);
|
|
@@ -1451,9 +1471,9 @@ void QDBusConnectionPrivate::handleObjectCall(const QDBusMessage &msg)
|
|
// that means the dispatchLock mutex is locked
|
|
// must not call out to user code in that case
|
|
//
|
|
- // however, if the message is internal, handleMessage was called
|
|
- // directly and no lock is in place. We can therefore call out to
|
|
- // user code, if necessary
|
|
+ // however, if the message is internal, handleMessage was called directly
|
|
+ // (user's thread) and no lock is in place. We can therefore call out to
|
|
+ // user code, if necessary.
|
|
ObjectTreeNode result;
|
|
int usedLength;
|
|
QThread *objThread = 0;
|
|
@@ -1492,12 +1512,14 @@ void QDBusConnectionPrivate::handleObjectCall(const QDBusMessage &msg)
|
|
usedLength, msg));
|
|
return;
|
|
} else if (objThread != QThread::currentThread()) {
|
|
- // synchronize with other thread
|
|
+ // looped-back message, targeting another thread:
|
|
+ // synchronize with it
|
|
postEventToThread(HandleObjectCallPostEventAction, result.obj,
|
|
new QDBusActivateObjectEvent(QDBusConnection(this), this, result,
|
|
usedLength, msg, &sem));
|
|
semWait = true;
|
|
} else {
|
|
+ // looped-back message, targeting current thread
|
|
semWait = false;
|
|
}
|
|
} // release the lock
|
|
diff --git a/src/dbus/qdbusintegrator_p.h b/src/dbus/qdbusintegrator_p.h
|
|
index 2bbebdf..c0d9c22 100644
|
|
--- a/src/dbus/qdbusintegrator_p.h
|
|
+++ b/src/dbus/qdbusintegrator_p.h
|
|
@@ -145,6 +145,7 @@ public:
|
|
{}
|
|
~QDBusSpyCallEvent();
|
|
void placeMetaCall(QObject *) Q_DECL_OVERRIDE;
|
|
+ static inline void invokeSpyHooks(const QDBusMessage &msg, const Hook *hooks, int hookCount);
|
|
|
|
QDBusConnection conn; // keeps the refcount in QDBusConnectionPrivate up
|
|
QDBusMessage msg;
|
|
--
|
|
2.7.1
|