Revert "Optimize QMetaObject::activate."

Andre reported issues to me which seem to arise from this patch.
Most notably, he showed me a backtrace containing the following
assertions. Note how the dtor of ConnectionListsRef is
accessing shared data which used to be guarded by the connection
lock, which got broken by my patch that is now reverted hereby.

A future patch can potentially reintroduce my performance
optimization but that will require more evaluation and better
checks with TSan or helgrind. Sorry for this.

Thread 7 (Thread 0x7fffc7fff700 (LWP 32705)):
.0  0x00007ffff55ef267 in raise () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
.1  0x00007ffff55f0eca in abort () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
.2  0x00007ffff62286d7 in qt_message_fatal (context=..., message=...) at
global/qlogging.cpp:1647
No locals.
.3  0x00007ffff622472a in QMessageLogger::fatal (this=0x7fffc7ff8650,
msg=0x7ffff658c9d0 "ASSERT: \"%s\" in file %s, line %d") at global/
qlogging.cpp:792
        message = {static null = {<No data fields>}, d = 0x7fffc14f5050}
        ap = {{gp_offset = 40, fp_offset = 48, overflow_arg_area =
0x7fffc7ff8630, reg_save_area = 0x7fffc7ff8570}}
.4  0x00007ffff621cf43 in qt_assert (assertion=0x7ffff6657ba8
"connectionLists->inUse >= 0", file=0x7ffff6656fa7 "kernel/qobject.cpp",
line=3646) at global/qglobal.cpp:3036
        __PRETTY_FUNCTION__ = "void qt_assert(const char*, const char*,
int)"
.5  0x00007ffff64db388 in
QMetaObject::ConnectionListsRef::~ConnectionListsRef
(this=0x7fffc7ff8710, __in_chrg=<optimized out>) at kernel/qobject.cpp:3646
No locals.
.6  0x00007ffff64dbe71 in QMetaObject::activate (sender=0x9d2110,
signalOffset=3, local_signal_index=1, argv=0x7fffc7ff8850) at kernel/
qobject.cpp:3685
        locker = {val = 140737328754040}
        connectionLists = {connectionLists = 0xa9c460}
        lists = {0xa9c4e8, 0x7ffff64a0ceb
<QMetaObjectPrivate::signalOffset(QMetaObject const*)+89>}
        numLists = 1
        currentThreadId = 0x7fffc7fff700
        signal_index = 4
        empty_argv = {0x0}
        __PRETTY_FUNCTION__ = "static void
QMetaObject::activate(QObject*, int, int, void**)"
.7  0x00007ffff64db300 in QMetaObject::activate (sender=0x9d2110,
m=0x7fffe2d66a80 <CppTools::CppModelManager::staticMetaObject>,
local_signal_index=1, argv=0x7fffc7ff8850) at kernel/qobject.cpp:3602
No locals.

<snip>

Thread 4 (Thread 0x7fffc77fe700 (LWP 32712)):
.0  <function called from gdb>
No symbol table info available.
.1  0x00007ffff55ef267 in raise () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
.2  0x00007ffff55f0eca in abort () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
.3  0x00007ffff62286d7 in qt_message_fatal (context=..., message=...) at
global/qlogging.cpp:1647
No locals.
.4  0x00007ffff622472a in QMessageLogger::fatal (this=0x7fffc77f7500,
msg=0x7ffff658c9d0 "ASSERT: \"%s\" in file %s, line %d") at global/
qlogging.cpp:792
        message = {static null = {<No data fields>}, d = 0x7fffb94f14b0}
        ap = {{gp_offset = 40, fp_offset = 48, overflow_arg_area =
0x7fffc77f74e0, reg_save_area = 0x7fffc77f7420}}
.5  0x00007ffff621cf43 in qt_assert (assertion=0x7ffff6657ba8
"connectionLists->inUse >= 0", file=0x7ffff6656fa7 "kernel/qobject.cpp",
line=3646) at global/qglobal.cpp:3036
        __PRETTY_FUNCTION__ = "void qt_assert(const char*, const char*,
int)"
.6  0x00007ffff64db388 in
QMetaObject::ConnectionListsRef::~ConnectionListsRef
(this=0x7fffc77f75c0, __in_chrg=<optimized out>) at kernel/qobject.cpp:3646
No locals.
.7  0x00007ffff64dbe71 in QMetaObject::activate (sender=0x9d2110,
signalOffset=3, local_signal_index=1, argv=0x7fffc77f7700) at kernel/
qobject.cpp:3685
        locker = {val = 140737328754040}
        connectionLists = {connectionLists = 0xa9c460}
        lists = {0xa9c4e8, 0x7ffff64a0ceb
<QMetaObjectPrivate::signalOffset(QMetaObject const*)+89>}
        numLists = 1
        currentThreadId = 0x7fffc77fe700
        signal_index = 4
        empty_argv = {0x0}
        __PRETTY_FUNCTION__ = "static void
QMetaObject::activate(QObject*, int, int, void**)"
.8  0x00007ffff64db300 in QMetaObject::activate (sender=0x9d2110,
m=0x7fffe2d66a80 <CppTools::CppModelManager::staticMetaObject>,
local_signal_index=1, argv=0x7fffc77f7700) at kernel/qobject.cpp:3602
No locals.
.9  0x00007fffe2a7aa0a in CppTools::CppModelManager::documentUpdated
(this=0x9d2110, _t1=...) at .moc/debug-shared/moc_cppmodelmanager.cpp:299
        _a = {0x0, 0x7fffc77f7740}

This reverts commit 8619214c5e76c70e32b47cd002be1adb1bc2f5bf.

Change-Id: I13df84012e74a01db750a99a8e5e4bf5357c7f78
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>

1 files changed, 19 insertions, 30 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 0bc0916b39..fc5e6abf00 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -3660,33 +3660,17 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
         return;
     }
 
-    // contains the non-empty connection lists
-    const QObjectPrivate::ConnectionList *lists[2];
-    int numLists = 0;
-    if (signal_index < connectionLists->count()) {
-        const auto *list = &connectionLists->at(signal_index);
-        if (list->first) // only add if non-empty
-            lists[numLists++] = list;
-    }
-    if (connectionLists->allsignals.first) // only add if non-empty
-        lists[numLists++] = &connectionLists->allsignals;
+    const QObjectPrivate::ConnectionList *list;
+    if (signal_index < connectionLists->count())
+        list = &connectionLists->at(signal_index);
+    else
+        list = &connectionLists->allsignals;
 
     Qt::HANDLE currentThreadId = QThread::currentThreadId();
 
-    for (int i = 0; i < numLists; ++i) {
-        const auto *list = lists[i];
-        if (i == 0) {
-            // on the first iteration, the mutex must be locked already
-            Q_ASSERT(!locker.mutex()->tryLock());
-        } else {
-            // otherwise the mutex is unlocked and must be relocked
-            locker.relock();
-            if (connectionLists->orphaned)
-                break;
-        }
-
+    do {
         QObjectPrivate::Connection *c = list->first;
-        Q_ASSERT(c);
+        if (!c) continue;
         // We need to check against last here to ensure that signals added
         // during the signal emission are not emitted in this emission.
         QObjectPrivate::Connection *last = list->last;
@@ -3739,6 +3723,8 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
                 // destructor of the slot object might also lock a mutex from the signalSlotLock() mutex pool,
                 // and that would deadlock if the pool happens to return the same mutex.
                 obj.reset();
+
+                locker.relock();
             } else if (c->callFunction && c->method_offset <= receiver->metaObject()->methodOffset()) {
                 //we compare the vtable to make sure we are not in the destructor of the object.
                 const int methodIndex = c->method();
@@ -3752,6 +3738,7 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
 
                 if (qt_signal_spy_callback_set.slot_end_callback != 0)
                     qt_signal_spy_callback_set.slot_end_callback(receiver, methodIndex);
+                locker.relock();
             } else {
                 const int method = c->method_relative + c->method_offset;
                 locker.unlock();
@@ -3766,17 +3753,19 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
 
                 if (qt_signal_spy_callback_set.slot_end_callback != 0)
                     qt_signal_spy_callback_set.slot_end_callback(receiver, method);
-            }
 
-            if (c == last) // early break without relock for the last signal
-                break;
-
-            locker.relock();
+                locker.relock();
+            }
 
             if (connectionLists->orphaned)
                 break;
-        } while ((c = c->nextConnectionList) != 0);
-    }
+        } while (c != last && (c = c->nextConnectionList) != 0);
+
+        if (connectionLists->orphaned)
+            break;
+    } while (list != &connectionLists->allsignals &&
+        //start over for all signals;
+        ((list = &connectionLists->allsignals), true));
 
     }