whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Implement the blocking QtDBus call in terms of the non-blocking one 

This simplifies the code a little by having a single code path. More
importantly, we no longer need to call the evil function
dbus_connection_send_with_reply_and_block. That function acquires a lock
on the socket transport inside libdbus-1, which means all threads need
to wait until the one call gets unblocked before they can continue.

To do that, this commit reimplements the QDBus::Block part of
QDBusConnectionPrivate::sendWithReply by reusing the existing call to
sendWithReplyAsync() and then doing a blocking-wait with
QDBusPendingCallPrivate::waitForFinished().

By using (Q)DBusPendingCall and the threaded connection approach (next
commit), now we never block on the socket. That also means the code to
call dbus_pending_call_block() is no longer necessary and the
waitForFinished() function itself can be considerably simplified.

As a side-effect of no longer blocking, a number of pre-existing race
conditions that used to be hidden showed up.

Note: this commit deadlocks without the threading (next commits).

Task-number: QTBUG-43585
Change-Id: Ic5d393bfd36e48a193fcffff13b73754954a3f7d
Reviewed-by: Albert Astals Cid <aacid@kde.org>
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>
bb10
Thiago Macieira 2014-12-29 17:14:18 -02:00
parent edaf7c30d4
commit 939b7c630d
7 changed files with 37 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/dbus/qdbus_symbols_p.h
View File

@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
** Copyright (C) 2015 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@ -205,11 +206,6 @@ DEFINEFUNC(dbus_bool_t , dbus_connection_send_with_reply, (DBusConnection
DBusPendingCall **pending_return,
int timeout_milliseconds),
(connection, message, pending_return, timeout_milliseconds), return)
DEFINEFUNC(DBusMessage * , dbus_connection_send_with_reply_and_block, (DBusConnection *connection,
DBusMessage *message,
int timeout_milliseconds,
DBusError *error),
(connection, message, timeout_milliseconds, error), return)
DEFINEFUNC(void , dbus_connection_set_exit_on_disconnect, (DBusConnection *connection,
dbus_bool_t exit_on_disconnect),
(connection, exit_on_disconnect), )

1
src/dbus/qdbusconnection_p.h
View File

@ -223,7 +223,6 @@ public:
void unregisterService(const QString &serviceName);
bool handleMessage(const QDBusMessage &msg);
void waitForFinished(QDBusPendingCallPrivate *pcall);
QDBusMetaObject *findMetaObject(const QString &service, const QString &path,
const QString &interface, QDBusError &error);

95
src/dbus/qdbusintegrator.cpp
View File

@ -1790,34 +1790,6 @@ static void qDBusResultReceived(DBusPendingCall *pending, void *user_data)
}
}
void QDBusConnectionPrivate::waitForFinished(QDBusPendingCallPrivate *pcall)
{
Q_ASSERT(pcall->pending);
//Q_ASSERT(pcall->mutex.isLocked()); // there's no such function
if (pcall->waitingForFinished) {
// another thread is already waiting
pcall->waitForFinishedCondition.wait(&pcall->mutex);
} else {
pcall->waitingForFinished = true;
pcall->mutex.unlock();
{
QDBusDispatchLocker locker(PendingCallBlockAction, this);
q_dbus_pending_call_block(pcall->pending);
// QDBusConnectionPrivate::processFinishedCall() is called automatically
}
pcall->mutex.lock();
if (pcall->pending) {
q_dbus_pending_call_unref(pcall->pending);
pcall->pending = 0;
}
pcall->waitForFinishedCondition.wakeAll();
}
}
void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
{
QDBusConnectionPrivate *connection = const_cast<QDBusConnectionPrivate *>(call->connection);
@ -1833,7 +1805,7 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
msg = QDBusMessagePrivate::fromDBusMessage(reply, connection->capabilities);
q_dbus_message_unref(reply);
}
qDBusDebug() << connection << "got message reply (async):" << msg;
qDBusDebug() << connection << "got message reply:" << msg;
// Check if the reply has the expected signature
call->checkReceivedSignature();
@ -1855,7 +1827,8 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
qDBusDebug() << "Deliver failed!";
}
if (call->pending && !call->waitingForFinished) {
if (call->pending) {
call->waitForFinishedCondition.wakeAll();
q_dbus_pending_call_unref(call->pending);
call->pending = 0;
}
@ -2008,43 +1981,12 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message,
// special case for synchronous local calls
return sendWithReplyLocal(message);
if (!QCoreApplication::instance() || sendMode == QDBus::Block) {
QDBusError err;
DBusMessage *msg = QDBusMessagePrivate::toDBusMessage(message, capabilities, &err);
if (!msg) {
qWarning("QDBusConnection: error: could not send message to service \"%s\" path \"%s\" interface \"%s\" member \"%s\": %s",
qPrintable(message.service()), qPrintable(message.path()),
qPrintable(message.interface()), qPrintable(message.member()),
qPrintable(err.message()));
lastError = err;
return QDBusMessage::createError(err);
}
QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, 0, 0, 0, timeout);
Q_ASSERT(pcall);
qDBusDebug() << this << "sending message (blocking):" << message;
QDBusErrorInternal error;
DBusMessage *reply;
{
QDBusDispatchLocker locker(SendWithReplyAndBlockAction, this);
reply = q_dbus_connection_send_with_reply_and_block(connection, msg, timeout, error);
}
q_dbus_message_unref(msg);
if (!!error) {
lastError = err = error;
return QDBusMessage::createError(err);
}
QDBusMessage amsg = QDBusMessagePrivate::fromDBusMessage(reply, capabilities);
q_dbus_message_unref(reply);
qDBusDebug() << this << "got message reply (blocking):" << amsg;
return amsg;
} else { // use the event loop
QDBusPendingCallPrivate *pcall = sendWithReplyAsync(message, 0, 0, 0, timeout);
Q_ASSERT(pcall);
if (pcall->replyMessage.type() == QDBusMessage::InvalidMessage) {
if (pcall->replyMessage.type() == QDBusMessage::InvalidMessage) {
// need to wait for the reply
if (sendMode == QDBus::BlockWithGui) {
pcall->watcherHelper = new QDBusPendingCallWatcherHelper;
QEventLoop loop;
loop.connect(pcall->watcherHelper, SIGNAL(reply(QDBusMessage)), SLOT(quit()));
@ -2052,18 +1994,17 @@ QDBusMessage QDBusConnectionPrivate::sendWithReply(const QDBusMessage &message,
// enter the event loop and wait for a reply
loop.exec(QEventLoop::ExcludeUserInputEvents | QEventLoop::WaitForMoreEvents);
} else {
pcall->waitForFinished();
}
QDBusMessage reply = pcall->replyMessage;
lastError = QDBusError(reply); // set or clear error
bool r = pcall->ref.deref();
Q_ASSERT(!r);
Q_UNUSED(r);
delete pcall;
return reply;
}
QDBusMessage reply = pcall->replyMessage;
lastError = QDBusError(reply); // set or clear error
if (!pcall->ref.deref())
delete pcall;
return reply;
}
QDBusMessage QDBusConnectionPrivate::sendWithReplyLocal(const QDBusMessage &message)
@ -2163,7 +2104,7 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
return pcall;
}
qDBusDebug() << this << "sending message (async):" << message;
qDBusDebug() << this << "sending message:" << message;
DBusPendingCall *pending = 0;
QDBusDispatchLocker locker(SendWithReplyAsyncAction, this);

3
src/dbus/qdbuspendingcall.cpp
View File

@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
** Copyright (C) 2015 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@ -231,7 +232,7 @@ void QDBusPendingCallPrivate::waitForFinished()
if (replyMessage.type() != QDBusMessage::InvalidMessage)
return; // already finished
connection->waitForFinished(this);
waitForFinishedCondition.wait(&mutex);
}
/*!

4
src/dbus/qdbuspendingcall_p.h
View File

@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
** Copyright (C) 2015 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@ -89,11 +90,10 @@ public:
DBusPendingCall *pending;
QString expectedReplySignature;
int expectedReplyCount;
bool waitingForFinished;
// }
QDBusPendingCallPrivate(const QDBusMessage &sent, QDBusConnectionPrivate *connection)
: sentMessage(sent), connection(connection), watcherHelper(0), pending(0), waitingForFinished(false)
: sentMessage(sent), connection(connection), watcherHelper(0), pending(0)
{ }
~QDBusPendingCallPrivate();
bool setReplyCallback(QObject *target, const char *member);

4
src/dbus/qdbusthreaddebug_p.h
View File

@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
** Copyright (C) 2015 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtDBus module of the Qt Toolkit.
@ -87,8 +88,7 @@ enum ThreadAction {
ActivateSignalAction = 27,
PendingCallBlockAction = 28,
SendMessageAction = 29,
SendWithReplyAndBlockAction = 30,
HuntAndEmitAction = 31,
HuntAndEmitAction = 30,
AddTimeoutAction = 50,
RealAddTimeoutAction = 51,

14
tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
View File

@ -1,6 +1,7 @@
/****************************************************************************
**
** Copyright (C) 2015 The Qt Company Ltd.
** Copyright (C) 2015 Intel Corporation.
** Contact: http://www.qt.io/licensing/
**
** This file is part of the test suite of the Qt Toolkit.
@ -525,6 +526,7 @@ public slots:
QVERIFY(isConnected());
QVERIFY(c.isConnected());
QVERIFY(registerObject(c));
QTestEventLoop::instance().exitLoop();
}
private:
@ -552,11 +554,14 @@ void tst_QDBusConnection::registerObjectPeer()
MyServer server(path);
QDBusConnection::connectToPeer(server.address(), "beforeFoo");
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
{
QDBusConnection con = QDBusConnection::connectToPeer(server.address(), "foo");
QCoreApplication::processEvents();
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
QVERIFY(con.isConnected());
MyObject obj;
@ -565,6 +570,7 @@ void tst_QDBusConnection::registerObjectPeer()
}
QDBusConnection::connectToPeer(server.address(), "afterFoo");
QTestEventLoop::instance().enterLoop(2);
{
QDBusConnection con("foo");
@ -713,6 +719,7 @@ public slots:
m_conn = c;
QVERIFY(isConnected());
QVERIFY(m_conn.isConnected());
QTestEventLoop::instance().exitLoop();
}
private:
@ -724,7 +731,8 @@ void tst_QDBusConnection::registerObjectPeer2()
{
MyServer2 server;
QDBusConnection con = QDBusConnection::connectToPeer(server.address(), "foo");
QCoreApplication::processEvents();
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
QVERIFY(con.isConnected());
QDBusConnection srv_con = server.connection();
@ -879,6 +887,8 @@ void tst_QDBusConnection::registerQObjectChildrenPeer()
{
MyServer2 server;
QDBusConnection con = QDBusConnection::connectToPeer(server.address(), "foo");
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
QCoreApplication::processEvents();
QVERIFY(con.isConnected());