Address thread safety issues in QProperty classes

While we do not support cross-thread bindings, reading of properties
from a different thread when no bindings are involved must continue to
work.
However the check whether bindings are involved used the QBindingStatus
in the QObjectPrivate. That one contains the wrong value when the
QObject is accessed from a different thread than the one it has affinity
with. This patch reads from the thread_local directly instead to
sidetstep the issue.

Change-Id: I8ce2092f35e210566934e2439beb5d48fd8cf226
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

3 files changed, 20 insertions, 6 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index ad25492283..1c642562a8 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -1830,11 +1830,14 @@ void QBindingStorage::maybeUpdateBindingAndRegister_helper(const QUntypedPropert
 void QBindingStorage::registerDependency_helper(const QUntypedPropertyData *data) const
 {
     Q_ASSERT(bindingStatus);
+    // Use ::bindingStatus to get the binding from TLS. This is required, so that reads from
+    // another thread do not register as dependencies
+    auto *currentBinding = QT_PREPEND_NAMESPACE(bindingStatus).currentlyEvaluatingBinding;
     QUntypedPropertyData *dd = const_cast<QUntypedPropertyData *>(data);
-    auto storage = QBindingStoragePrivate(d).get(dd, /*create=*/ bindingStatus->currentlyEvaluatingBinding != nullptr);
+    auto storage = QBindingStoragePrivate(d).get(dd, /*create=*/ currentBinding != nullptr);
     if (!storage)
         return;
-    storage->registerWithCurrentlyEvaluatingBinding(bindingStatus->currentlyEvaluatingBinding);
+    storage->registerWithCurrentlyEvaluatingBinding(currentBinding);
 }
 
 
@@ -1874,6 +1877,13 @@ bool isAnyBindingEvaluating()
 {
     return bindingStatus.currentlyEvaluatingBinding != nullptr;
 }
+
+bool isPropertyInBindingWrapper(const QUntypedPropertyData *property)
+{
+    return bindingStatus.currentCompatProperty &&
+           bindingStatus.currentCompatProperty->property == property;
+}
+
 } // namespace QtPrivate end
 
 QT_END_NAMESPACE
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index 40b7a9452d..1c557fbace 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -845,7 +845,7 @@ public:
 
     void registerDependency(const QUntypedPropertyData *data) const
     {
-        if (!d && !bindingStatus->currentlyEvaluatingBinding)
+        if (!bindingStatus->currentlyEvaluatingBinding)
             return;
         registerDependency_helper(data);
     }
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index 00978e8ea6..7ac02cb43d 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -368,6 +368,10 @@ inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() con
     return { reinterpret_cast<QPropertyObserver *>(ptr->d_ptr) };
 }
 
+namespace QtPrivate {
+    Q_CORE_EXPORT bool isPropertyInBindingWrapper(const QUntypedPropertyData *property);
+}
+
 template<typename Class, typename T, auto Offset, auto Setter, auto Signal=nullptr>
 class QObjectCompatProperty : public QPropertyData<T>
 {
@@ -397,10 +401,10 @@ class QObjectCompatProperty : public QPropertyData<T>
         (thisData->owner()->*Setter)(copy.valueBypassingBindings());
         return true;
     }
-    inline bool inBindingWrapper(const QBindingStorage *storage) const
+    bool inBindingWrapper(const QBindingStorage *storage) const
     {
-        return storage->bindingStatus->currentCompatProperty &&
-               storage->bindingStatus->currentCompatProperty->property == this;
+        return storage->bindingStatus->currentCompatProperty
+            && QtPrivate::isPropertyInBindingWrapper(this);
     }
 
 public: