From 2c81ba2df95cc07d5d147c8f3c7999c34848d274 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 13 Jun 2022 12:16:26 +0200
Subject: [PATCH] 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>
---
 src/corelib/kernel/qbindingstorage.h          |  1 +
 src/corelib/kernel/qobject.cpp                | 10 +++++++++
 src/corelib/thread/qthread.cpp                | 22 +++++++++++++++++++
 src/corelib/thread/qthread_p.h                |  3 +++
 .../corelib/thread/qthread/tst_qthread.cpp    | 14 ++++++++++++
 5 files changed, 50 insertions(+)

diff --git a/src/corelib/kernel/qbindingstorage.h b/src/corelib/kernel/qbindingstorage.h
index 8baf1b40e15..1c03b23bfae 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 e3f686d3dea..e07631d0015 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 30a52658264..e294fe2fe04 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 63c518fb571..b6479640569 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 b60f5670dd9..1cade32545b 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"