From 373a7277db0583fbbb8ed61f9f61d6f77ed95da4 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 24 Aug 2012 12:06:03 +0200 Subject: [PATCH] QDBusMetaTypeId: don't cache the result of qMetaTypeId<>() in static ints There's not much point in caching the result of qMetaTypeId<>, because it's already internally memoised. In addition, the code that initialised the static int caches wasn't protected against concurrent access under the assumption that the operations performed were thread-safe. That is true for most of them, but not for the stores to the static ints, which race against each other: // Thread A // Thread B r1 = initialized /*=false*/ r1 = initialized /*=false*/ r2 = qMetaTypeId<...>(); r2 = qMetaTypeId<...>(); message = r2; message = r2; // race, ditto for all other ints To fix, turn the ints into inline functions that just call the respective qMetaTypeId<>() function. Change-Id: I5aa80c624872c3867232abc26ffdcde70cd54022 Reviewed-by: Thiago Macieira --- src/dbus/qdbusabstractadaptor.cpp | 2 +- src/dbus/qdbusintegrator.cpp | 12 +++--- src/dbus/qdbusinterface.cpp | 8 ++-- src/dbus/qdbusinternalfilters.cpp | 2 +- src/dbus/qdbusmarshaller.cpp | 4 +- src/dbus/qdbusmetatype.cpp | 45 ++++++++--------------- src/dbus/qdbusmetatype_p.h | 49 +++++++++++++++++++++---- src/dbus/qdbusmisc.cpp | 2 +- src/dbus/qdbuspendingcall.cpp | 4 +- src/dbus/qdbusreply.cpp | 2 +- src/dbus/qdbusxmlgenerator.cpp | 4 +- src/tools/qdbuscpp2xml/qdbuscpp2xml.cpp | 4 +- 12 files changed, 79 insertions(+), 59 deletions(-) diff --git a/src/dbus/qdbusabstractadaptor.cpp b/src/dbus/qdbusabstractadaptor.cpp index e8236dd90a..d17f10f422 100644 --- a/src/dbus/qdbusabstractadaptor.cpp +++ b/src/dbus/qdbusabstractadaptor.cpp @@ -304,7 +304,7 @@ void QDBusAdaptorConnector::relay(QObject *senderObj, int lastSignalIdx, void ** // qDBusParametersForMethod has already complained return; if (inputCount + 1 != types.count() || - types.at(inputCount) == QDBusMetaTypeId::message) { + types.at(inputCount) == QDBusMetaTypeId::message()) { // invalid signal signature // qDBusParametersForMethod has not yet complained about this one qWarning("QDBusAbstractAdaptor: Cannot relay signal %s::%s", diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index ff6927ccfe..aa794d6cd9 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -667,7 +667,7 @@ static int findSlot(const QMetaObject *mo, const QByteArray &name, int flags, metaTypes[0] = returnType; bool hasMessage = false; if (inputCount > 0 && - metaTypes.at(inputCount) == QDBusMetaTypeId::message) { + metaTypes.at(inputCount) == QDBusMetaTypeId::message()) { // "no input parameters" is allowed as long as the message meta type is there hasMessage = true; --inputCount; @@ -738,7 +738,7 @@ QDBusCallDeliveryEvent* QDBusConnectionPrivate::prepareReply(QDBusConnectionPriv Q_UNUSED(object); int n = metaTypes.count() - 1; - if (metaTypes[n] == QDBusMetaTypeId::message) + if (metaTypes[n] == QDBusMetaTypeId::message()) --n; if (msg.arguments().count() < n) @@ -838,7 +838,7 @@ bool QDBusConnectionPrivate::activateCall(QObject* object, int flags, const QDBu // try with no parameters, but with a QDBusMessage slotData.slotIdx = ::findSlot(mo, memberName, flags, QString(), slotData.metaTypes); if (slotData.metaTypes.count() != 2 || - slotData.metaTypes.at(1) != QDBusMetaTypeId::message) { + slotData.metaTypes.at(1) != QDBusMetaTypeId::message()) { // not found // save the negative lookup slotData.slotIdx = -1; @@ -889,7 +889,7 @@ void QDBusConnectionPrivate::deliverCall(QObject *object, int /*flags*/, const Q int pCount = qMin(msg.arguments().count(), metaTypes.count() - 1); for (i = 1; i <= pCount; ++i) { int id = metaTypes[i]; - if (id == QDBusMetaTypeId::message) + if (id == QDBusMetaTypeId::message()) break; const QVariant &arg = msg.arguments().at(i - 1); @@ -918,7 +918,7 @@ void QDBusConnectionPrivate::deliverCall(QObject *object, int /*flags*/, const Q } } - if (metaTypes.count() > i && metaTypes[i] == QDBusMetaTypeId::message) { + if (metaTypes.count() > i && metaTypes[i] == QDBusMetaTypeId::message()) { params.append(const_cast(static_cast(&msg))); ++i; } @@ -1298,7 +1298,7 @@ bool QDBusConnectionPrivate::prepareHook(QDBusConnectionPrivate::SignalHook &hoo if (buildSignature) { hook.signature.clear(); for (int i = 1; i < hook.params.count(); ++i) - if (hook.params.at(i) != QDBusMetaTypeId::message) + if (hook.params.at(i) != QDBusMetaTypeId::message()) hook.signature += QLatin1String( QDBusMetaType::typeToSignature( hook.params.at(i) ) ); } diff --git a/src/dbus/qdbusinterface.cpp b/src/dbus/qdbusinterface.cpp index 47b5b7b173..29d00077ae 100644 --- a/src/dbus/qdbusinterface.cpp +++ b/src/dbus/qdbusinterface.cpp @@ -106,13 +106,13 @@ static void copyArgument(void *to, int id, const QVariant &arg) return; } - if (id == QDBusMetaTypeId::variant) { + if (id == QDBusMetaTypeId::variant()) { *reinterpret_cast(to) = arg.value(); return; - } else if (id == QDBusMetaTypeId::objectpath) { + } else if (id == QDBusMetaTypeId::objectpath()) { *reinterpret_cast(to) = arg.value(); return; - } else if (id == QDBusMetaTypeId::signature) { + } else if (id == QDBusMetaTypeId::signature()) { *reinterpret_cast(to) = arg.value(); return; } @@ -123,7 +123,7 @@ static void copyArgument(void *to, int id, const QVariant &arg) } // if we got here, it's either an un-dermarshalled type or a mismatch - if (arg.userType() != QDBusMetaTypeId::argument) { + if (arg.userType() != QDBusMetaTypeId::argument()) { // it's a mismatch //qWarning? return; diff --git a/src/dbus/qdbusinternalfilters.cpp b/src/dbus/qdbusinternalfilters.cpp index 6b38218a73..ac6cc08104 100644 --- a/src/dbus/qdbusinternalfilters.cpp +++ b/src/dbus/qdbusinternalfilters.cpp @@ -354,7 +354,7 @@ static int writeProperty(QObject *obj, const QByteArray &property_name, QVariant return PropertyWriteFailed; } - if (id != QMetaType::QVariant && value.userType() == QDBusMetaTypeId::argument) { + if (id != QMetaType::QVariant && value.userType() == QDBusMetaTypeId::argument()) { // we have to demarshall before writing void *null = 0; QVariant other(id, null); diff --git a/src/dbus/qdbusmarshaller.cpp b/src/dbus/qdbusmarshaller.cpp index 00ef7fd9d8..0a91a7c38e 100644 --- a/src/dbus/qdbusmarshaller.cpp +++ b/src/dbus/qdbusmarshaller.cpp @@ -185,7 +185,7 @@ inline bool QDBusMarshaller::append(const QDBusVariant &arg) QByteArray tmpSignature; const char *signature = 0; - if (id == QDBusMetaTypeId::argument) { + if (id == QDBusMetaTypeId::argument()) { // take the signature from the QDBusArgument object we're marshalling tmpSignature = qvariant_cast(value).currentSignature().toLatin1(); @@ -372,7 +372,7 @@ bool QDBusMarshaller::appendVariantInternal(const QVariant &arg) } // intercept QDBusArgument parameters here - if (id == QDBusMetaTypeId::argument) { + if (id == QDBusMetaTypeId::argument()) { QDBusArgument dbusargument = qvariant_cast(arg); QDBusArgumentPrivate *d = QDBusArgumentPrivate::d(dbusargument); if (!d->message) diff --git a/src/dbus/qdbusmetatype.cpp b/src/dbus/qdbusmetatype.cpp index dade9788b3..d8ee0b31e7 100644 --- a/src/dbus/qdbusmetatype.cpp +++ b/src/dbus/qdbusmetatype.cpp @@ -89,14 +89,6 @@ inline static void registerHelper(T * = 0) reinterpret_cast(df)); } -int QDBusMetaTypeId::message; -int QDBusMetaTypeId::argument; -int QDBusMetaTypeId::variant; -int QDBusMetaTypeId::objectpath; -int QDBusMetaTypeId::signature; -int QDBusMetaTypeId::error; -int QDBusMetaTypeId::unixfd; - void QDBusMetaTypeId::init() { static volatile bool initialized = false; @@ -104,16 +96,14 @@ void QDBusMetaTypeId::init() // reentrancy is not a problem since everything else is locked on their own // set the guard variable at the end if (!initialized) { -#ifndef QT_BOOTSTRAPPED // register our types with QtCore (calling qMetaTypeId() does this implicitly) - message = qMetaTypeId(); - error = qMetaTypeId(); -#endif - argument = qMetaTypeId(); - variant = qMetaTypeId(); - objectpath = qMetaTypeId(); - signature = qMetaTypeId(); - unixfd = qMetaTypeId(); + (void)message(); + (void)argument(); + (void)variant(); + (void)objectpath(); + (void)signature(); + (void)error(); + (void)unixfd(); #ifndef QDBUS_NO_SPECIALTYPES // and register QtCore's with us @@ -145,11 +135,6 @@ void QDBusMetaTypeId::init() qDBusRegisterMetaType >(); #endif -#ifdef QT_BOOTSTRAPPED - const int lastId = qDBusRegisterMetaType >(); - message = lastId + 1; - error = lastId + 2; -#endif initialized = true; } } @@ -353,16 +338,16 @@ int QDBusMetaType::signatureToType(const char *signature) return QVariant::String; case DBUS_TYPE_OBJECT_PATH: - return QDBusMetaTypeId::objectpath; + return QDBusMetaTypeId::objectpath(); case DBUS_TYPE_SIGNATURE: - return QDBusMetaTypeId::signature; + return QDBusMetaTypeId::signature(); case DBUS_TYPE_UNIX_FD: - return QDBusMetaTypeId::unixfd; + return QDBusMetaTypeId::unixfd(); case DBUS_TYPE_VARIANT: - return QDBusMetaTypeId::variant; + return QDBusMetaTypeId::variant(); case DBUS_TYPE_ARRAY: // special case switch (signature[1]) { @@ -444,13 +429,13 @@ const char *QDBusMetaType::typeToSignature(int type) } QDBusMetaTypeId::init(); - if (type == QDBusMetaTypeId::variant) + if (type == QDBusMetaTypeId::variant()) return DBUS_TYPE_VARIANT_AS_STRING; - else if (type == QDBusMetaTypeId::objectpath) + else if (type == QDBusMetaTypeId::objectpath()) return DBUS_TYPE_OBJECT_PATH_AS_STRING; - else if (type == QDBusMetaTypeId::signature) + else if (type == QDBusMetaTypeId::signature()) return DBUS_TYPE_SIGNATURE_AS_STRING; - else if (type == QDBusMetaTypeId::unixfd) + else if (type == QDBusMetaTypeId::unixfd()) return DBUS_TYPE_UNIX_FD_AS_STRING; // try the database diff --git a/src/dbus/qdbusmetatype_p.h b/src/dbus/qdbusmetatype_p.h index 8239657232..8814408cc1 100644 --- a/src/dbus/qdbusmetatype_p.h +++ b/src/dbus/qdbusmetatype_p.h @@ -55,21 +55,56 @@ #include +#include +#include +#include +#include +#include + QT_BEGIN_NAMESPACE struct QDBusMetaTypeId { - static int message; // QDBusMessage - static int argument; // QDBusArgument - static int variant; // QDBusVariant - static int objectpath; // QDBusObjectPath - static int signature; // QDBusSignature - static int error; // QDBusError - static int unixfd; // QDBusUnixFileDescriptor + static int message(); // QDBusMessage + static int argument(); // QDBusArgument + static int variant(); // QDBusVariant + static int objectpath(); // QDBusObjectPath + static int signature(); // QDBusSignature + static int error(); // QDBusError + static int unixfd(); // QDBusUnixFileDescriptor static void init(); }; +inline int QDBusMetaTypeId::message() +#ifdef QT_BOOTSTRAPPED +{ return qDBusRegisterMetaType >() + 1; } +#else +{ return qMetaTypeId(); } +#endif + +inline int QDBusMetaTypeId::argument() +{ return qMetaTypeId(); } + +inline int QDBusMetaTypeId::variant() +{ return qMetaTypeId(); } + +inline int QDBusMetaTypeId::objectpath() +{ return qMetaTypeId(); } + +inline int QDBusMetaTypeId::signature() +{ return qMetaTypeId(); } + +inline int QDBusMetaTypeId::error() +#ifdef QT_BOOTSTRAPPED +{ return qDBusRegisterMetaType >() + 2; } +#else +{ return qMetaTypeId(); } +#endif + +inline int QDBusMetaTypeId::unixfd() +{ return qMetaTypeId(); } + QT_END_NAMESPACE #endif diff --git a/src/dbus/qdbusmisc.cpp b/src/dbus/qdbusmisc.cpp index d92349b35c..3c8e906cab 100644 --- a/src/dbus/qdbusmisc.cpp +++ b/src/dbus/qdbusmisc.cpp @@ -186,7 +186,7 @@ int qDBusParametersForMethod(const QList ¶meterTypes, QVector= 1) { - if (reply.arguments().at(0).userType() == QDBusMetaTypeId::argument) { + if (reply.arguments().at(0).userType() == QDBusMetaTypeId::argument()) { // compare signatures instead QDBusArgument arg = qvariant_cast(reply.arguments().at(0)); receivedSignature = arg.currentSignature().toLatin1(); diff --git a/src/dbus/qdbusxmlgenerator.cpp b/src/dbus/qdbusxmlgenerator.cpp index 9b888f39bc..e342d5c2b5 100644 --- a/src/dbus/qdbusxmlgenerator.cpp +++ b/src/dbus/qdbusxmlgenerator.cpp @@ -172,7 +172,7 @@ static QString generateInterfaceXml(const QMetaObject *mo, int flags, int method continue; // invalid form if (isSignal && inputCount + 1 != types.count()) continue; // signal with output arguments? - if (isSignal && types.at(inputCount) == QDBusMetaTypeId::message) + if (isSignal && types.at(inputCount) == QDBusMetaTypeId::message()) continue; // signal with QDBusMessage argument? if (isSignal && mm.attributes() & QMetaMethod::Cloned) continue; // cloned signal? @@ -181,7 +181,7 @@ static QString generateInterfaceXml(const QMetaObject *mo, int flags, int method bool isScriptable = mm.attributes() & QMetaMethod::Scriptable; for (j = 1; j < types.count(); ++j) { // input parameter for a slot or output for a signal - if (types.at(j) == QDBusMetaTypeId::message) { + if (types.at(j) == QDBusMetaTypeId::message()) { isScriptable = true; continue; } diff --git a/src/tools/qdbuscpp2xml/qdbuscpp2xml.cpp b/src/tools/qdbuscpp2xml/qdbuscpp2xml.cpp index 7383362f40..370cabec7a 100644 --- a/src/tools/qdbuscpp2xml/qdbuscpp2xml.cpp +++ b/src/tools/qdbuscpp2xml/qdbuscpp2xml.cpp @@ -145,13 +145,13 @@ static QString addFunction(const FunctionDef &mm, bool isSignal = false) { return QString(); // invalid form if (isSignal && inputCount + 1 != types.count()) return QString(); // signal with output arguments? - if (isSignal && types.at(inputCount) == QDBusMetaTypeId::message) + if (isSignal && types.at(inputCount) == QDBusMetaTypeId::message()) return QString(); // signal with QDBusMessage argument? bool isScriptable = mm.isScriptable; for (int j = 1; j < types.count(); ++j) { // input parameter for a slot or output for a signal - if (types.at(j) == QDBusMetaTypeId::message) { + if (types.at(j) == QDBusMetaTypeId::message()) { isScriptable = true; continue; }