Don't iterate over the connections without holding a lock

When checking whether a slot is connected to a signal, we need to
hold the signalSlotLock to be sure about the answer, or we can
get crashes when a connection gets removed while doing the check.

The check in activate() can handle some uncertainty as it's only
a shortcut to the longer path.

Fixes: QTBUG-74604
Change-Id: I3fc822455fbadc0223ef68632f5fb3df3ff3e86d
Reviewed-by: Aapo Keskimolo <aapo.keskimolo@qt.io>

2 files changed, 18 insertions, 1 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 759ccaacd8..268f3949fd 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -415,6 +415,22 @@ bool QObjectPrivate::isSignalConnected(uint signalIndex, bool checkDeclarative)
     return false;
 }
 
+bool QObjectPrivate::maybeSignalConnected(uint signalIndex) const
+{
+    ConnectionData *cd = connections.load();
+    if (!cd)
+        return false;
+
+    if (cd->allsignals.first)
+        return true;
+
+    if (signalIndex < uint(cd->signalVector.count())) {
+        const QObjectPrivate::Connection *c = cd->signalVector.at(signalIndex).first;
+        return c != nullptr;
+    }
+    return false;
+}
+
 
 /*!
     \internal
@@ -3599,7 +3615,7 @@ void doActivate(QObject *sender, int signal_index, void **argv)
     if (!argv)
         argv = empty_argv;
 
-    if (!sp->isSignalConnected(signal_index, false)) {
+    if (!sp->maybeSignalConnected(signal_index)) {
         // The possible declarative connection is done, and nothing else is connected
         if (callbacks_enabled && signal_spy_set->signal_begin_callback != nullptr)
             signal_spy_set->signal_begin_callback(sender, signal_index, argv);
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 823c7a195a..863689fddd 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -254,6 +254,7 @@ public:
 
     int signalIndex(const char *signalName, const QMetaObject **meta = nullptr) const;
     bool isSignalConnected(uint signalIdx, bool checkDeclarative = true) const;
+    bool maybeSignalConnected(uint signalIndex) const;
     inline bool isDeclarativeSignalConnected(uint signalIdx) const;
 
     // To allow abitrary objects to call connectNotify()/disconnectNotify() without making