diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2022-06-13 12:16:26 +0200 |
---|---|---|
committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2022-07-14 19:34:46 +0200 |
commit | 2c81ba2df95cc07d5d147c8f3c7999c34848d274 (patch) | |
tree | 6924ccb7737e3d756699e606c91b4f8253f1f764 | |
parent | 6db91c0df1900df2ddcd5abeb243b8852b02b7ab (diff) |
QThread: Clean up bindingStatusOrList if object gets deleted
Deal with the case that the object gets deleted between a call to
moveToThread and the start of the thread by removing the object from the
list in that case.
Fixes: QTBUG-104014
Pick-to: 6.4
Change-Id: Ib249b6e8e8dfbc4d1332bb99a57fa9d3cff16465
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
-rw-r--r-- | src/corelib/kernel/qbindingstorage.h | 1 | ||||
-rw-r--r-- | src/corelib/kernel/qobject.cpp | 10 | ||||
-rw-r--r-- | src/corelib/thread/qthread.cpp | 22 | ||||
-rw-r--r-- | src/corelib/thread/qthread_p.h | 3 | ||||
-rw-r--r-- | tests/auto/corelib/thread/qthread/tst_qthread.cpp | 14 |
5 files changed, 50 insertions, 0 deletions
diff --git a/src/corelib/kernel/qbindingstorage.h b/src/corelib/kernel/qbindingstorage.h index 8baf1b40e1..1c03b23bfa 100644 --- a/src/corelib/kernel/qbindingstorage.h +++ b/src/corelib/kernel/qbindingstorage.h @@ -50,6 +50,7 @@ public: ~QBindingStorage(); bool isEmpty() { return !d; } + bool isValid() const noexcept { return bindingStatus; } const QBindingStatus *status(QtPrivate::QBindingStatusAccessToken) const; diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index e3f686d3de..e07631d001 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -970,6 +970,16 @@ QObject::~QObject() d->wasDeleted = true; d->blockSig = 0; // unblock signals so we always emit destroyed() + if (!d->bindingStorage.isValid()) { + // this might be the case after an incomplete thread-move + // remove this object from the pending list in that case + if (QThread *ownThread = thread()) { + auto *privThread = static_cast<QThreadPrivate *>( + QObjectPrivate::get(ownThread)); + privThread->removeObjectWithPendingBindingStatusChange(this); + } + } + // If we reached this point, we need to clear the binding data // as the corresponding properties are no longer useful d->clearBindingStorage(); diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index 30a5265826..e294fe2fe0 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -605,6 +605,19 @@ QBindingStatus *QtPrivate::BindingStatusOrList::addObjectUnlessAlreadyStatus(QOb return nullptr; } +/*! + \internal + If BindingStatusOrList is a list, remove \a object from it + */ +void QtPrivate::BindingStatusOrList::removeObject(QObject *object) +{ + List *objectList = list(); + if (!objectList) + return; + auto it = std::remove(objectList->begin(), objectList->end(), object); + objectList->erase(it, objectList->end()); +} + QBindingStatus *QThreadPrivate::addObjectWithPendingBindingStatusChange(QObject *obj) { if (auto status = m_statusOrPendingObjects.bindingStatus()) @@ -613,6 +626,15 @@ QBindingStatus *QThreadPrivate::addObjectWithPendingBindingStatusChange(QObject return m_statusOrPendingObjects.addObjectUnlessAlreadyStatus(obj); } +void QThreadPrivate::removeObjectWithPendingBindingStatusChange(QObject *obj) +{ + if (m_statusOrPendingObjects.bindingStatus()) + return; + QMutexLocker lock(&mutex); + m_statusOrPendingObjects.removeObject(obj); +} + + /*! \threadsafe Tells the thread's event loop to exit with a return code. diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 63c518fb57..b647964056 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -108,6 +108,7 @@ public: // requires external synchronization: QBindingStatus *addObjectUnlessAlreadyStatus(QObject *object); + void removeObject(QObject *object); void setStatusAndClearList(QBindingStatus *status) noexcept; @@ -237,6 +238,7 @@ public: if that one has been set in the meantime */ QBindingStatus *addObjectWithPendingBindingStatusChange(QObject *obj); + void removeObjectWithPendingBindingStatusChange(QObject *obj); // manipulating m_statusOrPendingObjects requires mutex to be locked QtPrivate::BindingStatusOrList m_statusOrPendingObjects = {}; @@ -263,6 +265,7 @@ public: QBindingStatus* bindingStatus() { return m_bindingStatus; } QBindingStatus *addObjectWithPendingBindingStatusChange(QObject *) { return nullptr; } + void removeObjectWithPendingBindingStatusChange(QObject *) {} static void setCurrentThread(QThread *) { } static QAbstractEventDispatcher *createEventDispatcher(QThreadData *data); diff --git a/tests/auto/corelib/thread/qthread/tst_qthread.cpp b/tests/auto/corelib/thread/qthread/tst_qthread.cpp index b60f5670dd..1cade32545 100644 --- a/tests/auto/corelib/thread/qthread/tst_qthread.cpp +++ b/tests/auto/corelib/thread/qthread/tst_qthread.cpp @@ -97,6 +97,8 @@ private slots: void terminateAndPrematureDestruction(); void terminateAndDoubleDestruction(); + + void bindingListCleanupAfterDelete(); }; enum { one_minute = 60 * 1000, five_minutes = 5 * one_minute }; @@ -1830,5 +1832,17 @@ void tst_QThread::terminateAndDoubleDestruction() TestObject obj; } +void tst_QThread::bindingListCleanupAfterDelete() +{ + QThread t; + auto optr = std::make_unique<QObject>(); + optr->moveToThread(&t); + auto threadPriv = static_cast<QThreadPrivate *>(QObjectPrivate::get(&t)); + auto list = threadPriv->m_statusOrPendingObjects.list(); + QVERIFY(list); + optr.reset(); + QVERIFY(list->empty()); +} + QTEST_MAIN(tst_QThread) #include "tst_qthread.moc" |