summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThiago Macieira <thiago.macieira@nokia.com>2010-05-18 13:58:24 (GMT)
committerThiago Macieira <thiago.macieira@nokia.com>2010-05-24 19:09:31 (GMT)
commitfd256203708e79d64bad856e39bb5a43357bfd06 (patch)
tree7ec4edc7310b4a0724c3d6ffd3fce8a234f7c0c2
parent314a36895e1a31dd2b62de265d9160238d96e512 (diff)
downloadQt-fd256203708e79d64bad856e39bb5a43357bfd06.zip
Qt-fd256203708e79d64bad856e39bb5a43357bfd06.tar.gz
Qt-fd256203708e79d64bad856e39bb5a43357bfd06.tar.bz2
Fix a race condition with QtDBus blocking for replies.
If an auxiliary thread tried to block on waiting for a reply, and at the same time the main thread handled the reply, there's room for a race condition. So ensure only one thread is stopped at dbus_pending_call_block(). The other thread(s) will be waiting on the QWaitCondition. It's not a race condition for the main thread to process (and finish processing) the reply while the auxiliary thread hasn't even started to wait. The code will ensure that the reply is properly seen. Task-Id: https://projects.maemo.org/bugzilla/show_bug.cgi?id=155306 Reviewed-By: Trust Me
-rw-r--r--src/dbus/qdbusintegrator.cpp44
-rw-r--r--src/dbus/qdbuspendingcall.cpp35
-rw-r--r--src/dbus/qdbuspendingcall_p.h35
-rw-r--r--src/dbus/qdbuspendingreply.cpp3
4 files changed, 83 insertions, 34 deletions
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index 6354770..0f49dcc 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1687,15 +1687,31 @@ static void qDBusResultReceived(DBusPendingCall *pending, void *user_data)
void QDBusConnectionPrivate::waitForFinished(QDBusPendingCallPrivate *pcall)
{
Q_ASSERT(pcall->pending);
- QDBusDispatchLocker locker(PendingCallBlockAction, this);
- q_dbus_pending_call_block(pcall->pending);
- // QDBusConnectionPrivate::processFinishedCall() is called automatically
+ Q_ASSERT(!pcall->autoDelete);
+ //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();
+ }
}
void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
{
QDBusConnectionPrivate *connection = const_cast<QDBusConnectionPrivate *>(call->connection);
+ QMutexLocker locker(&call->mutex);
+
QDBusMessage &msg = call->replyMessage;
if (call->pending) {
// decode the message
@@ -1725,6 +1741,12 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
qDBusDebug() << "Deliver failed!";
}
+ if (call->pending)
+ q_dbus_pending_call_unref(call->pending);
+ call->pending = 0;
+
+ locker.unlock();
+
// Are there any watchers?
if (call->watcherHelper)
call->watcherHelper->emitSignals(msg, call->sentMessage);
@@ -1732,12 +1754,10 @@ void QDBusConnectionPrivate::processFinishedCall(QDBusPendingCallPrivate *call)
if (msg.type() == QDBusMessage::ErrorMessage)
emit connection->callWithCallbackFailed(QDBusError(msg), call->sentMessage);
- if (call->pending)
- q_dbus_pending_call_unref(call->pending);
- call->pending = 0;
-
- if (call->autoDelete)
+ if (call->autoDelete) {
+ Q_ASSERT(!call->waitingForFinished); // can't wait on a call with autoDelete!
delete call;
+ }
}
int QDBusConnectionPrivate::send(const QDBusMessage& message)
@@ -1879,17 +1899,14 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
{
if (isServiceRegisteredByThread(message.service())) {
// special case for local calls
- QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate;
- pcall->sentMessage = message;
+ QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate(message, this);
pcall->replyMessage = sendWithReplyLocal(message);
- pcall->connection = this;
return pcall;
}
checkThread();
- QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate;
- pcall->sentMessage = message;
+ QDBusPendingCallPrivate *pcall = new QDBusPendingCallPrivate(message, this);
pcall->ref = 0;
QDBusError error;
@@ -1913,7 +1930,6 @@ QDBusPendingCallPrivate *QDBusConnectionPrivate::sendWithReplyAsync(const QDBusM
q_dbus_message_unref(msg);
pcall->pending = pending;
- pcall->connection = this;
q_dbus_pending_call_set_notify(pending, qDBusResultReceived, pcall, 0);
return pcall;
diff --git a/src/dbus/qdbuspendingcall.cpp b/src/dbus/qdbuspendingcall.cpp
index 3db5d9f..86b585d 100644
--- a/src/dbus/qdbuspendingcall.cpp
+++ b/src/dbus/qdbuspendingcall.cpp
@@ -208,6 +208,8 @@ void QDBusPendingCallPrivate::setMetaTypes(int count, const int *types)
void QDBusPendingCallPrivate::checkReceivedSignature()
{
+ // MUST BE CALLED WITH A LOCKED MUTEX!
+
if (replyMessage.type() == QDBusMessage::InvalidMessage)
return; // not yet finished - no message to
// validate against
@@ -230,6 +232,8 @@ void QDBusPendingCallPrivate::checkReceivedSignature()
void QDBusPendingCallPrivate::waitForFinished()
{
+ QMutexLocker locker(&mutex);
+
if (replyMessage.type() != QDBusMessage::InvalidMessage)
return; // already finished
@@ -310,7 +314,11 @@ QDBusPendingCall &QDBusPendingCall::operator=(const QDBusPendingCall &other)
bool QDBusPendingCall::isFinished() const
{
- return !d || (d->replyMessage.type() != QDBusMessage::InvalidMessage);
+ if (!d)
+ return true; // considered finished
+
+ QMutexLocker locker(&d->mutex);
+ return d->replyMessage.type() != QDBusMessage::InvalidMessage;
}
void QDBusPendingCall::waitForFinished()
@@ -329,7 +337,10 @@ void QDBusPendingCall::waitForFinished()
*/
bool QDBusPendingCall::isValid() const
{
- return d ? d->replyMessage.type() == QDBusMessage::ReplyMessage : false;
+ if (!d)
+ return false;
+ QMutexLocker locker(&d->mutex);
+ return d->replyMessage.type() == QDBusMessage::ReplyMessage;
}
/*!
@@ -343,7 +354,10 @@ bool QDBusPendingCall::isValid() const
*/
bool QDBusPendingCall::isError() const
{
- return d ? d->replyMessage.type() == QDBusMessage::ErrorMessage : true;
+ if (!d)
+ return true; // considered finished and an error
+ QMutexLocker locker(&d->mutex);
+ return d->replyMessage.type() == QDBusMessage::ErrorMessage;
}
/*!
@@ -356,8 +370,10 @@ bool QDBusPendingCall::isError() const
*/
QDBusError QDBusPendingCall::error() const
{
- if (d)
+ if (d) {
+ QMutexLocker locker(&d->mutex);
return d->replyMessage;
+ }
// not connected, return an error
QDBusError err = QDBusError(QDBusError::Disconnected,
@@ -378,7 +394,10 @@ QDBusError QDBusPendingCall::error() const
*/
QDBusMessage QDBusPendingCall::reply() const
{
- return d ? d->replyMessage : QDBusMessage::createError(error());
+ if (!d)
+ return QDBusMessage::createError(error());
+ QMutexLocker locker(&d->mutex);
+ return d->replyMessage;
}
#if 0
@@ -439,9 +458,8 @@ QDBusPendingCall QDBusPendingCall::fromCompletedCall(const QDBusMessage &msg)
QDBusPendingCallPrivate *d = 0;
if (msg.type() == QDBusMessage::ErrorMessage ||
msg.type() == QDBusMessage::ReplyMessage) {
- d = new QDBusPendingCallPrivate;
+ d = new QDBusPendingCallPrivate(QDBusMessage(), 0);
d->replyMessage = msg;
- d->connection = 0;
}
return QDBusPendingCall(d);
@@ -471,9 +489,10 @@ QDBusPendingCallWatcher::QDBusPendingCallWatcher(const QDBusPendingCall &call, Q
: QObject(*new QDBusPendingCallWatcherPrivate, parent), QDBusPendingCall(call)
{
if (d) { // QDBusPendingCall::d
+ QMutexLocker locker(&d->mutex);
if (!d->watcherHelper) {
d->watcherHelper = new QDBusPendingCallWatcherHelper;
- if (isFinished()) {
+ if (d->replyMessage.type() != QDBusMessage::InvalidMessage) {
// cause a signal emission anyways
QMetaObject::invokeMethod(d->watcherHelper, "finished", Qt::QueuedConnection);
}
diff --git a/src/dbus/qdbuspendingcall_p.h b/src/dbus/qdbuspendingcall_p.h
index 641c397..c3aed53 100644
--- a/src/dbus/qdbuspendingcall_p.h
+++ b/src/dbus/qdbuspendingcall_p.h
@@ -57,6 +57,8 @@
#include <qshareddata.h>
#include <qpointer.h>
#include <qlist.h>
+#include <qmutex.h>
+#include <qwaitcondition.h>
#include "qdbusmessage.h"
#include "qdbus_symbols_p.h"
@@ -71,24 +73,35 @@ class QDBusConnectionPrivate;
class QDBusPendingCallPrivate: public QSharedData
{
public:
- QDBusMessage sentMessage;
- QDBusMessage replyMessage;
-// QDBusMessage pendingReplyMessage; // used in the local loop
- QDBusPendingCallWatcherHelper *watcherHelper;
- DBusPendingCall *pending;
- QDBusConnectionPrivate *connection;
+ // {
+ // set only during construction:
+ const QDBusMessage sentMessage;
+ QDBusConnectionPrivate * const connection;
- QString expectedReplySignature;
- int expectedReplyCount;
-
- // for the callback
+ // for the callback mechanism (see setReplyCallback and QDBusConnectionPrivate::sendWithReplyAsync)
QPointer<QObject> receiver;
QList<int> metaTypes;
int methodIdx;
bool autoDelete;
+ // }
+
+ mutable QMutex mutex;
+ QWaitCondition waitForFinishedCondition;
+
+ // {
+ // protected by the mutex above:
+ QDBusPendingCallWatcherHelper *watcherHelper;
+ QDBusMessage replyMessage;
+ DBusPendingCall *pending;
+ volatile bool waitingForFinished;
+
+ QString expectedReplySignature;
+ int expectedReplyCount;
+ // }
- QDBusPendingCallPrivate() : watcherHelper(0), pending(0), autoDelete(false)
+ QDBusPendingCallPrivate(const QDBusMessage &sent, QDBusConnectionPrivate *connection)
+ : sentMessage(sent), connection(connection), autoDelete(false), watcherHelper(0), pending(0), waitingForFinished(false)
{ }
~QDBusPendingCallPrivate();
bool setReplyCallback(QObject *target, const char *member);
diff --git a/src/dbus/qdbuspendingreply.cpp b/src/dbus/qdbuspendingreply.cpp
index b492660..1df47f4 100644
--- a/src/dbus/qdbuspendingreply.cpp
+++ b/src/dbus/qdbuspendingreply.cpp
@@ -252,7 +252,7 @@ void QDBusPendingReplyData::assign(const QDBusPendingCall &other)
void QDBusPendingReplyData::assign(const QDBusMessage &message)
{
- d = new QDBusPendingCallPrivate; // drops the reference to the old one
+ d = new QDBusPendingCallPrivate(QDBusMessage(), 0); // drops the reference to the old one
d->replyMessage = message;
}
@@ -271,6 +271,7 @@ QVariant QDBusPendingReplyData::argumentAt(int index) const
void QDBusPendingReplyData::setMetaTypes(int count, const int *types)
{
Q_ASSERT(d);
+ QMutexLocker locker(&d->mutex);
d->setMetaTypes(count, types);
d->checkReceivedSignature();
}