From acd0dae3f49662047d76f5e7d6ab1f333c38549e Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sun, 8 Jun 2014 14:49:36 -0400 Subject: [PATCH] Fix data race in accessing QDBusConnectionPrivate::serviceNames The trick of creating a copy is not thread-safe. I'd known this since the moment I wrote that code, but thought "what could go wrong?". Task-number: QTBUG-39285 Change-Id: If521d4a649c06e6a34926687e85623aa25cb4c35 Reviewed-by: David Faure --- src/dbus/qdbusconnection_p.h | 2 +- src/dbus/qdbusintegrator.cpp | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index f2590f9c54..fd4ced078d 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -256,7 +256,7 @@ private: void deliverCall(QObject *object, int flags, const QDBusMessage &msg, const QVector &metaTypes, int slotIdx); - bool isServiceRegisteredByThread(const QString &serviceName) const; + bool isServiceRegisteredByThread(const QString &serviceName); QString getNameOwnerNoCache(const QString &service); diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index 1fef6d4d49..6b6ac6bc62 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -2488,12 +2488,15 @@ void QDBusConnectionPrivate::unregisterServiceNoLock(const QString &serviceName) serviceNames.removeAll(serviceName); } -bool QDBusConnectionPrivate::isServiceRegisteredByThread(const QString &serviceName) const +bool QDBusConnectionPrivate::isServiceRegisteredByThread(const QString &serviceName) { if (!serviceName.isEmpty() && serviceName == baseService) return true; - QStringList copy = serviceNames; - return copy.contains(serviceName); + if (serviceName == dbusServiceString()) + return false; + + QDBusReadLocker locker(UnregisterServiceAction, this); + return serviceNames.contains(serviceName); } void QDBusConnectionPrivate::postEventToThread(int action, QObject *object, QEvent *ev)