1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
|
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
|
