From b6323f0c7ba24a4ad5daa3535fcc9d6b285cfe28 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Tue, 8 Jun 2021 17:47:46 +0200
Subject: [PATCH] cleanOrphanedConnectionsImpl: Allow to skip locking

This function is/will be used in a few places where we already have a
lock. Temporarily unlocking and relocking invites all kinds of troubles.
By adding a flag we can instead tell the function that we already hold
the lock.

Change-Id: Ibca089de61133661d5cd75290f2a55c22c5d013c
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
(cherry picked from commit 556fc646cfac4dca43a34f5c4a4f7e6e3ef9104d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/kernel/qobject.cpp | 19 ++++++++++++++++---
 src/corelib/kernel/qobject_p.h | 13 ++++++++++---
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index ea706b93a04..33afcaaa7ef 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -70,6 +70,7 @@
 #include <qtcore_tracepoints_p.h>
 
 #include <new>
+#include <mutex>
 
 #include <ctype.h>
 #include <limits.h>
@@ -405,11 +406,14 @@ void QObjectPrivate::ConnectionData::removeConnection(QObjectPrivate::Connection
 
 }
 
-void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender)
+void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy)
 {
+    QBasicMutex *senderMutex = signalSlotLock(sender);
     ConnectionOrSignalVector *c = nullptr;
     {
-        QBasicMutexLocker l(signalSlotLock(sender));
+        std::unique_lock<QBasicMutex> lock(*senderMutex, std::defer_lock_t{});
+        if (lockPolicy == NeedToLock)
+            lock.lock();
         if (ref.loadAcquire() > 1)
             return;
 
@@ -419,7 +423,16 @@ void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sende
         c = orphaned.loadRelaxed();
         orphaned.storeRelaxed(nullptr);
     }
-    deleteOrphaned(c);
+    if (c) {
+        // Deleting c might run arbitrary user code, so we must not hold the lock
+        if (lockPolicy == AlreadyLockedAndTemporarilyReleasingLock) {
+            senderMutex->unlock();
+            deleteOrphaned(c);
+            senderMutex->lock();
+        } else {
+            deleteOrphaned(c);
+        }
+    }
 }
 
 void QObjectPrivate::ConnectionData::deleteOrphaned(QObjectPrivate::ConnectionOrSignalVector *o)
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 4483e2b6ce7..8dd29e05cea 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -266,12 +266,19 @@ public:
         // must be called on the senders connection data
         // assumes the senders and receivers lock are held
         void removeConnection(Connection *c);
-        void cleanOrphanedConnections(QObject *sender)
+        enum LockPolicy {
+            NeedToLock,
+            // Beware that we need to temporarily release the lock
+            // and thus calling code must carefully consider whether
+            // invariants still hold.
+            AlreadyLockedAndTemporarilyReleasingLock
+        };
+        void cleanOrphanedConnections(QObject *sender, LockPolicy lockPolicy = NeedToLock)
         {
             if (orphaned.loadRelaxed() && ref.loadAcquire() == 1)
-                cleanOrphanedConnectionsImpl(sender);
+                cleanOrphanedConnectionsImpl(sender, lockPolicy);
         }
-        void cleanOrphanedConnectionsImpl(QObject *sender);
+        void cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy);
 
         ConnectionList &connectionsForSignal(int signal)
         {