Resolve data race on QQmlPropertyCache's arguments object

We need make its assignment atomic.

This does not apply to Qt6, because in Qt6 we don't need the property
cache to resolve argument types.

Task-number: QTBUG-93973
Change-Id: I9588c3582754a9aba3a6aa9e325de87d44d06aa2
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 21 insertions, 25 deletions

diff --git a/src/qml/qml/qqmlmetaobject.cpp b/src/qml/qml/qqmlmetaobject.cpp
index 988f4f2811..a6a1b57c17 100644
--- a/src/qml/qml/qqmlmetaobject.cpp
+++ b/src/qml/qml/qqmlmetaobject.cpp
@@ -232,8 +232,6 @@ int *QQmlMetaObject::methodParameterTypes(int index, ArgTypeStorage *argStorage,
     Q_ASSERT(!_m.isNull() && index >= 0);
 
     if (_m.isT1()) {
-        typedef QQmlPropertyCacheMethodArguments A;
-
         QQmlPropertyCache *c = _m.asT1();
         Q_ASSERT(index < c->methodIndexCacheStart + c->methodIndexCache.count());
 
@@ -242,19 +240,16 @@ int *QQmlMetaObject::methodParameterTypes(int index, ArgTypeStorage *argStorage,
 
         QQmlPropertyData *rv = const_cast<QQmlPropertyData *>(&c->methodIndexCache.at(index - c->methodIndexCacheStart));
 
-        if (rv->arguments() && static_cast<A *>(rv->arguments())->argumentsValid)
-            return static_cast<A *>(rv->arguments())->arguments;
+        if (QQmlPropertyCacheMethodArguments *args = rv->arguments())
+            return args->arguments;
 
         const QMetaObject *metaObject = c->createMetaObject();
         Q_ASSERT(metaObject);
         QMetaMethod m = metaObject->method(index);
 
         int argc = m.parameterCount();
-        if (!rv->arguments()) {
-            A *args = c->createArgumentsObject(argc, m.parameterNames());
-            rv->setArguments(args);
-        }
-        A *args = static_cast<A *>(rv->arguments());
+
+        QQmlPropertyCacheMethodArguments *args = c->createArgumentsObject(argc, m.parameterNames());
 
         QList<QByteArray> argTypeNames; // Only loaded if needed
 
@@ -280,8 +275,14 @@ int *QQmlMetaObject::methodParameterTypes(int index, ArgTypeStorage *argStorage,
             }
             args->arguments[ii + 1] = type;
         }
-        args->argumentsValid = true;
-        return static_cast<A *>(rv->arguments())->arguments;
+
+        // If we cannot set it, then another thread has set it in the mean time.
+        // Just return that one, then. We don't have to delete the arguments object
+        // we've created as it's tracked in a linked list.
+        if (rv->setArguments(args))
+            return args->arguments;
+        else
+            return rv->arguments()->arguments;
 
     } else {
         QMetaMethod m = _m.asT2()->method(index);
diff --git a/src/qml/qml/qqmlpropertycache.cpp b/src/qml/qml/qqmlpropertycache.cpp
index c5e34e1d15..11e967105c 100644
--- a/src/qml/qml/qqmlpropertycache.cpp
+++ b/src/qml/qml/qqmlpropertycache.cpp
@@ -161,7 +161,6 @@ void QQmlPropertyData::load(const QMetaProperty &p)
 static void populate(QQmlPropertyData *data, const QMetaMethod &m)
 {
     data->setCoreIndex(m.methodIndex());
-    data->setArguments(nullptr);
 
     QQmlPropertyData::Flags flags = data->flags();
     flags.type = QQmlPropertyData::Flags::FunctionType;
@@ -322,7 +321,6 @@ void QQmlPropertyCache::appendSignal(const QString &name, QQmlPropertyData::Flag
     data.setPropType(QMetaType::UnknownType);
     data.setCoreIndex(coreIndex);
     data.setFlags(flags);
-    data.setArguments(nullptr);
 
     QQmlPropertyData handler = data;
     handler.m_flags.setIsSignalHandler(true);
@@ -331,7 +329,6 @@ void QQmlPropertyCache::appendSignal(const QString &name, QQmlPropertyData::Flag
         int argumentCount = *types;
         QQmlPropertyCacheMethodArguments *args = createArgumentsObject(argumentCount, names);
         ::memcpy(args->arguments, types, (argumentCount + 1) * sizeof(int));
-        args->argumentsValid = true;
         data.setArguments(args);
     }
 
@@ -365,7 +362,6 @@ void QQmlPropertyCache::appendMethod(const QString &name, QQmlPropertyData::Flag
     QQmlPropertyCacheMethodArguments *args = createArgumentsObject(argumentCount, names);
     for (int ii = 0; ii < argumentCount; ++ii)
         args->arguments[ii + 1] = parameterTypes.at(ii);
-    args->argumentsValid = true;
     data.setArguments(args);
 
     data.setFlags(flags);
@@ -897,12 +893,11 @@ QQmlPropertyCacheMethodArguments *QQmlPropertyCache::createArgumentsObject(int a
     typedef QQmlPropertyCacheMethodArguments A;
     A *args = static_cast<A *>(malloc(sizeof(A) + (argc) * sizeof(int)));
     args->arguments[0] = argc;
-    args->argumentsValid = false;
     args->signalParameterStringForJS = nullptr;
-    args->parameterError = false;
     args->names = argc ? new QList<QByteArray>(names) : nullptr;
-    args->next = argumentsCache;
-    argumentsCache = args;
+    do {
+        args->next = argumentsCache;
+    } while (!argumentsCache.testAndSetRelease(args->next, args));
     return args;
 }
 
@@ -1196,7 +1191,6 @@ void QQmlPropertyCache::toMetaObjectBuilder(QMetaObjectBuilder &builder)
         QQmlPropertyCacheMethodArguments *arguments = nullptr;
         if (data->hasArguments()) {
             arguments = (QQmlPropertyCacheMethodArguments *)data->arguments();
-            Q_ASSERT(arguments->argumentsValid);
             for (int ii = 0; ii < arguments->arguments[0]; ++ii) {
                 if (ii != 0) signature.append(',');
                 signature.append(QMetaType::typeName(arguments->arguments[1 + ii]));
diff --git a/src/qml/qml/qqmlpropertycache_p.h b/src/qml/qml/qqmlpropertycache_p.h
index 088c382d7d..46f924a938 100644
--- a/src/qml/qml/qqmlpropertycache_p.h
+++ b/src/qml/qml/qqmlpropertycache_p.h
@@ -241,7 +241,7 @@ private:
     QByteArray _dynamicClassName;
     QByteArray _dynamicStringData;
     QString _defaultPropertyName;
-    QQmlPropertyCacheMethodArguments *argumentsCache;
+    QAtomicPointer<QQmlPropertyCacheMethodArguments> argumentsCache;
     int _jsFactoryMethodIndex;
     QByteArray _checksum;
 };
diff --git a/src/qml/qml/qqmlpropertycachemethodarguments_p.h b/src/qml/qml/qqmlpropertycachemethodarguments_p.h
index 978f6ec1e4..096571040c 100644
--- a/src/qml/qml/qqmlpropertycachemethodarguments_p.h
+++ b/src/qml/qml/qqmlpropertycachemethodarguments_p.h
@@ -64,8 +64,6 @@ public:
 
     //for signal handler rewrites
     QString *signalParameterStringForJS;
-    int parameterError:1;
-    int argumentsValid:1;
 
     QList<QByteArray> *names;
 
diff --git a/src/qml/qml/qqmlpropertydata_p.h b/src/qml/qml/qqmlpropertydata_p.h
index 345342af16..6dd58d6900 100644
--- a/src/qml/qml/qqmlpropertydata_p.h
+++ b/src/qml/qml/qqmlpropertydata_p.h
@@ -322,7 +322,10 @@ public:
     }
 
     QQmlPropertyCacheMethodArguments *arguments() const { return m_arguments; }
-    void setArguments(QQmlPropertyCacheMethodArguments *args) { m_arguments = args; }
+    bool setArguments(QQmlPropertyCacheMethodArguments *args)
+    {
+        return m_arguments.testAndSetRelease(nullptr, args);
+    }
 
     int metaObjectOffset() const { return m_metaObjectOffset; }
     void setMetaObjectOffset(int off)
@@ -435,7 +438,7 @@ private:
     quint8 m_typeMinorVersion = 0;
     qint16 m_metaObjectOffset = -1;
 
-    QQmlPropertyCacheMethodArguments *m_arguments = nullptr;
+    QAtomicPointer<QQmlPropertyCacheMethodArguments> m_arguments;
     StaticMetaCallFunction m_staticMetaCallFunction = nullptr;
 };