From 13ab090977439cf432c7b99dbdd2b1263b4d8cd4 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Wed, 9 Jan 2019 22:17:51 +0100 Subject: [PATCH] Add safe way to resize the signalVector Change-Id: Ib55da020f22e981bc379af3b4cf3431bf0fa0c20 Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qobject.cpp | 92 +++++++++++++++++++--------------- src/corelib/kernel/qobject_p.h | 85 +++++++++++++++++++++++++------ 2 files changed, 122 insertions(+), 55 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index a0e4450d28d..158d92799c1 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/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()) { - const QObjectPrivate::Connection *c = cd->signalVector.at(signal_index).first; + if (signal_index < cd->signalVectorCount()) { + 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()) { - const QObjectPrivate::Connection *c = cd->signalVector.at(signal_index).first; + if (signal_index < cd->signalVectorCount()) { + 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->signalVector.resize(signal + 1); + cd->resizeSignalVector(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) { - Q_ASSERT(!c->receiver); - Q_ASSERT(!c->prev); - QObjectPrivate::Connection *next = c->nextInOrphanList; - c->freeSlotObject(); - c->deref(); - c = next; + deleteOrphaned(c); +} + +void QObjectPrivate::ConnectionData::deleteOrphaned(QObjectPrivate::ConnectionOrSignalVector *o) +{ + while (o) { + QObjectPrivate::ConnectionOrSignalVector *next = nullptr; + if (SignalVector *v = ConnectionOrSignalVector::asSignalVector(o)) { + next = v->nextInOrphanList; + free(v); + } else { + QObjectPrivate::Connection *c = static_cast(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())) { - const QObjectPrivate::Connection *c = cd->signalVector.at(signalIndex).first; + if (signalIndex < uint(cd->signalVectorCount())) { + 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())) { - const QObjectPrivate::Connection *c = cd->signalVector.at(signalIndex).first; + if (signalIndex < uint(cd->signalVectorCount())) { + 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()) { - const QObjectPrivate::Connection *c = - cd->signalVector.at(signal_index).first; + if (cd && signal_index < cd->signalVectorCount()) { + const QObjectPrivate::Connection *c = 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) { - const QObjectPrivate::Connection *c2 = scd->signalVector.at(signal_index).first; + if (scd->signalVectorCount() > signal_index) { + 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()) - list = &connections->signalVector.at(signal_index); + if (signal_index < connections->signalVector->count()) + 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()) { - for (int signal_index = 0; signal_index < cd->signalVector.count(); ++signal_index) { + if (cd && cd->signalVectorCount()) { + 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(" "); @@ -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) { - const QObjectPrivate::Connection *c2 = connections->signalVector.at(signal_index).first; + if (connections->signalVectorCount() > signal_index) { + const QObjectPrivate::Connection *c2 = connections->signalVector->at(signal_index).first; while (c2) { if (c2->receiver == receiver && c2->isSlotObject && c2->slotObj->compare(slot)) { diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index da3d0350879..f01b709faaa 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/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(c) & 1) + return reinterpret_cast(reinterpret_cast(c) & ~quintptr(1u)); + return nullptr; + } + static Connection *fromSignalVector(SignalVector *v) { + return reinterpret_cast(reinterpret_cast(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(this + 1)[i + 1]; + } + const ConnectionList &at(int i) const + { + return reinterpret_cast(this + 1)[i + 1]; + } + int count() { return static_cast(allocated); } + }; + + + /* This contains the all connections from and to an object. @@ -236,22 +268,16 @@ public: }; Ref ref; - ConnectionList allsignals; - QVector signalVector; + SignalVector *signalVector = nullptr; Connection *senders = nullptr; Sender *currentSender = nullptr; // object currently activating the object QAtomicPointer orphaned; ~ConnectionData() { - Connection *c = orphaned.load(); - while (c) { - Q_ASSERT(!c->receiver); - QObjectPrivate::Connection *next = c->nextInOrphanList; - c->freeSlotObject(); - c->deref(); - c = next; - } + deleteOrphaned(orphaned.load()); + if (signalVector) + free(signalVector); } // 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(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);