rpms/qt5-qtbase
6
0
Fork 0
Code Issues Pull Requests Releases Activity

candidate fixes for various QtDBus deadlocks (QTBUG-51648,QTBUG-51649,QTBUG-51676)

Browse Source
This commit is contained in:
Rex Dieter 2016-03-10 10:44:24 -06:00
parent 5523e1423f
commit f06686fa09
4 changed files with 284 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
QTBUG-51648-QtDBus-clean-up-signal-hooks-and-object-tree-in-clos.patch Normal file
View File

@ -0,0 +1,88 @@
From b024fbe83863fc57364a52c717d5b43d654bdb5d Mon Sep 17 00:00:00 2001
From: Weng Xuetian <wengxt@gmail.com>
Date: Sat, 5 Mar 2016 12:23:21 -0800
Subject: [PATCH] QtDBus: clean up signal hooks and object tree in
closeConnection
If a QObject is added or passed as receiver to QDBusConnection::connect()
and it is managed by Q_GLOBAL_STATIC or similar mechanism, it is
possible that when that its destructor is called after the dbus daemon
thread ends. In that case, QObject::destroyed connected via
Qt::BlockingQueuedConnection to QDBusConnectionPrivate will cause dead
lock since the thread is no longer processing events.
Task-number: QTBUG-51648
Change-Id: I1a1810a6d6d0234af0269d5f3fc1f54101bf1547
---
src/dbus/qdbusconnection_p.h | 1 +
src/dbus/qdbusintegrator.cpp | 28 +++++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h
index c77daf7..565eb83 100644
--- a/src/dbus/qdbusconnection_p.h
+++ b/src/dbus/qdbusconnection_p.h
@@ -254,6 +254,7 @@ private:
const QVector<int> &metaTypes, int slotIdx);
SignalHookHash::Iterator removeSignalHookNoLock(SignalHookHash::Iterator it);
+ void disconnectObjectTree(ObjectTreeNode &node);
bool isServiceRegisteredByThread(const QString &serviceName);
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index cd44861..a3cd47b 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1030,7 +1030,6 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
qPrintable(name));
closeConnection();
- rootNode.children.clear(); // free resources
qDeleteAll(cachedMetaObjects);
if (mode == ClientMode || mode == PeerMode) {
@@ -1052,6 +1051,20 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
}
}
+void QDBusConnectionPrivate::disconnectObjectTree(QDBusConnectionPrivate::ObjectTreeNode &haystack)
+{
+ QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator it = haystack.children.begin();
+
+ while (it != haystack.children.end()) {
+ disconnectObjectTree(*it);
+ it++;
+ }
+
+ if (haystack.obj) {
+ haystack.obj->disconnect(this);
+ }
+}
+
void QDBusConnectionPrivate::closeConnection()
{
QDBusWriteLocker locker(CloseConnectionAction, this);
@@ -1075,6 +1088,19 @@ void QDBusConnectionPrivate::closeConnection()
}
qDeleteAll(pendingCalls);
+
+ // clean up all signal hook and object tree, to avoid QObject::destroyed
+ // being activated to dbus daemon thread which already quits.
+ // dbus connection is already closed, so there is nothing we could do be clean
+ // up everything here.
+ SignalHookHash::iterator sit = signalHooks.begin();
+ while (sit != signalHooks.end()) {
+ sit.value().obj->disconnect(this);
+ sit++;
+ }
+
+ disconnectObjectTree(rootNode);
+ rootNode.children.clear(); // free resources
}
void QDBusConnectionPrivate::checkThread()
--
2.5.0

142
QTBUG-51649-QtDBus-finish-all-pending-call-with-error-if-disconn.patch Normal file
View File

@ -0,0 +1,142 @@
From 136eeec876ed5b995e7c27bcdcefe0199f5f183d Mon Sep 17 00:00:00 2001
From: Weng Xuetian <wengxt@gmail.com>
Date: Thu, 3 Mar 2016 21:56:53 -0800
Subject: [PATCH] QtDBus: finish all pending call with error if disconnected
libdbus will send a local signal if connection gets disconnected. When
this happens, end all pending calls with QDBusError::Disconnected.
Task-number: QTBUG-51649
Change-Id: I5c7d2a468bb5da746d0c0e53e458c1e376f186a9
---
src/dbus/qdbusintegrator.cpp | 26 +++++++++++++++++-----
src/dbus/qdbusutil_p.h | 6 +++++
.../dbus/qdbusconnection/tst_qdbusconnection.cpp | 22 ++++++++++++++++++
.../dbus/qdbusconnection/tst_qdbusconnection.h | 1 +
4 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index cd44861..320419f 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -519,6 +519,14 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
switch (amsg.type()) {
case QDBusMessage::SignalMessage:
handleSignal(amsg);
+ // Check local disconnected signal from libdbus
+ if (amsg.interface() == QDBusUtil::dbusInterfaceLocal()
+ && amsg.path() == QDBusUtil::dbusPathLocal()
+ && amsg.member() == QDBusUtil::disconnected()
+ && !QDBusMessagePrivate::isLocal(amsg)) {
+ while (!pendingCalls.isEmpty())
+ processFinishedCall(pendingCalls.first());
+ }
// if there are any other filters in this DBusConnection,
// let them see the signal too
return false;
@@ -1767,10 +1775,16 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
QDBusMessage &msg = call->replyMessage;
if (call->pending) {
- // decode the message
- DBusMessage *reply = q_dbus_pending_call_steal_reply(call->pending);
- msg = QDBusMessagePrivate::fromDBusMessage(reply, connection->capabilities);
- q_dbus_message_unref(reply);
+ // when processFinishedCall is called and pending call is not completed,
+ // it means we received disconnected signal from libdbus
+ if (q_dbus_pending_call_get_completed(call->pending)) {
+ // decode the message
+ DBusMessage *reply = q_dbus_pending_call_steal_reply(call->pending);
+ msg = QDBusMessagePrivate::fromDBusMessage(reply, connection->capabilities);
+ q_dbus_message_unref(reply);
+ } else {
+ msg = QDBusMessage::createError(QDBusError::Disconnected, QDBusUtil::disconnectedErrorMessage());
+ }
}
qDBusDebug() << connection << "got message reply:" << msg;
@@ -2070,8 +2084,8 @@ void QDBusConnectionPrivate::sendInternal(QDBusPendingCallPrivate *pcall, void *
pcall->pending = pending;
q_dbus_pending_call_set_notify(pending, qDBusResultReceived, pcall, 0);
- // DBus won't notify us when a peer disconnects so we need to track these ourselves
- if (mode == QDBusConnectionPrivate::PeerMode)
+ // DBus won't notify us when a peer disconnects or server terminates so we need to track these ourselves
+ if (mode == QDBusConnectionPrivate::PeerMode || mode == QDBusConnectionPrivate::ClientMode)
pendingCalls.append(pcall);
return;
diff --git a/src/dbus/qdbusutil_p.h b/src/dbus/qdbusutil_p.h
index 8f5ae92..ca70ff9 100644
--- a/src/dbus/qdbusutil_p.h
+++ b/src/dbus/qdbusutil_p.h
@@ -155,6 +155,8 @@ namespace QDBusUtil
{ return QStringLiteral(DBUS_SERVICE_DBUS); }
inline QString dbusPath()
{ return QStringLiteral(DBUS_PATH_DBUS); }
+ inline QString dbusPathLocal()
+ { return QStringLiteral(DBUS_PATH_LOCAL); }
inline QString dbusInterface()
{
// it's the same string, but just be sure
@@ -165,8 +167,12 @@ namespace QDBusUtil
{ return QStringLiteral(DBUS_INTERFACE_PROPERTIES); }
inline QString dbusInterfaceIntrospectable()
{ return QStringLiteral(DBUS_INTERFACE_INTROSPECTABLE); }
+ inline QString dbusInterfaceLocal()
+ { return QStringLiteral(DBUS_INTERFACE_LOCAL); }
inline QString nameOwnerChanged()
{ return QStringLiteral("NameOwnerChanged"); }
+ inline QString disconnected()
+ { return QStringLiteral("Disconnected"); }
inline QString disconnectedErrorMessage()
{ return QStringLiteral("Not connected to D-Bus server"); }
}
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
index e91f87d..6c7e6b1 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
@@ -1218,6 +1218,28 @@ void tst_QDBusConnection::callVirtualObjectLocal()
QCOMPARE(obj.replyArguments, subPathReply.arguments());
}
+void tst_QDBusConnection::pendingCallWhenDisconnected()
+{
+ QDBusServer *server = new QDBusServer;
+ QDBusConnection con = QDBusConnection::connectToPeer(server->address(), "disconnect");
+ QTestEventLoop::instance().enterLoop(2);
+ QVERIFY(!QTestEventLoop::instance().timeout());
+ QVERIFY(con.isConnected());
+
+ delete server;
+
+ // Make sure we call the method before we know it is disconnected.
+ QVERIFY(con.isConnected());
+ QDBusMessage message = QDBusMessage::createMethodCall("", "/", QString(), "method");
+ QDBusPendingCall reply = con.asyncCall(message);
+
+ QTestEventLoop::instance().enterLoop(2);
+ QVERIFY(!con.isConnected());
+ QVERIFY(reply.isFinished());
+ QVERIFY(reply.isError());
+ QVERIFY(reply.error().type() == QDBusError::Disconnected);
+}
+
QString MyObject::path;
QString MyObjectWithoutInterface::path;
QString MyObjectWithoutInterface::interface;
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
index a53ba32..720e484 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
@@ -121,6 +121,7 @@ private slots:
void registerVirtualObject();
void callVirtualObject();
void callVirtualObjectLocal();
+ void pendingCallWhenDisconnected();
public:
QString serviceName() const { return "org.qtproject.Qt.Autotests.QDBusConnection"; }
--
2.5.0

37
QTBUG-51676-QtDBus-do-not-synchrnoize-local-message-in-daemon-th.patch Normal file
View File

@ -0,0 +1,37 @@
From d011c92b47f95554fe639ad9c0542768d802666b Mon Sep 17 00:00:00 2001
From: Weng Xuetian <wengxt@gmail.com>
Date: Fri, 4 Mar 2016 10:53:46 -0800
Subject: [PATCH] QtDBus: do not synchrnoize local message in daemon thread
When qDBusAddSpyHook is used and there is a local message, the
handleObjectCall on this message will be deferred to daemon thread. It
would cause dead lock if dbus daemon thread wait for this message and
there is no point to wait for the reply since daemon thread is not same
as the message sender thread.
Task-number: QTBUG-51676
Change-Id: I1dc112894cde7121e8ce302ae51b438ade1ff612
---
src/dbus/qdbusintegrator.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index cd44861..caf2e92 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1484,8 +1484,10 @@ void QDBusConnectionPrivate::handleObjectCall(const QDBusMessage &msg)
return;
}
- if (!QDBusMessagePrivate::isLocal(msg)) {
- // external incoming message
+ if (!QDBusMessagePrivate::isLocal(msg) || QThread::currentThread() == QDBusConnectionManager::instance()) {
+ // two cases:
+ // 1. external incoming message
+ // 2. local message deferred by spy hook
// post it and forget
postEventToThread(HandleObjectCallPostEventAction, result.obj,
new QDBusActivateObjectEvent(QDBusConnection(this), this, result,
--
2.5.0

18
qt5-qtbase.spec
View File

@ -48,7 +48,7 @@
Summary: Qt5 - QtBase components Summary: Qt5 - QtBase components
Name: qt5-qtbase Name: qt5-qtbase
Version: 5.6.0 Version: 5.6.0
Release: 0.38.%{prerelease}%{?dist} Release: 0.39.%{prerelease}%{?dist}
# See LGPL_EXCEPTIONS.txt, for exception details # See LGPL_EXCEPTIONS.txt, for exception details
License: LGPLv2 with exceptions or GPLv3 with exceptions License: LGPLv2 with exceptions or GPLv3 with exceptions
@ -96,6 +96,16 @@ Patch53: qtbase-opensource-src-5.6.0-alsa-1.1.patch
# arm patch # arm patch
Patch54: qtbase-opensource-src-5.6.0-arm.patch Patch54: qtbase-opensource-src-5.6.0-arm.patch
# https://codereview.qt-project.org/#/c/151496/
Patch55: QTBUG-51648-QtDBus-clean-up-signal-hooks-and-object-tree-in-clos.patch
# https://codereview.qt-project.org/#/c/151340/
Patch56: QTBUG-51649-QtDBus-finish-all-pending-call-with-error-if-disconn.patch
# https://codereview.qt-project.org/#/c/151459/
Patch57: QTBUG-51676-QtDBus-do-not-synchrnoize-local-message-in-daemon-th.patch
# Epel patches # Epel patches
Patch100: qt5-qtbase-5.6.0-el6-sqrt.patch Patch100: qt5-qtbase-5.6.0-el6-sqrt.patch
@ -357,6 +367,9 @@ RPM macros for building Qt5 packages.
%patch52 -p1 -b .moc_WORDSIZE %patch52 -p1 -b .moc_WORDSIZE
%patch53 -p1 -b .alsa1.1 %patch53 -p1 -b .alsa1.1
%patch54 -p1 -b .arm %patch54 -p1 -b .arm
%patch55 -p1 -b .QTBUG-51648
%patch56 -p1 -b .QTBUG-51649
%patch57 -p1 -b .QTBUG-51676
%patch100 -p1 -b .sqrt %patch100 -p1 -b .sqrt
@ -958,6 +971,9 @@ fi
%changelog %changelog
* Thu Mar 10 2016 Rex Dieter <rdieter@fedoraproject.org> 5.6.0-0.39.rc
- candidate fixes for various QtDBus deadlocks (QTBUG-51648,QTBUG-51649,QTBUG-51676)
* Mon Mar 07 2016 Rex Dieter <rdieter@fedoraproject.org> 5.6.0-0.38.rc * Mon Mar 07 2016 Rex Dieter <rdieter@fedoraproject.org> 5.6.0-0.38.rc
- backport "crash on start if system bus is not available" (QTBUG-51299) - backport "crash on start if system bus is not available" (QTBUG-51299)