Replace the ConnectionData::inUse int with a proper refcount

The main difference is that QObject itself also holds on reference
on the structure.

Also rename the orphaned flag to objectDeleted for clarity.

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

2 files changed, 38 insertions, 56 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index b8e973a0a3..c3271b2c35 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -354,7 +354,7 @@ void QObjectPrivate::addConnection(int signal, Connection *c)
 void QObjectPrivate::cleanConnectionLists()
 {
     ConnectionData *cd = connections.load();
-    if (cd->dirty && !cd->inUse) {
+    if (cd->dirty && cd->ref == 1) {
         // remove broken connections
         for (int signal = -1; signal < cd->signalVector.count(); ++signal) {
             ConnectionList &connectionList = cd->connectionsForSignal(signal);
@@ -883,7 +883,6 @@ QObject::~QObject()
 
         QMutex *signalSlotMutex = signalSlotLock(this);
         QMutexLocker locker(signalSlotMutex);
-        ++cd->inUse;
 
         // disconnect all receivers
         int receiverCount = cd->signalVector.count();
@@ -949,7 +948,7 @@ QObject::~QObject()
                 continue;
             }
             node->receiver = 0;
-            QObjectPrivate::ConnectionData *senderData = sender->d_func()->connections;
+            QObjectPrivate::ConnectionData *senderData = sender->d_func()->connections.load();
             if (senderData)
                 senderData->dirty = true;
 
@@ -972,13 +971,11 @@ QObject::~QObject()
             }
         }
 
-        if (!--cd->inUse) {
-            delete cd;
-        } else {
-            cd->orphaned = true;
-        }
-        d->connections.store(nullptr);
+        cd->objectDeleted = true;
     }
+    if (cd && !cd->ref.deref())
+        delete cd;
+    d->connections.store(nullptr);
 
     if (!d->children.isEmpty())
         d->deleteChildren();
@@ -3385,32 +3382,29 @@ bool QMetaObjectPrivate::disconnect(const QObject *sender,
     if (!scd)
         return false;
 
-    // prevent incoming connections changing the connections->receivers while unlocked
-    ++scd->inUse;
-
     bool success = false;
-    if (signal_index < 0) {
-        // remove from all connection lists
-        for (int sig_index = -1; sig_index < scd->signalVector.count(); ++sig_index) {
-            QObjectPrivate::Connection *c = scd->connectionsForSignal(sig_index).first;
+    {
+        // prevent incoming connections changing the connections->receivers while unlocked
+        QObjectPrivate::ConnectionDataPointer connections(scd);
+
+        if (signal_index < 0) {
+            // remove from all connection lists
+            for (int sig_index = -1; sig_index < scd->signalVector.count(); ++sig_index) {
+                QObjectPrivate::Connection *c = scd->connectionsForSignal(sig_index).first;
+                if (disconnectHelper(c, receiver, method_index, slot, senderMutex, disconnectType)) {
+                    success = true;
+                    scd->dirty = true;
+                }
+            }
+        } else if (signal_index < scd->signalVector.count()) {
+            QObjectPrivate::Connection *c = scd->signalVector.at(signal_index).first;
             if (disconnectHelper(c, receiver, method_index, slot, senderMutex, disconnectType)) {
                 success = true;
                 scd->dirty = true;
             }
         }
-    } else if (signal_index < scd->signalVector.count()) {
-        QObjectPrivate::Connection *c = scd->signalVector.at(signal_index).first;
-        if (disconnectHelper(c, receiver, method_index, slot, senderMutex, disconnectType)) {
-            success = true;
-            scd->dirty = true;
-        }
     }
 
-    --scd->inUse;
-    Q_ASSERT(scd->inUse >= 0);
-    if (scd->orphaned && !scd->inUse)
-        delete scd;
-
     locker.unlock();
     if (success) {
         QMetaMethod smethod = QMetaObjectPrivate::signal(smeta, signal_index);
@@ -3621,30 +3615,8 @@ void doActivate(QObject *sender, int signal_index, void **argv)
 
     {
     QMutexLocker locker(signalSlotLock(sender));
-    struct ConnectionDataRef {
-        QObjectPrivate::ConnectionData *connections;
-        ConnectionDataRef(QObjectPrivate::ConnectionData *connections) : connections(connections)
-        {
-            if (connections)
-                ++connections->inUse;
-        }
-        ~ConnectionDataRef()
-        {
-            if (!connections)
-                return;
-
-            --connections->inUse;
-            Q_ASSERT(connections->inUse >= 0);
-            if (connections->orphaned) {
-                if (!connections->inUse)
-                    delete connections;
-            }
-        }
-
-        QObjectPrivate::ConnectionData *operator->() const { return connections; }
-    };
-    Q_ASSERT(sp->connections.load());
-    ConnectionDataRef connections = sp->connections.load();
+    Q_ASSERT(sp->connections);
+    QObjectPrivate::ConnectionDataPointer connections(sp->connections.load());
 
     const QObjectPrivate::ConnectionList *list;
     if (signal_index < connections->signalVector.count())
@@ -3744,11 +3716,11 @@ void doActivate(QObject *sender, int signal_index, void **argv)
                 locker.relock();
             }
 
-            if (connections->orphaned)
+            if (connections->objectDeleted)
                 break;
         } while (c != last && (c = c->nextConnectionList) != 0);
 
-        if (connections->orphaned)
+        if (connections->objectDeleted)
             break;
     } while (list != &connections->allsignals &&
         //start over for all signals;
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 64998797ac..823c7a195a 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -210,9 +210,16 @@ public:
         linked list.
     */
     struct ConnectionData {
-        bool orphaned = false; //the QObject owner of this vector has been destroyed while the vector was inUse
+        bool objectDeleted = false; //the QObject owner of this vector has been destroyed while the vector was inUse
+        struct Ref {
+            int _ref = 0;
+            void ref() { ++_ref; }
+            int deref() { return --_ref; }
+            operator int() const { return _ref; }
+        };
+
+        Ref ref;
         bool dirty = false; //some Connection have been disconnected (their receiver is 0) but not removed from the list yet
-        int inUse = 0; //number of functions that are currently accessing this object or its connections
         ConnectionList allsignals;
         QVector<ConnectionList> signalVector;
         Connection *senders = nullptr;
@@ -274,12 +281,15 @@ public:
     {
         if (connections.load())
             return;
-        connections.store(new ConnectionData);
+        ConnectionData *cd = new ConnectionData;
+        cd->ref.ref();
+        connections.store(cd);
     }
 public:
     ExtraData *extraData;    // extra data set by the user
     QThreadData *threadData; // id of the thread that owns the object
 
+    using ConnectionDataPointer = QExplicitlySharedDataPointer<ConnectionData>;
     QAtomicPointer<ConnectionData> connections;
 
     union {