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 <[email protected]>
HamedMasafi · Sep 22, 2012 · 373a727 · 373a727
1 parent f799e57
commit 373a727
Show file tree

Hide file tree

Showing 12 changed files with 79 additions and 59 deletions.
diff --git 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
@@ -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<void*>(static_cast<const void*>(&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
@@ -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<QDBusVariant *>(to) = arg.value<QDBusVariant>();
             return;
-        } else if (id == QDBusMetaTypeId::objectpath) {
+        } else if (id == QDBusMetaTypeId::objectpath()) {
             *reinterpret_cast<QDBusObjectPath *>(to) = arg.value<QDBusObjectPath>();
             return;
-        } else if (id == QDBusMetaTypeId::signature) {
+        } else if (id == QDBusMetaTypeId::signature()) {
             *reinterpret_cast<QDBusSignature *>(to) = arg.value<QDBusSignature>();
             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
@@ -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
@@ -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<QDBusArgument>(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<QDBusArgument>(arg);
         QDBusArgumentPrivate *d = QDBusArgumentPrivate::d(dbusargument);
         if (!d->message)

diff --git a/src/dbus/qdbusmetatype.cpp b/src/dbus/qdbusmetatype.cpp
@@ -89,31 +89,21 @@ inline static void registerHelper(T * = 0)
         reinterpret_cast<QDBusMetaType::DemarshallFunction>(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;
 
     // 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<T>() does this implicitly)
-        message = qMetaTypeId<QDBusMessage>();
-        error = qMetaTypeId<QDBusError>();
-#endif
-        argument = qMetaTypeId<QDBusArgument>();
-        variant = qMetaTypeId<QDBusVariant>();
-        objectpath = qMetaTypeId<QDBusObjectPath>();
-        signature = qMetaTypeId<QDBusSignature>();
-        unixfd = qMetaTypeId<QDBusUnixFileDescriptor>();
+        (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<QList<QDBusUnixFileDescriptor> >();
 #endif
 
-#ifdef QT_BOOTSTRAPPED
-        const int lastId = qDBusRegisterMetaType<QList<QDBusUnixFileDescriptor> >();
-        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
@@ -55,21 +55,56 @@
 
 #include <qdbusmetatype.h>
 
+#include <qdbusmessage.h>
+#include <qdbusargument.h>
+#include <qdbusextratypes.h>
+#include <qdbuserror.h>
+#include <qdbusunixfiledescriptor.h>
+
 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<QList<QDBusUnixFileDescriptor> >() + 1; }
+#else
+{ return qMetaTypeId<QDBusMessage>(); }
+#endif
+
+inline int QDBusMetaTypeId::argument()
+{ return qMetaTypeId<QDBusArgument>(); }
+
+inline int QDBusMetaTypeId::variant()
+{ return qMetaTypeId<QDBusVariant>(); }
+
+inline int QDBusMetaTypeId::objectpath()
+{ return qMetaTypeId<QDBusObjectPath>(); }
+
+inline int QDBusMetaTypeId::signature()
+{ return qMetaTypeId<QDBusSignature>(); }
+
+inline int QDBusMetaTypeId::error()
+#ifdef QT_BOOTSTRAPPED
+{ return qDBusRegisterMetaType<QList<QDBusUnixFileDescriptor> >() + 2; }
+#else
+{ return qMetaTypeId<QDBusError>(); }
+#endif
+
+inline int QDBusMetaTypeId::unixfd()
+{ return qMetaTypeId<QDBusUnixFileDescriptor>(); }
+
 QT_END_NAMESPACE
 
 #endif
diff --git a/src/dbus/qdbusmisc.cpp b/src/dbus/qdbusmisc.cpp
@@ -186,7 +186,7 @@ int qDBusParametersForMethod(const QList<QByteArray> &parameterTypes, QVector<in
             return -1;
         }
 
-        if (id == QDBusMetaTypeId::message)
+        if (id == QDBusMetaTypeId::message())
             seenMessage = true;
         else if (QDBusMetaType::typeToSignature(id) == 0)
             return -1;

diff --git a/src/dbus/qdbuspendingcall.cpp b/src/dbus/qdbuspendingcall.cpp
@@ -172,12 +172,12 @@ bool QDBusPendingCallPrivate::setReplyCallback(QObject *target, const char *memb
     // success
     // construct the expected signature
     int count = metaTypes.count() - 1;
-    if (count == 1 && metaTypes.at(1) == QDBusMetaTypeId::message) {
+    if (count == 1 && metaTypes.at(1) == QDBusMetaTypeId::message()) {
         // wildcard slot, can receive anything, so don't set the signature
         return true;
     }
 
-    if (metaTypes.at(count) == QDBusMetaTypeId::message)
+    if (metaTypes.at(count) == QDBusMetaTypeId::message())
         --count;
 
     setMetaTypes(count, count ? metaTypes.constData() + 1 : 0);

diff --git a/src/dbus/qdbusreply.cpp b/src/dbus/qdbusreply.cpp
@@ -203,7 +203,7 @@ void qDBusReplyFill(const QDBusMessage &reply, QDBusError &error, QVariant &data
     QByteArray receivedSignature;
 
     if (reply.arguments().count() >= 1) {
-        if (reply.arguments().at(0).userType() == QDBusMetaTypeId::argument) {
+        if (reply.arguments().at(0).userType() == QDBusMetaTypeId::argument()) {
             // compare signatures instead
             QDBusArgument arg = qvariant_cast<QDBusArgument>(reply.arguments().at(0));
             receivedSignature = arg.currentSignature().toLatin1();

diff --git 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
@@ -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;
         }