QSignalSpy: pull makeArgs() out of ctor critical section

First store the result in a temporary, and, in the critical section,
only move it into place. This minimizes the amount of code in the
critical section (Amdahl's Law) and also means we don't need to
permanently drop the mutex when we call unbounded code using
metacall(). That, in turn, makes sure the args member is only ever
seen empty or fully populated.

Since makeArgs() no longer accesses member functions now, we can make
it static.

Task-number: QTBUG-123544
Change-Id: If19db53f85d7c9eb18d4fb2c61e1aa3d4b9c2e00
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: David Faure <david.faure@kdab.com>

2 files changed, 9 insertions, 5 deletions

diff --git a/src/testlib/qsignalspy.cpp b/src/testlib/qsignalspy.cpp
index 05b9868e34..52be7ff212 100644
--- a/src/testlib/qsignalspy.cpp
+++ b/src/testlib/qsignalspy.cpp
@@ -223,17 +223,14 @@ QSignalSpy::ObjectSignal QSignalSpy::verify(const QObject *obj, QMetaMethod sign
 QList<int> QSignalSpy::makeArgs(const QMetaMethod &member, const QObject *obj)
 {
     QList<int> result;
-    QMutexLocker locker(&m_mutex);
     result.reserve(member.parameterCount());
     for (int i = 0; i < member.parameterCount(); ++i) {
         QMetaType tp = member.parameterMetaType(i);
         if (!tp.isValid() && obj) {
-            locker.unlock();
             void *argv[] = { &tp, &i };
             QMetaObject::metacall(const_cast<QObject*>(obj),
                                   QMetaObject::RegisterMethodArgumentMetaType,
                                   member.methodIndex(), argv);
-            locker.relock();
         }
         if (!tp.isValid()) {
             qWarning("QSignalSpy: Unable to handle parameter '%s' of type '%s' of method '%s',"
diff --git a/src/testlib/qsignalspy.h b/src/testlib/qsignalspy.h
index af9587b369..cbb3bfbe42 100644
--- a/src/testlib/qsignalspy.h
+++ b/src/testlib/qsignalspy.h
@@ -12,6 +12,8 @@
 #include <QtCore/qvariant.h>
 #include <QtCore/qmutex.h>
 
+#include <mutex>
+
 QT_BEGIN_NAMESPACE
 
 
@@ -66,7 +68,12 @@ private:
     {
         if (!os.obj)
             return;
-        args = makeArgs(os.sig, os.obj);
+
+        auto tmp = makeArgs(os.sig, os.obj);
+        {
+            const auto lock = std::scoped_lock(m_mutex);
+            args = std::move(tmp);
+        }
         if (!connectToSignal(os.obj, os.sig.methodIndex()))
             return;
 
@@ -82,7 +89,7 @@ private:
     Q_TESTLIB_EXPORT static bool isSignalMetaMethodValid(const QMetaMethod &signal);
     Q_TESTLIB_EXPORT static bool isObjectValid(const QObject *object);
 
-    Q_TESTLIB_EXPORT QList<int> makeArgs(const QMetaMethod &member, const QObject *obj);
+    Q_TESTLIB_EXPORT static QList<int> makeArgs(const QMetaMethod &member, const QObject *obj);
     Q_TESTLIB_EXPORT void appendArgs(void **a);
 
     // the full, normalized signal name