From ef7cda00dd64acca3a5fa48ee3cb129c781506f2 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 17 Jun 2022 12:36:21 +0200 Subject: [PATCH] Avoid misleading bindingStatus Set it to nullptr on clear, and deal with possibly null bindingStatus. Task-number: QTBUG-101177 Task-number: QTBUG-102403 Change-Id: I66cb4d505a4f7b377dc90b45ac13834fca19d399 Reviewed-by: Fabian Kosmale Reviewed-by: Qt CI Bot (cherry picked from commit 0bd287627508c61a7abfd6430d8c1243ea153081) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/kernel/qbindingstorage.h | 2 +- src/corelib/kernel/qobject.cpp | 2 +- src/corelib/kernel/qproperty.cpp | 1 + src/corelib/kernel/qproperty_p.h | 4 +- .../kernel/qproperty/tst_qproperty.cpp | 38 ++++++++++++++++++- 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/corelib/kernel/qbindingstorage.h b/src/corelib/kernel/qbindingstorage.h index d8853fa265f..8baf1b40e15 100644 --- a/src/corelib/kernel/qbindingstorage.h +++ b/src/corelib/kernel/qbindingstorage.h @@ -55,7 +55,7 @@ public: void registerDependency(const QUntypedPropertyData *data) const { - if (!bindingStatus->currentlyEvaluatingBinding) + if (!bindingStatus || !bindingStatus->currentlyEvaluatingBinding) return; registerDependency_helper(data); } diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 05a881b63c2..ebdc9e43f0f 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -1873,9 +1873,9 @@ void QObjectPrivate::moveToThread_helper() Q_Q(QObject); QEvent e(QEvent::ThreadChange); QCoreApplication::sendEvent(q, &e); + bindingStorage.clear(); for (int i = 0; i < children.size(); ++i) { QObject *child = children.at(i); - child->d_func()->bindingStorage.clear(); child->d_func()->moveToThread_helper(); } } diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 55e30c3069a..482ed3bf350 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -2193,6 +2193,7 @@ void QBindingStorage::clear() { QBindingStoragePrivate(d).destroy(); d = nullptr; + bindingStatus = nullptr; } void QBindingStorage::registerDependency_helper(const QUntypedPropertyData *data) const diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 00e811f4c3f..a569c172c51 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -442,7 +442,7 @@ class QObjectCompatProperty : public QPropertyData } bool inBindingWrapper(const QBindingStorage *storage) const { - return storage->bindingStatus->currentCompatProperty + return storage->bindingStatus && storage->bindingStatus->currentCompatProperty && QtPrivate::isPropertyInBindingWrapper(this); } @@ -467,7 +467,7 @@ public: { const QBindingStorage *storage = qGetBindingStorage(owner()); // make sure we don't register this binding as a dependency to itself - if (storage->bindingStatus->currentlyEvaluatingBinding && !inBindingWrapper(storage)) + if (storage->bindingStatus && storage->bindingStatus->currentlyEvaluatingBinding && !inBindingWrapper(storage)) storage->registerDependency_helper(this); return this->val; } diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp index 92de7725e4d..c0471890ee8 100644 --- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp +++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp @@ -79,6 +79,7 @@ private slots: void noFakeDependencies(); void threadSafety(); + void threadSafety2(); void bindablePropertyWithInitialization(); void noDoubleNotification(); @@ -1694,7 +1695,7 @@ void tst_QProperty::threadSafety() auto child1 = new ThreadSafetyTester(obj1); obj1->moveToThread(&workerThread); const auto mainThreadBindingStatus = QtPrivate::getBindingStatus({}); - QCOMPARE(qGetBindingStorage(child1)->status({}), mainThreadBindingStatus); + QCOMPARE(qGetBindingStorage(child1)->status({}), nullptr); workerThread.start(); bool correctStatus = false; @@ -1743,6 +1744,41 @@ void tst_QProperty::threadSafety() QCOMPARE(obj3->objectName(), "moved again"); } +class QPropertyUsingThread : public QThread +{ +public: + QPropertyUsingThread(QObject **dest, QThread *destThread) : dest(dest), destThread(destThread) {} + void run() override + { + scopedObj1.reset(new ThreadSafetyTester()); + scopedObj1->setObjectName("test"); + QObject *child = new ThreadSafetyTester(scopedObj1.get()); + child->setObjectName("child"); + exec(); + scopedObj1->moveToThread(destThread); + *dest = scopedObj1.release(); + } + std::unique_ptr scopedObj1; + QObject **dest; + QThread *destThread; +}; + +void tst_QProperty::threadSafety2() +{ + std::unique_ptr movedObj; + { + QObject *tmp = nullptr; + QPropertyUsingThread workerThread(&tmp, QThread::currentThread()); + workerThread.start(); + workerThread.quit(); + workerThread.wait(); + movedObj.reset(tmp); + } + + QCOMPARE(movedObj->objectName(), "test"); + QCOMPARE(movedObj->children().first()->objectName(), "child"); +} + struct CustomType { CustomType() = default;