From 7ddba3c23fc7f656dba79be1109ed16711b9353b Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Fri, 3 Feb 2023 14:35:55 +0100 Subject: [PATCH] Fix proxy-data handling This addresses two different issues: - Firstly, we were casting the resolved binding data pointer to QPropertyProxyBindingData, instead of the d_ptr of QPropertyBindingData. Fix this by introducing a helper function, and consistently using it to access the proxy data. - Secondly, we were not resetting the originalBindingData when the pointed to object was destoyed. Fix that, too. Task-number: QTBUG-110899 Change-Id: I7691c9df5cc26e761f6b0e5f16d152f7f2183208 Reviewed-by: Ulf Hermann (cherry picked from commit 55ca636180db2b7870b5b519c4163487b672a9f1) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/kernel/qproperty.cpp | 2 ++ src/corelib/kernel/qproperty_p.h | 18 ++++++++-- src/corelib/kernel/qpropertyprivate.h | 7 +++- .../kernel/qproperty/tst_qproperty.cpp | 34 +++++++++++++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 5269b5d18f9..b8b5c0bdc79 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -445,6 +445,8 @@ QMetaType QUntypedPropertyBinding::valueMetaType() const QPropertyBindingData::~QPropertyBindingData() { QPropertyBindingDataPointer d{this}; + if (isNotificationDelayed()) + proxyData()->originalBindingData = nullptr; for (auto observer = d.firstObserver(); observer;) { auto next = observer.nextObserver(); observer.unlink(); diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 9c32a63fc3e..8e747b4f647 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -81,6 +81,7 @@ struct QPropertyBindingDataPointer void Q_ALWAYS_INLINE addObserver(QPropertyObserver *observer); inline void setFirstObserver(QPropertyObserver *observer); inline QPropertyObserverPointer firstObserver() const; + static QPropertyProxyBindingData *proxyData(QtPrivate::QPropertyBindingData *ptr); inline int observerCount() const; @@ -423,9 +424,9 @@ inline void QPropertyBindingDataPointer::fixupAfterMove(QtPrivate::QPropertyBind { auto &d = ptr->d_ref(); if (ptr->isNotificationDelayed()) { - QPropertyProxyBindingData *proxyData - = reinterpret_cast(d & ~QtPrivate::QPropertyBindingData::BindingBit); - proxyData->originalBindingData = ptr; + QPropertyProxyBindingData *proxy = ptr->proxyData(); + Q_ASSERT(proxy); + proxy->originalBindingData = ptr; } // If QPropertyBindingData has been moved, and it has an observer // we have to adjust the firstObserver's prev pointer to point to @@ -443,6 +444,17 @@ inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() con return { reinterpret_cast(ptr->d()) }; } +/*! + \internal + Returns the proxy data of \a ptr, or \c nullptr if \a ptr has no delayed notification + */ +inline QPropertyProxyBindingData *QPropertyBindingDataPointer::proxyData(QtPrivate::QPropertyBindingData *ptr) +{ + if (!ptr->isNotificationDelayed()) + return nullptr; + return ptr->proxyData(); +} + inline int QPropertyBindingDataPointer::observerCount() const { int count = 0; diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index 1356a3702ec..aec10e1994d 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -304,10 +304,15 @@ private: { quintptr &d = d_ptr; if (isNotificationDelayed()) - return reinterpret_cast(d_ptr & ~(BindingBit|DelayedNotificationBit))->d_ptr; + return proxyData()->d_ptr; return d; } quintptr d() const { return d_ref(); } + QPropertyProxyBindingData *proxyData() const + { + Q_ASSERT(isNotificationDelayed()); + return reinterpret_cast(d_ptr & ~(BindingBit|DelayedNotificationBit)); + } void registerWithCurrentlyEvaluatingBinding_helper(BindingEvaluationState *currentBinding) const; void removeBinding_helper(); diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp index 216a10bd574..97dda5f437c 100644 --- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp +++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #if __has_include() && __cplusplus >= 202002L && !defined(Q_CLANG_QDOC) #include @@ -85,6 +86,7 @@ private slots: void noDoubleNotification(); void groupedNotifications(); void groupedNotificationConsistency(); + void bindingGroupMovingBindingData(); void uninstalledBindingDoesNotEvaluate(); void notify(); @@ -1942,6 +1944,38 @@ void tst_QProperty::groupedNotificationConsistency() QVERIFY(areEqual); // value changed runs after everything has been evaluated } +void tst_QProperty::bindingGroupMovingBindingData() +{ + auto tester = std::make_unique(); + auto testerPriv = QObjectPrivate::get(tester.get()); + + auto dummyNotifier = tester->property.addNotifier([](){}); + auto bindingData = testerPriv->bindingStorage.bindingData(&tester->property); + QVERIFY(bindingData); // we have a notifier, so there should be binding data + + Qt::beginPropertyUpdateGroup(); + auto cleanup = qScopeGuard([](){ Qt::endPropertyUpdateGroup(); }); + tester->property = 42; + QCOMPARE(testerPriv->bindingStorage.bindingData(&tester->property), bindingData); + auto proxyData = QPropertyBindingDataPointer::proxyData(bindingData); + // as we've modified the property, we now should have a proxy for the delayed notification + QVERIFY(proxyData); + // trigger binding data reallocation + std::array propertyDataArray; + for (auto&& data: propertyDataArray) + testerPriv->bindingStorage.bindingData(&data, true); + // binding data has moved + QVERIFY(testerPriv->bindingStorage.bindingData(&tester->property) != bindingData); + bindingData = testerPriv->bindingStorage.bindingData(&tester->property); + // the proxy data has been updated + QCOMPARE(proxyData->originalBindingData, bindingData); + + tester.reset(); + // the property data is gone, proxyData should have been informed + QCOMPARE(proxyData->originalBindingData, nullptr); + QVERIFY(proxyData); +} + void tst_QProperty::uninstalledBindingDoesNotEvaluate() { QProperty i;