QObject: don't hold mutex when copying arguments in a QueuedConnection

QMetaType::create can call user code and we should not keep mutex held as
this may cause dead lock.

Make sure the tst_qobjectrace actually emit some signal so the test check
there is no race if the receiver object is destroyed while
the mutex is unlocked.

Task-number: QTBUG-39990
Change-Id: I56ca1ae7a11cd7b33c1a68727370972862e11c2f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/kernel/qobject.cpp

@@ -3512,7 +3512,8 @@ void QMetaObject::connectSlotsByName(QObject *o)
 
     \a signal must be in the signal index range (see QObjectPrivate::signalIndex()).
 */
-static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connection *c, void **argv)
+static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connection *c, void **argv,
+                            QMutexLocker &locker)
 {
     const int *argumentTypes = c->argumentTypes.load();
     if (!argumentTypes && argumentTypes != &DIRECT_CONNECTION_ONLY) {
@@ -3537,8 +3538,28 @@ static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connect
     Q_CHECK_PTR(args);
     types[0] = 0; // return type
     args[0] = 0; // return value
-    for (int n = 1; n < nargs; ++n)
+
-        args[n] = QMetaType::create((types[n] = argumentTypes[n-1]), argv[n]);
+    if (nargs > 1) {
+        for (int n = 1; n < nargs; ++n)
+            types[n] = argumentTypes[n-1];
+
+        locker.unlock();
+        for (int n = 1; n < nargs; ++n)
+            args[n] = QMetaType::create(types[n], argv[n]);
+        locker.relock();
+
+        if (!c->receiver) {
+            locker.unlock();
+            // we have been disconnected while the mutex was unlocked
+            for (int n = 1; n < nargs; ++n)
+                QMetaType::destroy(types[n], args[n]);
+            free(types);
+            free(args);
+            locker.relock();
+            return;
+        }
+    }
+
     QMetaCallEvent *ev = c->isSlotObject ?
         new QMetaCallEvent(c->slotObj, sender, signal, nargs, types, args) :
         new QMetaCallEvent(c->method_offset, c->method_relative, c->callFunction, sender, signal, nargs, types, args);
@@ -3638,7 +3659,7 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
             // put into the event queue
             if ((c->connectionType == Qt::AutoConnection && !receiverInSameThread)
                 || (c->connectionType == Qt::QueuedConnection)) {
-                queued_activate(sender, signal_index, c, argv ? argv : empty_argv);
+                queued_activate(sender, signal_index, c, argv ? argv : empty_argv, locker);
                 continue;
 #ifndef QT_NO_THREAD
             } else if (c->connectionType == Qt::BlockingQueuedConnection) {

tests/auto/corelib/kernel/qobject/tst_qobject.cpp

@@ -270,6 +270,21 @@ public slots:
 
 int ReceiverObject::sequence = 0;
 
+static void playWithObjects()
+{
+    // Do operations that will lock the internal signalSlotLock mutex on many QObjects.
+    // The more QObjects, the higher the chance that the signalSlotLock mutex used
+    // is already in use. If the number of objects is higher than the number of mutexes in
+    // the pool (currently 131), the deadlock should always trigger. Use an even higher number
+    // to be on the safe side.
+    const int objectCount = 1024;
+    SenderObject lotsOfObjects[objectCount];
+    for (int i = 0; i < objectCount; ++i) {
+        QObject::connect(&lotsOfObjects[i], &SenderObject::signal1,
+                         &lotsOfObjects[i], &SenderObject::aPublicSlot);
+    }
+}
+
 void tst_QObject::initTestCase()
 {
     const QString testDataDir = QFileInfo(QFINDTESTDATA("signalbug")).absolutePath();
@@ -1368,10 +1383,10 @@ struct CheckInstanceCount
 struct CustomType
 {
     CustomType(int l1 = 0, int l2 = 0, int l3 = 0): i1(l1), i2(l2), i3(l3)
-    { ++instanceCount; }
+    { ++instanceCount; playWithObjects(); }
     CustomType(const CustomType &other): i1(other.i1), i2(other.i2), i3(other.i3)
-    { ++instanceCount; }
+    { ++instanceCount; playWithObjects(); }
-    ~CustomType() { --instanceCount; }
+    ~CustomType() { --instanceCount; playWithObjects(); }
 
     int i1, i2, i3;
     int value() { return i1 + i2 + i3; }
@@ -5749,17 +5764,7 @@ public:
     {}
 
     ~MyFunctor() {
-        // Do operations that will lock the internal signalSlotLock mutex on many QObjects.
+        playWithObjects();
-        // The more QObjects, the higher the chance that the signalSlotLock mutex used
-        // is already in use. If the number of objects is higher than the number of mutexes in
-        // the pool (currently 131), the deadlock should always trigger. Use an even higher number
-        // to be on the safe side.
-        const int objectCount = 1024;
-        SenderObject lotsOfObjects[objectCount];
-        for (int i = 0; i < objectCount; ++i) {
-            QObject::connect(&lotsOfObjects[i], &SenderObject::signal1,
-                             &lotsOfObjects[i], &SenderObject::aPublicSlot);
-        }
     }
 
     void operator()() {

tests/auto/other/qobjectrace/tst_qobjectrace.cpp

@@ -172,14 +172,18 @@ void tst_QObjectRace::moveToThreadRace()
 
 class MyObject : public QObject
 {   Q_OBJECT
+        bool ok;
+    public:
+        MyObject() : ok(true) {}
+        ~MyObject() { Q_ASSERT(ok); ok = false; }
     public slots:
-        void slot1() { emit signal1(); }
+        void slot1() { Q_ASSERT(ok); }
-        void slot2() { emit signal2(); }
+        void slot2() { Q_ASSERT(ok); }
-        void slot3() { emit signal3(); }
+        void slot3() { Q_ASSERT(ok); }
-        void slot4() { emit signal4(); }
+        void slot4() { Q_ASSERT(ok); }
-        void slot5() { emit signal5(); }
+        void slot5() { Q_ASSERT(ok); }
-        void slot6() { emit signal6(); }
+        void slot6() { Q_ASSERT(ok); }
-        void slot7() { emit signal7(); }
+        void slot7() { Q_ASSERT(ok); }
     signals:
         void signal1();
         void signal2();
@@ -237,6 +241,10 @@ public:
             disconnect(objects[((i+4)*41) % nAlive], _signalsPMF[(18*i)%7], objects[((i+5)*43) % nAlive],  _slotsPMF[(19*i+2)%7] );
 
             QMetaObject::Connection c = connect(objects[((i+5)*43) % nAlive], _signalsPMF[(9*i+1)%7], Functor());
+
+            for (int f = 0; f < 7; ++f)
+                emit (objects[i]->*_signalsPMF[f])();
+
             disconnect(c);
 
             disconnect(objects[i], _signalsPMF[(10*i+5)%7], 0, 0);
@@ -249,6 +257,9 @@ public:
 
             delete objects[i];
         }
+
+        //run the possible queued slots
+        qApp->processEvents();
     }
 };