diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 54229a9f718..62bee3d90bf 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -3764,7 +3764,7 @@ void doActivate(QObject *sender, int signal_index, void **argv) } QSemaphore semaphore; { - QBasicMutexLocker locker(signalSlotLock(sender)); + QBasicMutexLocker locker(signalSlotLock(receiver)); if (!c->receiver.loadAcquire()) continue; QMetaCallEvent *ev = c->isSlotObject ? diff --git a/tests/auto/other/qobjectrace/tst_qobjectrace.cpp b/tests/auto/other/qobjectrace/tst_qobjectrace.cpp index e09d304ff38..dba1fddb765 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(&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 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 countedStructObjectsCount; struct CountedFunctor {