From 6e0b5dadc7e91be786411809f1f9667c239168e2 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Wed, 9 Jan 2019 19:12:46 +0100 Subject: Change cleanup mechanism for orphaned connections Put all connections that get disconnected into a singly linked orphaned list. Whenever the refcount on the connectionData drops down to one, this list can safely be cleared, even with the planned removal of locking in activate(). Use an id integer in the connection to acoid activating newly added connections. Fixes: QTBUG-72649 Change-Id: Ide3d116ae7fc9ca497598c1c2b71d43b4339c92d Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qobject_p.h | 109 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 99 insertions(+), 10 deletions(-) (limited to 'src/corelib/kernel/qobject_p.h') diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index 863689fddd..2c1502c2a0 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2017 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Copyright (C) 2013 Olivier Goffart ** Contact: https://www.qt.io/licensing/ ** @@ -126,22 +126,29 @@ public: typedef void (*StaticMetaCallFunction)(QObject *, QMetaObject::Call, int, void **); struct Connection { + union { + // linked list of orphaned connections that need cleaning up + Connection *nextInOrphanList; + // linked list of connections connected to slots in this object + Connection *next; + }; + Connection **prev; + // linked list of connections connected to signals in this object + Connection *nextConnectionList; + Connection *prevConnectionList; + QObject *sender; QObject *receiver; union { StaticMetaCallFunction callFunction; QtPrivate::QSlotObjectBase *slotObj; }; - // The next pointer for the singly-linked ConnectionList - Connection *nextConnectionList; - //senders linked list - Connection *next; - Connection **prev; QAtomicPointer argumentTypes; QAtomicInt ref_; + uint id = 0; ushort method_offset; ushort method_relative; - uint signal_index : 27; // In signal range (see QObjectPrivate::signalIndex()) + int signal_index : 27; // In signal range (see QObjectPrivate::signalIndex()) ushort connectionType : 3; // 0 == auto, 1 == direct, 2 == queued, 4 == blocking ushort isSlotObject : 1; ushort ownArgumentTypes : 1; @@ -151,9 +158,17 @@ public: ~Connection(); int method() const { Q_ASSERT(!isSlotObject); return method_offset + method_relative; } void ref() { ref_.ref(); } + void freeSlotObject() + { + if (isSlotObject) { + slotObj->destroyIfLastRef(); + isSlotObject = false; + } + } void deref() { if (!ref_.deref()) { Q_ASSERT(!receiver); + Q_ASSERT(!isSlotObject); delete this; } } @@ -210,7 +225,9 @@ public: linked list. */ struct ConnectionData { - bool objectDeleted = false; //the QObject owner of this vector has been destroyed while the vector was inUse + // the id below is used to avoid activating new connections. When the object gets + // deleted it's set to 0, so that signal emission stops + QAtomicInteger currentConnectionId; struct Ref { int _ref = 0; void ref() { ++_ref; } @@ -219,11 +236,84 @@ public: }; Ref ref; - bool dirty = false; //some Connection have been disconnected (their receiver is 0) but not removed from the list yet ConnectionList allsignals; QVector signalVector; 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; + } + } + + // must be called on the senders connection data + // assumes the senders and receivers lock are held + void removeConnection(Connection *c) + { + Q_ASSERT(c->receiver); + ConnectionList &connections = connectionsForSignal(c->signal_index); + c->receiver = nullptr; + +#ifndef QT_NO_DEBUG + bool found = false; + for (Connection *cc = connections.first; cc; cc = cc->nextConnectionList) { + if (cc == c) { + found = true; + break; + } + } + Q_ASSERT(found); +#endif + + // remove from the senders linked list + *c->prev = c->next; + if (c->next) + c->next->prev = c->prev; + c->prev = nullptr; + + if (connections.first == c) + connections.first = c->nextConnectionList; + if (connections.last == c) + connections.last = c->prevConnectionList; + + // keep c->nextConnectionList intact, as it might still get accessed by activate + if (c->nextConnectionList) + c->nextConnectionList->prevConnectionList = c->prevConnectionList; + if (c->prevConnectionList) + c->prevConnectionList->nextConnectionList = c->nextConnectionList; + c->prevConnectionList = nullptr; + + Q_ASSERT(c != orphaned.load()); + // add c to orphanedConnections + c->nextInOrphanList = orphaned.load(); + orphaned.store(c); + +#ifndef QT_NO_DEBUG + found = false; + for (Connection *cc = connections.first; cc; cc = cc->nextConnectionList) { + if (cc == c) { + found = true; + break; + } + } + Q_ASSERT(!found); +#endif + + } + void cleanOrphanedConnections(QObject *sender) + { + if (orphaned.load() && ref == 1) + cleanOrphanedConnectionsImpl(sender); + } + void cleanOrphanedConnectionsImpl(QObject *sender); ConnectionList &connectionsForSignal(int signal) { @@ -245,7 +335,6 @@ public: QObjectList senderList() const; void addConnection(int signal, Connection *c); - void cleanConnectionLists(); static QObjectPrivate *get(QObject *o) { return o->d_func(); -- cgit v1.2.3