QProperty: fix threading issues

QObject's cache the binding status pointer to avoid TLS lookups.
However, when an object is moved to a different thread, we need to
update the cached pointer (as the original thread might stop and thus no
longer exist, and to correctly allow setting up bindings in the object's
thread).
Fix this by also storing the binding status in QThreadPrivate and
updating the object's binding status when moved. This does only work
when the thread is already running, though. If it is not running, we
instead treat the QThreadPrivate's status pointer as a pointer to a
vector of pending objects. Once the QThread has been started, we check
if there are pending objects, and update them at this point.

Pick-to: 6.2 6.3
Fixes: QTBUG-101177
Change-Id: I0490bbbdc1a17cb5f85044ad6eb2e1a8c759d4b7
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

1 files changed, 86 insertions, 0 deletions

diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index ef4c6868cf..877896407a 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -103,6 +103,7 @@ private slots:
     void compatPropertySignals();
 
     void noFakeDependencies();
+    void threadSafety();
 
     void bindablePropertyWithInitialization();
     void noDoubleNotification();
@@ -1681,6 +1682,91 @@ void tst_QProperty::noFakeDependencies()
     QCOMPARE(old, bindingFunctionCalled);
 }
 
+struct ThreadSafetyTester : public QObject
+{
+    Q_OBJECT
+
+public:
+    ThreadSafetyTester(QObject *parent = nullptr) : QObject(parent) {}
+
+    Q_INVOKABLE bool hasCorrectStatus() const
+    {
+        return qGetBindingStorage(this)->status({}) == QtPrivate::getBindingStatus({});
+    }
+
+    Q_INVOKABLE bool bindingTest()
+    {
+        QProperty<QString> name(u"inThread"_qs);
+        bindableObjectName().setBinding([&]() -> QString { return name; });
+        name = u"inThreadChanged"_qs;
+        const bool nameChangedCorrectly = objectName() == name;
+        bindableObjectName().takeBinding();
+        return nameChangedCorrectly;
+    }
+};
+
+
+void tst_QProperty::threadSafety()
+{
+    QThread workerThread;
+    auto cleanup = qScopeGuard([&](){
+        QMetaObject::invokeMethod(&workerThread, "quit");
+        workerThread.wait();
+    });
+    QScopedPointer<ThreadSafetyTester> scopedObj1(new ThreadSafetyTester);
+    auto obj1 = scopedObj1.data();
+    auto child1 = new ThreadSafetyTester(obj1);
+    obj1->moveToThread(&workerThread);
+    const auto mainThreadBindingStatus = QtPrivate::getBindingStatus({});
+    QCOMPARE(qGetBindingStorage(child1)->status({}), mainThreadBindingStatus);
+    workerThread.start();
+
+    bool correctStatus = false;
+    bool ok = QMetaObject::invokeMethod(obj1, "hasCorrectStatus", Qt::BlockingQueuedConnection,
+            Q_RETURN_ARG(bool, correctStatus));
+    QVERIFY(ok);
+    QVERIFY(correctStatus);
+
+    bool bindingWorks = false;
+    ok = QMetaObject::invokeMethod(obj1, "bindingTest", Qt::BlockingQueuedConnection,
+            Q_RETURN_ARG(bool, bindingWorks));
+    QVERIFY(ok);
+    QVERIFY(bindingWorks);
+
+    correctStatus = false;
+    ok = QMetaObject::invokeMethod(child1, "hasCorrectStatus", Qt::BlockingQueuedConnection,
+            Q_RETURN_ARG(bool, correctStatus));
+    QVERIFY(ok);
+    QVERIFY(correctStatus);
+
+    QScopedPointer scopedObj2(new ThreadSafetyTester);
+    auto obj2 = scopedObj2.data();
+    QCOMPARE(qGetBindingStorage(obj2)->status({}), mainThreadBindingStatus);
+
+    obj2->setObjectName("moved");
+    QCOMPARE(obj2->objectName(), "moved");
+
+    obj2->moveToThread(&workerThread);
+    correctStatus = false;
+    ok = QMetaObject::invokeMethod(obj2, "hasCorrectStatus", Qt::BlockingQueuedConnection,
+            Q_RETURN_ARG(bool, correctStatus));
+
+    QVERIFY(ok);
+    QVERIFY(correctStatus);
+    // potentially unsafe, but should still work (no writes in owning thread)
+    QCOMPARE(obj2->objectName(), "moved");
+
+
+    QScopedPointer scopedObj3(new ThreadSafetyTester);
+    auto obj3 = scopedObj3.data();
+    obj3->setObjectName("moved");
+    QCOMPARE(obj3->objectName(), "moved");
+    obj3->moveToThread(nullptr);
+    QCOMPARE(obj2->objectName(), "moved");
+    obj3->setObjectName("moved again");
+    QCOMPARE(obj3->objectName(), "moved again");
+}
+
 struct CustomType
 {
     CustomType() = default;