Add safe way to resize the signalVector

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

src/corelib/kernel/qobject.cpp

@@ -265,8 +265,8 @@ bool QObjectPrivate::isSender(const QObject *receiver, const char *signal) const
     if (signal_index < 0 || !cd)
         return false;
     QBasicMutexLocker locker(signalSlotLock(q));
-    if (signal_index < cd->signalVector.count()) {
+    if (signal_index < cd->signalVectorCount()) {
-        const QObjectPrivate::Connection *c = cd->signalVector.at(signal_index).first;
+        const QObjectPrivate::Connection *c = cd->signalVector->at(signal_index).first;
 
         while (c) {
             if (c->receiver == receiver)
@@ -287,8 +287,8 @@ QObjectList QObjectPrivate::receiverList(const char *signal) const
     if (signal_index < 0 || !cd)
         return returnValue;
     QBasicMutexLocker locker(signalSlotLock(q));
-    if (signal_index < cd->signalVector.count()) {
+    if (signal_index < cd->signalVectorCount()) {
-        const QObjectPrivate::Connection *c = cd->signalVector.at(signal_index).first;
+        const QObjectPrivate::Connection *c = cd->signalVector->at(signal_index).first;
 
         while (c) {
             if (c->receiver)
@@ -327,8 +327,7 @@ void QObjectPrivate::addConnection(int signal, Connection *c)
     Q_ASSERT(c->sender == q_ptr);
     ensureConnectionData();
     ConnectionData *cd = connections.load();
-    if (signal >= cd->signalVector.count())
+    cd->resizeSignalVector(signal + 1);
-        cd->signalVector.resize(signal + 1);
 
     ConnectionList &connectionList = cd->connectionsForSignal(signal);
     if (connectionList.last) {
@@ -353,7 +352,7 @@ void QObjectPrivate::addConnection(int signal, Connection *c)
 
 void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender)
 {
-    Connection *c = nullptr;
+    ConnectionOrSignalVector *c = nullptr;
     {
         QBasicMutexLocker l(signalSlotLock(sender));
         if (ref > 1)
@@ -365,13 +364,25 @@ void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sende
         c = orphaned.load();
         orphaned.store(nullptr);
     }
-    while (c) {
+    deleteOrphaned(c);
-        Q_ASSERT(!c->receiver);
+}
-        Q_ASSERT(!c->prev);
+
-        QObjectPrivate::Connection *next = c->nextInOrphanList;
+void QObjectPrivate::ConnectionData::deleteOrphaned(QObjectPrivate::ConnectionOrSignalVector *o)
-        c->freeSlotObject();
+{
-        c->deref();
+    while (o) {
-        c = next;
+        QObjectPrivate::ConnectionOrSignalVector *next = nullptr;
+        if (SignalVector *v = ConnectionOrSignalVector::asSignalVector(o)) {
+            next = v->nextInOrphanList;
+            free(v);
+        } else {
+            QObjectPrivate::Connection *c = static_cast<Connection *>(o);
+            next = c->nextInOrphanList;
+            Q_ASSERT(!c->receiver);
+            Q_ASSERT(!c->prev);
+            c->freeSlotObject();
+            c->deref();
+        }
+        o = next;
     }
 }
 
@@ -387,14 +398,14 @@ bool QObjectPrivate::isSignalConnected(uint signalIndex, bool checkDeclarative)
         return true;
 
     ConnectionData *cd = connections.load();
-    if (!cd)
+    if (!cd || !cd->signalVector)
         return false;
 
-    if (cd->allsignals.first)
+    if (cd->signalVector->at(-1).first)
         return true;
 
-    if (signalIndex < uint(cd->signalVector.count())) {
+    if (signalIndex < uint(cd->signalVectorCount())) {
-        const QObjectPrivate::Connection *c = cd->signalVector.at(signalIndex).first;
+        const QObjectPrivate::Connection *c = cd->signalVector->at(signalIndex).first;
         while (c) {
             if (c->receiver)
                 return true;
@@ -407,14 +418,14 @@ bool QObjectPrivate::isSignalConnected(uint signalIndex, bool checkDeclarative)
 bool QObjectPrivate::maybeSignalConnected(uint signalIndex) const
 {
     ConnectionData *cd = connections.load();
-    if (!cd)
+    if (!cd || !cd->signalVector)
         return false;
 
-    if (cd->allsignals.first)
+    if (cd->signalVector->at(-1).first)
         return true;
 
-    if (signalIndex < uint(cd->signalVector.count())) {
+    if (signalIndex < uint(cd->signalVectorCount())) {
-        const QObjectPrivate::Connection *c = cd->signalVector.at(signalIndex).first;
+        const QObjectPrivate::Connection *c = cd->signalVector->at(signalIndex).first;
         return c != nullptr;
     }
     return false;
@@ -895,7 +906,7 @@ QObject::~QObject()
         QBasicMutexLocker locker(signalSlotMutex);
 
         // disconnect all receivers
-        int receiverCount = cd->signalVector.count();
+        int receiverCount = cd->signalVectorCount();
         for (int signal = -1; signal < receiverCount; ++signal) {
             QObjectPrivate::ConnectionList &connectionList = cd->connectionsForSignal(signal);
 
@@ -2416,9 +2427,8 @@ int QObject::receivers(const char *signal) const
 
         QObjectPrivate::ConnectionData *cd = d->connections.load();
         QBasicMutexLocker locker(signalSlotLock(this));
-        if (cd && signal_index < cd->signalVector.count()) {
+        if (cd && signal_index < cd->signalVectorCount()) {
-            const QObjectPrivate::Connection *c =
+            const QObjectPrivate::Connection *c = cd->signalVector->at(signal_index).first;
-                cd->signalVector.at(signal_index).first;
             while (c) {
                 receivers += c->receiver ? 1 : 0;
                 c = c->nextConnectionList;
@@ -3232,8 +3242,8 @@ QObjectPrivate::Connection *QMetaObjectPrivate::connect(const QObject *sender,
 
     QObjectPrivate::ConnectionData *scd  = QObjectPrivate::get(s)->connections.load();
     if (type & Qt::UniqueConnection && scd) {
-        if (scd->signalVector.count() > signal_index) {
+        if (scd->signalVectorCount() > signal_index) {
-            const QObjectPrivate::Connection *c2 = scd->signalVector.at(signal_index).first;
+            const QObjectPrivate::Connection *c2 = scd->signalVector->at(signal_index).first;
 
             int method_index_absolute = method_index + method_offset;
 
@@ -3365,11 +3375,11 @@ bool QMetaObjectPrivate::disconnect(const QObject *sender,
 
         if (signal_index < 0) {
             // remove from all connection lists
-            for (int sig_index = -1; sig_index < scd->signalVector.count(); ++sig_index) {
+            for (int sig_index = -1; sig_index < scd->signalVectorCount(); ++sig_index) {
                 if (disconnectHelper(connections.data(), sig_index, receiver, method_index, slot, senderMutex, disconnectType))
                     success = true;
             }
-        } else if (signal_index < scd->signalVector.count()) {
+        } else if (signal_index < scd->signalVectorCount()) {
             if (disconnectHelper(connections.data(), signal_index, receiver, method_index, slot, senderMutex, disconnectType))
                 success = true;
         }
@@ -3592,10 +3602,10 @@ void doActivate(QObject *sender, int signal_index, void **argv)
     QObjectPrivate::ConnectionDataPointer connections(sp->connections.load());
 
     const QObjectPrivate::ConnectionList *list;
-    if (signal_index < connections->signalVector.count())
+    if (signal_index < connections->signalVector->count())
-        list = &connections->signalVector.at(signal_index);
+        list = &connections->signalVector->at(signal_index);
     else
-        list = &connections->allsignals;
+        list = &connections->signalVector->at(-1);
 
     Qt::HANDLE currentThreadId = QThread::currentThreadId();
 
@@ -3691,9 +3701,9 @@ void doActivate(QObject *sender, int signal_index, void **argv)
             }
         } while ((c = c->nextConnectionList) != 0 && c->id <= highestConnectionId);
 
-    } while (list != &connections->allsignals &&
+    } while (list != &connections->signalVector->at(-1) &&
         //start over for all signals;
-        ((list = &connections->allsignals), true));
+        ((list = &connections->signalVector->at(-1)), true));
 
         if (connections->currentConnectionId.load() == 0)
             senderDeleted = true;
@@ -3994,13 +4004,15 @@ void QObject::dumpObjectInfo() const
     qDebug("  SIGNALS OUT");
 
     QObjectPrivate::ConnectionData *cd = d->connections.load();
-    if (cd && cd->signalVector.count()) {
+    if (cd && cd->signalVectorCount()) {
-        for (int signal_index = 0; signal_index < cd->signalVector.count(); ++signal_index) {
+        for (int signal_index = 0; signal_index < cd->signalVectorCount(); ++signal_index) {
+            const QObjectPrivate::Connection *c = cd->signalVector->at(signal_index).first;
+            if (!c)
+                continue;
             const QMetaMethod signal = QMetaObjectPrivate::signal(metaObject(), signal_index);
             qDebug("        signal: %s", signal.methodSignature().constData());
 
             // receivers
-            const QObjectPrivate::Connection *c = cd->signalVector.at(signal_index).first;
             while (c) {
                 if (!c->receiver) {
                     qDebug("          <Disconnected receiver>");
@@ -4766,8 +4778,8 @@ QMetaObject::Connection QObjectPrivate::connectImpl(const QObject *sender, int s
 
     if (type & Qt::UniqueConnection && slot && QObjectPrivate::get(s)->connections.load()) {
         QObjectPrivate::ConnectionData *connections = QObjectPrivate::get(s)->connections.load();
-        if (connections->signalVector.count() > signal_index) {
+        if (connections->signalVectorCount() > signal_index) {
-            const QObjectPrivate::Connection *c2 = connections->signalVector.at(signal_index).first;
+            const QObjectPrivate::Connection *c2 = connections->signalVector->at(signal_index).first;
 
             while (c2) {
                 if (c2->receiver == receiver && c2->isSlotObject && c2->slotObj->compare(slot)) {

src/corelib/kernel/qobject_p.h

@@ -124,14 +124,30 @@ public:
     };
 
     typedef void (*StaticMetaCallFunction)(QObject *, QMetaObject::Call, int, void **);
-    struct Connection
+    struct Connection;
-    {
+    struct SignalVector;
+
+    struct ConnectionOrSignalVector {
         union {
             // linked list of orphaned connections that need cleaning up
-            Connection *nextInOrphanList;
+            ConnectionOrSignalVector *nextInOrphanList;
             // linked list of connections connected to slots in this object
             Connection *next;
         };
+
+        static SignalVector *asSignalVector(ConnectionOrSignalVector *c) {
+            if (reinterpret_cast<quintptr>(c) & 1)
+                return reinterpret_cast<SignalVector *>(reinterpret_cast<quintptr>(c) & ~quintptr(1u));
+            return nullptr;
+        }
+        static Connection *fromSignalVector(SignalVector *v) {
+            return reinterpret_cast<Connection *>(reinterpret_cast<quintptr>(v) | quintptr(1u));
+        }
+    };
+
+    struct Connection : public ConnectionOrSignalVector
+    {
+        // linked list of connections connected to slots in this object, next is in base class
         Connection **prev;
         // linked list of connections connected to signals in this object
         Connection *nextConnectionList;
@@ -210,6 +226,22 @@ public:
         int signal;
     };
 
+    struct SignalVector : public ConnectionOrSignalVector {
+        quintptr allocated;
+        // ConnectionList signals[]
+        ConnectionList &at(int i)
+        {
+            return reinterpret_cast<ConnectionList *>(this + 1)[i + 1];
+        }
+        const ConnectionList &at(int i) const
+        {
+            return reinterpret_cast<const ConnectionList *>(this + 1)[i + 1];
+        }
+        int count() { return static_cast<int>(allocated); }
+    };
+
+
+
     /*
         This contains the all connections from and to an object.
 
@@ -236,22 +268,16 @@ public:
         };
 
         Ref ref;
-        ConnectionList allsignals;
+        SignalVector *signalVector = nullptr;
-        QVector<ConnectionList> signalVector;
         Connection *senders = nullptr;
         Sender *currentSender = nullptr;   // object currently activating the object
         QAtomicPointer<Connection> orphaned;
 
         ~ConnectionData()
         {
-            Connection *c = orphaned.load();
+            deleteOrphaned(orphaned.load());
-            while (c) {
+            if (signalVector)
-                Q_ASSERT(!c->receiver);
+                free(signalVector);
-                QObjectPrivate::Connection *next = c->nextInOrphanList;
-                c->freeSlotObject();
-                c->deref();
-                c = next;
-            }
         }
 
         // must be called on the senders connection data
@@ -259,7 +285,7 @@ public:
         void removeConnection(Connection *c)
         {
             Q_ASSERT(c->receiver);
-            ConnectionList &connections = connectionsForSignal(c->signal_index);
+            ConnectionList &connections = signalVector->at(c->signal_index);
             c->receiver = nullptr;
 
 #ifndef QT_NO_DEBUG
@@ -283,6 +309,8 @@ public:
                 connections.first = c->nextConnectionList;
             if (connections.last == c)
                 connections.last = c->prevConnectionList;
+            Q_ASSERT(signalVector->at(c->signal_index).first != c);
+            Q_ASSERT(signalVector->at(c->signal_index).last != c);
 
             // keep c->nextConnectionList intact, as it might still get accessed by activate
             if (c->nextConnectionList)
@@ -317,8 +345,35 @@ public:
 
         ConnectionList &connectionsForSignal(int signal)
         {
-            return signal < 0 ? allsignals : signalVector[signal];
+            return signalVector->at(signal);
         }
+
+        void resizeSignalVector(uint size) {
+            if (signalVector && signalVector->allocated > size)
+                return;
+            size = (size + 7) & ~7;
+            SignalVector *v = reinterpret_cast<SignalVector *>(malloc(sizeof(SignalVector) + (size + 1) * sizeof(ConnectionList)));
+            int start = -1;
+            if (signalVector) {
+                memcpy(v, signalVector, sizeof(SignalVector) + (signalVector->allocated + 1) * sizeof(ConnectionList));
+                start = signalVector->count();
+            }
+            for (int i = start; i < int(size); ++i)
+                v->at(i) = ConnectionList();
+            v->next = nullptr;
+            v->allocated = size;
+
+            qSwap(v, signalVector);
+            if (v) {
+                v->next = orphaned.load();
+                orphaned.store(ConnectionOrSignalVector::fromSignalVector(v));
+            }
+        }
+        int signalVectorCount() const {
+            return  signalVector ? signalVector->count() : -1;
+        }
+
+        static void deleteOrphaned(ConnectionOrSignalVector *c);
     };
 
     QObjectPrivate(int version = QObjectPrivateVersion);