On Monday 18 January 2016 09:23:51 David Faure wrote:
> Thiago: indeed the BlockingQueuedConnection call from messageFilter to the
> main thread occasionnally leads to deadlocks, we need your
> "postpone incoming messages" idea.
Please try the attached patch, also found at
https://codereview.qt-project.org/146639
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
>From fafa4b3b89a02e136f8f4af8455ee2fd8a7cf9f4 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macie...@intel.com>
Date: Tue, 19 Jan 2016 22:29:33 -0800
Subject: [PATCH 1/1] Call out to QtDBus message spies in the main thread
Whenever there are spies installed, we call out to the main thread to
call to the kded/kiod message spies. This allows the spy code to do just
about anything, where previously it was restricted in what it could do
to avoid deadlocking or triggering assertions if it recursed back into
QDBusConnection code in the manager thread.
After the spies are done with, the message is re-inserted into the
QDBusConnection processing pipeline. That means it may be delayed again
if the dispatching is disabled.
Change-Id: I3d11545be52c43119f0fffff142b0e9d447415c2
---
src/dbus/qdbusconnection_p.h | 4 +++-
src/dbus/qdbusintegrator_p.h | 19 +++++++++++++++++
src/dbus/qdbusintegrator.cpp | 51 ++++++++++++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 12 deletions(-)
diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h
index f030a3f..d7787fa 100644
--- a/src/dbus/qdbusconnection_p.h
+++ b/src/dbus/qdbusconnection_p.h
@@ -1,7 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
-** Copyright (C) 2015 Intel Corporation.
+** Copyright (C) 2016 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@@ -228,6 +228,7 @@ public:
void registerService(const QString &serviceName);
void unregisterService(const QString &serviceName);
+ bool preHandleMessage(const QDBusMessage &msg);
bool handleMessage(const QDBusMessage &msg);
QDBusMetaObject *findMetaObject(const QString &service, const QString &path,
@@ -281,6 +282,7 @@ private slots:
signals:
void dispatchStatusChanged();
+ void messageNeedsHandling(const QDBusMessage &msg);
void messageNeedsSending(QDBusPendingCallPrivate *pcall, void *msg, int timeout = -1);
void signalNeedsConnecting(const QString &key, const QDBusConnectionPrivate::SignalHook &hook);
bool signalNeedsDisconnecting(const QString &key, const QDBusConnectionPrivate::SignalHook &hook);
diff --git a/src/dbus/qdbusintegrator_p.h b/src/dbus/qdbusintegrator_p.h
index 2038ac9..e266512 100644
--- a/src/dbus/qdbusintegrator_p.h
+++ b/src/dbus/qdbusintegrator_p.h
@@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
+** Copyright (C) 2016 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@@ -65,6 +66,7 @@
QT_BEGIN_NAMESPACE
class QDBusConnectionPrivate;
+class QDBusMessage;
// Really private structs used by qdbusintegrator.cpp
// Things that aren't used by any other file
@@ -133,6 +135,23 @@ private:
bool handled;
};
+class QDBusSpyCallEvent : public QMetaCallEvent
+{
+public:
+ typedef void (*Hook)(const QDBusMessage&);
+ QDBusSpyCallEvent(QDBusConnectionPrivate *cp, const QDBusConnection &c, const QDBusMessage &msg,
+ const Hook *hooks, int count)
+ : QMetaCallEvent(0, 0, Q_NULLPTR, cp, 0), conn(c), msg(msg), hooks(hooks), hookCount(count)
+ {}
+ ~QDBusSpyCallEvent();
+ void placeMetaCall(QObject *) Q_DECL_OVERRIDE;
+
+ QDBusConnection conn; // keeps the refcount in QDBusConnectionPrivate up
+ QDBusMessage msg;
+ const Hook *hooks;
+ int hookCount;
+};
+
QT_END_NAMESPACE
Q_DECLARE_METATYPE(QDBusSlotCache)
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index dffdfe2..652924f 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1,7 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
-** Copyright (C) 2015 Intel Corporation.
+** Copyright (C) 2016 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@@ -120,8 +120,7 @@ void qdbusDefaultThreadDebug(int action, int condition, QDBusConnectionPrivate *
qdbusThreadDebugFunc qdbusThreadDebug = 0;
#endif
-typedef void (*QDBusSpyHook)(const QDBusMessage&);
-typedef QVarLengthArray<QDBusSpyHook, 4> QDBusSpyHookList;
+typedef QVarLengthArray<QDBusSpyCallEvent::Hook, 4> QDBusSpyHookList;
Q_GLOBAL_STATIC(QDBusSpyHookList, qDBusSpyHookList)
extern "C" {
@@ -461,12 +460,29 @@ static QStringList matchArgsForService(const QString &service, QDBusServiceWatch
}
-extern Q_DBUS_EXPORT void qDBusAddSpyHook(QDBusSpyHook);
-void qDBusAddSpyHook(QDBusSpyHook hook)
+extern Q_DBUS_EXPORT void qDBusAddSpyHook(QDBusSpyCallEvent::Hook);
+void qDBusAddSpyHook(QDBusSpyCallEvent::Hook hook)
{
qDBusSpyHookList()->append(hook);
}
+QDBusSpyCallEvent::~QDBusSpyCallEvent()
+{
+ // Reinsert the message into the processing queue for the connection.
+ // This is done in the destructor so the message is reinserted even if
+ // QCoreApplication is destroyed.
+ QDBusConnectionPrivate *d = static_cast<QDBusConnectionPrivate *>(const_cast<QObject *>(sender()));
+ qDBusDebug() << d << "message spies done for" << msg;
+ emit d->messageNeedsHandling(msg);
+}
+
+void QDBusSpyCallEvent::placeMetaCall(QObject *)
+{
+ // call the spy hook list
+ for (int i = 0; i < hookCount; ++i)
+ hooks[i](msg);
+}
+
extern "C" {
static DBusHandlerResult
qDBusSignalFilter(DBusConnection *connection, DBusMessage *message, void *data)
@@ -480,20 +496,31 @@ qDBusSignalFilter(DBusConnection *connection, DBusMessage *message, void *data)
QDBusMessage amsg = QDBusMessagePrivate::fromDBusMessage(message, d->capabilities);
qDBusDebug() << d << "got message (signal):" << amsg;
- return d->handleMessage(amsg) ?
+ return d->preHandleMessage(amsg) ?
DBUS_HANDLER_RESULT_HANDLED :
DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
}
-bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
+bool QDBusConnectionPrivate::preHandleMessage(const QDBusMessage &amsg)
{
- const QDBusSpyHookList *list = qDBusSpyHookList();
- for (int i = 0; list && i < list->size(); ++i) {
- qDBusDebug() << "calling the message spy hook";
- (*(*list)[i])(amsg);
+ // run it through the spy filters (if any) before the regular processing
+ if (Q_UNLIKELY(qDBusSpyHookList.exists()) && qApp) {
+ const QDBusSpyHookList &list = *qDBusSpyHookList;
+ if (!list.isEmpty()) {
+ qDBusDebug() << this << "invoking message spies";
+ QCoreApplication::postEvent(qApp, new QDBusSpyCallEvent(this, QDBusConnection(this),
+ amsg, list.constData(), list.size()));
+ return amsg.type() == QDBusMessage::MethodCallMessage;
+ }
}
+ // no spies, do the regular processing only
+ return handleMessage(amsg);
+}
+
+bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
+{
if (!ref.load())
return false;
if (!dispatchEnabled && !QDBusMessagePrivate::isLocal(amsg)) {
@@ -979,6 +1006,8 @@ QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p)
QDBusMetaTypeId::init();
connect(this, &QDBusConnectionPrivate::dispatchStatusChanged,
this, &QDBusConnectionPrivate::doDispatch, Qt::QueuedConnection);
+ connect(this, &QDBusConnectionPrivate::messageNeedsHandling,
+ this, &QDBusConnectionPrivate::handleMessage, Qt::QueuedConnection);
connect(this, &QDBusConnectionPrivate::messageNeedsSending,
this, &QDBusConnectionPrivate::sendInternal);
connect(this, &QDBusConnectionPrivate::signalNeedsConnecting,
--
2.7.0
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
