diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2021-06-08 17:47:46 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2021-06-17 09:27:43 +0000 |
commit | b6323f0c7ba24a4ad5daa3535fcc9d6b285cfe28 (patch) | |
tree | 75b7863d5d5f9710f3539d44a79ffb1fe8e388ea | |
parent | 32eb742b04e57a71183524dc7e9aa51e262b95d7 (diff) |
cleanOrphanedConnectionsImpl: Allow to skip locking
This function is/will be used in a few places where we already have a
lock. Temporarily unlocking and relocking invites all kinds of troubles.
By adding a flag we can instead tell the function that we already hold
the lock.
Change-Id: Ibca089de61133661d5cd75290f2a55c22c5d013c
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
(cherry picked from commit 556fc646cfac4dca43a34f5c4a4f7e6e3ef9104d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/corelib/kernel/qobject.cpp | 19 | ||||
-rw-r--r-- | src/corelib/kernel/qobject_p.h | 13 |
2 files changed, 26 insertions, 6 deletions
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index ea706b93a0..33afcaaa7e 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -70,6 +70,7 @@ #include <qtcore_tracepoints_p.h> #include <new> +#include <mutex> #include <ctype.h> #include <limits.h> @@ -405,11 +406,14 @@ void QObjectPrivate::ConnectionData::removeConnection(QObjectPrivate::Connection } -void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender) +void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy) { + QBasicMutex *senderMutex = signalSlotLock(sender); ConnectionOrSignalVector *c = nullptr; { - QBasicMutexLocker l(signalSlotLock(sender)); + std::unique_lock<QBasicMutex> lock(*senderMutex, std::defer_lock_t{}); + if (lockPolicy == NeedToLock) + lock.lock(); if (ref.loadAcquire() > 1) return; @@ -419,7 +423,16 @@ void QObjectPrivate::ConnectionData::cleanOrphanedConnectionsImpl(QObject *sende c = orphaned.loadRelaxed(); orphaned.storeRelaxed(nullptr); } - deleteOrphaned(c); + if (c) { + // Deleting c might run arbitrary user code, so we must not hold the lock + if (lockPolicy == AlreadyLockedAndTemporarilyReleasingLock) { + senderMutex->unlock(); + deleteOrphaned(c); + senderMutex->lock(); + } else { + deleteOrphaned(c); + } + } } void QObjectPrivate::ConnectionData::deleteOrphaned(QObjectPrivate::ConnectionOrSignalVector *o) diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index 4483e2b6ce..8dd29e05ce 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -266,12 +266,19 @@ public: // must be called on the senders connection data // assumes the senders and receivers lock are held void removeConnection(Connection *c); - void cleanOrphanedConnections(QObject *sender) + enum LockPolicy { + NeedToLock, + // Beware that we need to temporarily release the lock + // and thus calling code must carefully consider whether + // invariants still hold. + AlreadyLockedAndTemporarilyReleasingLock + }; + void cleanOrphanedConnections(QObject *sender, LockPolicy lockPolicy = NeedToLock) { if (orphaned.loadRelaxed() && ref.loadAcquire() == 1) - cleanOrphanedConnectionsImpl(sender); + cleanOrphanedConnectionsImpl(sender, lockPolicy); } - void cleanOrphanedConnectionsImpl(QObject *sender); + void cleanOrphanedConnectionsImpl(QObject *sender, LockPolicy lockPolicy); ConnectionList &connectionsForSignal(int signal) { |