AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make sure we don't deadlock when connecting signals

Browse Source 
This commit moves the code that finishes the signal-slot connection into
the QtDBus auxiliary thread. That is necessary because we're holding the
lock for writing while making blocking calls. The auxiliary thread might
be waiting for us to release that lock while processing some previous
message.

Change-Id: Iee8cbc07c4434ce9b560ffff13d0521b94a51833
Reviewed-by: Albert Astals Cid <aacid@kde.org>
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>
This commit is contained in:
Thiago Macieira 2015-03-30 09:02:40 -07:00
parent e15e5b1d37
commit e94f512b1e
3 changed files with 40 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/dbus/qdbusconnection.cpp
View File

@ -765,7 +765,6 @@ bool QDBusConnection::connect(const QString &service, const QString &path, const
return false;
}
QDBusWriteLocker locker(ConnectAction, d);
return d->connectSignal(service, path, interface, name, argumentMatch, signature, receiver, slot);
}
@ -821,7 +820,6 @@ bool QDBusConnection::disconnect(const QString &service, const QString &path, co
if (interface.isEmpty() && name.isEmpty())
return false;
QDBusWriteLocker locker(DisconnectAction, d);
return d->disconnectSignal(service, path, interface, name, argumentMatch, signature, receiver, slot);
}

8
src/dbus/qdbusconnection_p.h
View File

@ -213,8 +213,6 @@ public:
bool connectSignal(const QString &service, const QString &path, const QString& interface,
const QString &name, const QStringList &argumentMatch, const QString &signature,
QObject *receiver, const char *slot);
void connectSignal(const QString &key, const SignalHook &hook);
SignalHookHash::Iterator disconnectSignal(SignalHookHash::Iterator &it);
bool disconnectSignal(const QString &service, const QString &path, const QString& interface,
const QString &name, const QStringList &argumentMatch, const QString &signature,
QObject *receiver, const char *slot);
@ -254,6 +252,8 @@ private:
void deliverCall(QObject *object, int flags, const QDBusMessage &msg,
const QVector<int> &metaTypes, int slotIdx);
SignalHookHash::Iterator removeSignalHookNoLock(SignalHookHash::Iterator it);
bool isServiceRegisteredByThread(const QString &serviceName);
QString getNameOwnerNoCache(const QString &service);
@ -270,6 +270,8 @@ public slots:
void socketWrite(int);
void objectDestroyed(QObject *o);
void relaySignal(QObject *obj, const QMetaObject *, int signalId, const QVariantList &args);
void addSignalHook(const QString &key, const SignalHook &hook);
bool removeSignalHook(const QString &key, const SignalHook &hook);
private slots:
void serviceOwnerChangedNoLock(const QString &name, const QString &oldOwner, const QString &newOwner);
@ -279,6 +281,8 @@ private slots:
signals:
void dispatchStatusChanged();
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);
void serviceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner);
void callWithCallbackFailed(const QDBusError &error, const QDBusMessage &message);
void newServerConnection(QDBusConnectionPrivate *newConnection);

81
src/dbus/qdbusintegrator.cpp
View File

@ -968,6 +968,10 @@ QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p)
this, &QDBusConnectionPrivate::doDispatch, Qt::QueuedConnection);
connect(this, &QDBusConnectionPrivate::messageNeedsSending,
this, &QDBusConnectionPrivate::sendInternal);
connect(this, &QDBusConnectionPrivate::signalNeedsConnecting,
this, &QDBusConnectionPrivate::addSignalHook, Qt::BlockingQueuedConnection);
connect(this, &QDBusConnectionPrivate::signalNeedsDisconnecting,
this, &QDBusConnectionPrivate::removeSignalHook, Qt::BlockingQueuedConnection);
rootNode.flags = 0;
@ -1108,7 +1112,7 @@ void QDBusConnectionPrivate::objectDestroyed(QObject *obj)
SignalHookHash::iterator sit = signalHooks.begin();
while (sit != signalHooks.end()) {
if (static_cast<QObject *>(sit.value().obj) == obj)
sit = disconnectSignal(sit);
sit = removeSignalHookNoLock(sit);
else
++sit;
}
@ -2053,6 +2057,15 @@ bool QDBusConnectionPrivate::connectSignal(const QString &service,
if (!prepareHook(hook, key, service, path, interface, name, argumentMatch, receiver, slot, 0, false))
return false; // don't connect
Q_ASSERT(thread() != QThread::currentThread());
emit signalNeedsConnecting(key, hook);
return true;
}
void QDBusConnectionPrivate::addSignalHook(const QString &key, const SignalHook &hook)
{
QDBusWriteLocker locker(ConnectAction, this);
// avoid duplicating:
QDBusConnectionPrivate::SignalHookHash::ConstIterator it = signalHooks.constFind(key);
QDBusConnectionPrivate::SignalHookHash::ConstIterator end = signalHooks.constEnd();
@ -2065,24 +2078,18 @@ bool QDBusConnectionPrivate::connectSignal(const QString &service,
entry.midx == hook.midx &&
entry.argumentMatch == hook.argumentMatch) {
// no need to compare the parameters if it's the same slot
return true; // already there
return; // already there
}
}
connectSignal(key, hook);
return true;
}
void QDBusConnectionPrivate::connectSignal(const QString &key, const SignalHook &hook)
{
signalHooks.insertMulti(key, hook);
connect(hook.obj, SIGNAL(destroyed(QObject*)), SLOT(objectDestroyed(QObject*)),
Qt::ConnectionType(Qt::DirectConnection | Qt::UniqueConnection));
Qt::ConnectionType(Qt::BlockingQueuedConnection | Qt::UniqueConnection));
MatchRefCountHash::iterator it = matchRefCounts.find(hook.matchRule);
MatchRefCountHash::iterator mit = matchRefCounts.find(hook.matchRule);
if (it != matchRefCounts.end()) { // Match already present
it.value() = it.value() + 1;
if (mit != matchRefCounts.end()) { // Match already present
mit.value() = mit.value() + 1;
return;
}
@ -2128,7 +2135,14 @@ bool QDBusConnectionPrivate::disconnectSignal(const QString &service,
if (!prepareHook(hook, key, service, path, interface, name, argumentMatch, receiver, slot, 0, false))
return false; // don't disconnect
// avoid duplicating:
Q_ASSERT(thread() != QThread::currentThread());
return emit signalNeedsDisconnecting(key, hook);
}
bool QDBusConnectionPrivate::removeSignalHook(const QString &key, const SignalHook &hook)
{
// remove it from our list:
QDBusWriteLocker locker(ConnectAction, this);
QDBusConnectionPrivate::SignalHookHash::Iterator it = signalHooks.find(key);
QDBusConnectionPrivate::SignalHookHash::Iterator end = signalHooks.end();
for ( ; it != end && it.key() == key; ++it) {
@ -2140,7 +2154,7 @@ bool QDBusConnectionPrivate::disconnectSignal(const QString &service,
entry.midx == hook.midx &&
entry.argumentMatch == hook.argumentMatch) {
// no need to compare the parameters if it's the same slot
disconnectSignal(it);
removeSignalHookNoLock(it);
return true; // it was there
}
}
@ -2150,7 +2164,7 @@ bool QDBusConnectionPrivate::disconnectSignal(const QString &service,
}
QDBusConnectionPrivate::SignalHookHash::Iterator
QDBusConnectionPrivate::disconnectSignal(SignalHookHash::Iterator &it)
QDBusConnectionPrivate::removeSignalHookNoLock(SignalHookHash::Iterator it)
{
const SignalHook &hook = it.value();
@ -2196,7 +2210,7 @@ QDBusConnectionPrivate::disconnectSignal(SignalHookHash::Iterator &it)
void QDBusConnectionPrivate::registerObject(const ObjectTreeNode *node)
{
connect(node->obj, SIGNAL(destroyed(QObject*)), SLOT(objectDestroyed(QObject*)),
Qt::DirectConnection);
Qt::ConnectionType(Qt::BlockingQueuedConnection | Qt::UniqueConnection));
if (node->flags & (QDBusConnection::ExportAdaptors
| QDBusConnection::ExportScriptableSignals
@ -2250,21 +2264,8 @@ void QDBusConnectionPrivate::connectRelay(const QString &service,
QDBusAbstractInterface::staticMetaObject.methodCount(), true))
return; // don't connect
// add it to our list:
QDBusWriteLocker locker(ConnectRelayAction, this);
SignalHookHash::ConstIterator it = signalHooks.constFind(key);
SignalHookHash::ConstIterator end = signalHooks.constEnd();
for ( ; it != end && it.key() == key; ++it) {
const SignalHook &entry = it.value();
if (entry.service == hook.service &&
entry.path == hook.path &&
entry.signature == hook.signature &&
entry.obj == hook.obj &&
entry.midx == hook.midx)
return; // already there, no need to re-add
}
connectSignal(key, hook);
Q_ASSERT(thread() != QThread::currentThread());
emit signalNeedsConnecting(key, hook);
}
void QDBusConnectionPrivate::disconnectRelay(const QString &service,
@ -2284,22 +2285,8 @@ void QDBusConnectionPrivate::disconnectRelay(const QString &service,
QDBusAbstractInterface::staticMetaObject.methodCount(), true))
return; // don't connect
// remove it from our list:
QDBusWriteLocker locker(DisconnectRelayAction, this);
SignalHookHash::Iterator it = signalHooks.find(key);
SignalHookHash::Iterator end = signalHooks.end();
for ( ; it != end && it.key() == key; ++it) {
const SignalHook &entry = it.value();
if (entry.service == hook.service &&
entry.path == hook.path &&
entry.signature == hook.signature &&
entry.obj == hook.obj &&
entry.midx == hook.midx) {
// found it
disconnectSignal(it);
return;
}
}
Q_ASSERT(thread() != QThread::currentThread());
emit signalNeedsDisconnecting(key, hook);
}
bool QDBusConnectionPrivate::shouldWatchService(const QString &service)