From 0e534bf8b704b8ae24b87a9b1dcb5cd829c9dd2f Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Wed, 9 Jan 2019 09:48:59 +0100 Subject: 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) --- src/corelib/kernel/qobject.cpp | 78 ++++++++++++++---------------------------- src/corelib/kernel/qobject_p.h | 16 +++++++-- 2 files changed, 38 insertions(+), 56 deletions(-) (limited to 'src/corelib/kernel') 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 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; QAtomicPointer connections; union { -- cgit v1.2.3