Take the right lock before using a connection's receiver

When a signal/slot connection is activated, a lock on the receiver
object is taken (to be sure it doesn't get destroyed).
The path for blocking queued connections however took the lock on
the sender by accident, fix that.

Pick-to: 5.15 5.12
Change-Id: I8cd938a50eca2bf71e7bfb86768ee0c8431afdfa
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

1 files changed, 87 insertions, 0 deletions

diff --git a/tests/auto/other/qobjectrace/tst_qobjectrace.cpp b/tests/auto/other/qobjectrace/tst_qobjectrace.cpp
index e09d304ff3..dba1fddb76 100644
--- a/tests/auto/other/qobjectrace/tst_qobjectrace.cpp
+++ b/tests/auto/other/qobjectrace/tst_qobjectrace.cpp
@@ -47,6 +47,7 @@ class tst_QObjectRace: public QObject
 private slots:
     void moveToThreadRace();
     void destroyRace();
+    void blockingQueuedDestroyRace();
     void disconnectRace();
 };
 
@@ -298,6 +299,92 @@ void tst_QObjectRace::destroyRace()
         delete threads[i];
 }
 
+class BlockingQueuedDestroyRaceObject : public QObject
+{
+    Q_OBJECT
+
+public:
+    enum class Behavior { Normal, Crash };
+    explicit BlockingQueuedDestroyRaceObject(Behavior b = Behavior::Normal)
+        : m_behavior(b) {}
+
+signals:
+    bool aSignal();
+
+public slots:
+    bool aSlot()
+    {
+        switch (m_behavior) {
+        case Behavior::Normal:
+            return true;
+        case Behavior::Crash:
+            qFatal("Race detected in a blocking queued connection");
+            break;
+        }
+
+        Q_UNREACHABLE();
+        return false;
+    }
+
+private:
+    Behavior m_behavior;
+};
+
+void tst_QObjectRace::blockingQueuedDestroyRace()
+{
+    enum { MinIterations = 100, MinTime = 3000, WaitTime = 25 };
+
+    BlockingQueuedDestroyRaceObject sender;
+
+    QDeadlineTimer timer(MinTime);
+    int iteration = 0;
+
+    while (iteration++ < MinIterations || !timer.hasExpired()) {
+        // Manually allocate some storage, and create a receiver in there
+        std::aligned_storage<
+                sizeof(BlockingQueuedDestroyRaceObject),
+                alignof(BlockingQueuedDestroyRaceObject)
+            >::type storage;
+
+        auto *receiver = reinterpret_cast<BlockingQueuedDestroyRaceObject *>(&storage);
+        new (receiver) BlockingQueuedDestroyRaceObject(BlockingQueuedDestroyRaceObject::Behavior::Normal);
+
+        // Connect it to the sender via BlockingQueuedConnection
+        QVERIFY(connect(&sender, &BlockingQueuedDestroyRaceObject::aSignal,
+                        receiver, &BlockingQueuedDestroyRaceObject::aSlot,
+                        Qt::BlockingQueuedConnection));
+
+        const auto emitUntilDestroyed = [&sender] {
+            // Hack: as long as the receiver is alive and the connection
+            // established, the signal will return true (from the slot).
+            // When the receiver gets destroyed, the signal is disconnected
+            // and therefore the emission returns false.
+            while (emit sender.aSignal())
+                ;
+        };
+
+        std::unique_ptr<QThread> thread(QThread::create(emitUntilDestroyed));
+        thread->start();
+
+        QTest::qWait(WaitTime);
+
+        // Destroy the receiver, and immediately allocate a new one at
+        // the same address. In case of a race, this might cause:
+        // - the metacall event to be posted to a destroyed object;
+        // - the metacall event to be posted to the wrong object.
+        // In both cases we hope to catch the race by crashing.
+        receiver->~BlockingQueuedDestroyRaceObject();
+        new (receiver) BlockingQueuedDestroyRaceObject(BlockingQueuedDestroyRaceObject::Behavior::Crash);
+
+        // Flush events
+        QTest::qWait(0);
+
+        thread->wait();
+
+        receiver->~BlockingQueuedDestroyRaceObject();
+    }
+}
+
 static QAtomicInteger<unsigned> countedStructObjectsCount;
 struct CountedFunctor
 {