1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid referring to invalidated observer during binding evaluation

Browse Source 
The property observer reference used during binding evaluation can
become invalidated as the container used to maintain the
the observer would invalidate the existing observers during its
resize, leading to referring to invalid memory.

Recognize that we do not need to store the observer, but only its
binding - the observer itself is not used later.

Therefore, change the PendingBindingObserverList to become a list of
QPropertyBindingPrivatePtr to avoid the issue.

Fixes: QTBUG-127596
Original-patch-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
Pick-to: 6.5 6.2
Change-Id: I8c97721ca563083d6280194175f8a915dac2ff04
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
(cherry picked from commit 275202215376238bf414d5008d3c727085e58a4c)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Santhosh Kumar 2024-09-25 17:18:40 +02:00 committed by Qt Cherry-pick Bot
parent c60505140d
commit be974c4855
4 changed files with 44 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/corelib/kernel/qproperty.cpp
View File

@ -226,8 +226,8 @@ void Qt::endPropertyUpdateGroup()
data = data->next;
}
// notify all delayed notifications from binding evaluation
for (const QBindingObserverPtr &observer: bindingObservers) {
QPropertyBindingPrivate *binding = observer.binding();
for (const auto &bindingPtr: bindingObservers) {
auto *binding = static_cast<QPropertyBindingPrivate *>(bindingPtr.get());
binding->notifyNonRecursive();
}
// do the same for properties which only have observers
@ -324,8 +324,9 @@ bool QPropertyBindingPrivate::evaluateRecursive(PendingBindingObserverList &bind
void QPropertyBindingPrivate::notifyNonRecursive(const PendingBindingObserverList &bindingObservers)
{
notifyNonRecursive();
for (auto &&bindingObserver: bindingObservers) {
bindingObserver.binding()->notifyNonRecursive();
for (auto &&bindingPtr: bindingObservers) {
auto *binding = static_cast<QPropertyBindingPrivate *>(bindingPtr.get());
binding->notifyNonRecursive();
}
}
@ -639,8 +640,10 @@ void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr
d = QPropertyBindingDataPointer {storage->bindingData(propertyDataPtr)};
if (QPropertyObserverPointer observer = d.firstObserver())
observer.notify(propertyDataPtr);
for (auto &&bindingObserver: bindingObservers)
bindingObserver.binding()->notifyNonRecursive();
for (auto &&bindingPtr: bindingObservers) {
auto *binding = static_cast<QPropertyBindingPrivate *>(bindingPtr.get());
binding->notifyNonRecursive();
}
}
}
}
@ -805,9 +808,11 @@ void QPropertyObserverPointer::evaluateBindings(PendingBindingObserverList &bind
if (QPropertyObserver::ObserverTag(observer->next.tag()) == QPropertyObserver::ObserverNotifiesBinding) {
auto bindingToEvaluate = observer->binding;
QPropertyObserverNodeProtector protector(observer);
QBindingObserverPtr bindingObserver(observer); // binding must not be gone after evaluateRecursive_inline
if (bindingToEvaluate->evaluateRecursive_inline(bindingObservers, status))
bindingObservers.push_back(std::move(bindingObserver));
// binding must not be gone after evaluateRecursive_inline
QPropertyBindingPrivatePtr currentBinding(observer->binding);
const bool evalStatus = bindingToEvaluate->evaluateRecursive_inline(bindingObservers, status);
if (evalStatus)
bindingObservers.push_back(std::move(currentBinding));
next = protector.next();
}

8
src/corelib/kernel/qproperty_p.h
View File

@ -59,7 +59,7 @@ public:
inline QPropertyObserver *operator ->();
};
using PendingBindingObserverList = QVarLengthArray<QBindingObserverPtr>;
using PendingBindingObserverList = QVarLengthArray<QPropertyBindingPrivatePtr>;
// Keep all classes related to QProperty in one compilation unit. Performance of this code is crucial and
// we need to allow the compiler to inline where it makes sense.
@ -665,8 +665,10 @@ public:
// evaluateBindings() can trash the observers. We need to re-fetch here.
if (QPropertyObserverPointer observer = d.firstObserver())
observer.notify(this);
for (auto&& bindingObserver: bindingObservers)
bindingObserver.binding()->notifyNonRecursive();
for (auto&& bindingPtr: bindingObservers) {
auto *binding = static_cast<QPropertyBindingPrivate *>(bindingPtr.get());
binding->notifyNonRecursive();
}
}
}
}

4
src/corelib/kernel/qpropertyprivate.h
View File

@ -30,8 +30,8 @@ class QBindingStorage;
template<typename Class, typename T, auto Offset, auto Setter, auto Signal, auto Getter>
class QObjectCompatProperty;
struct QBindingObserverPtr;
using PendingBindingObserverList = QVarLengthArray<QBindingObserverPtr>;
class QPropertyBindingPrivatePtr;
using PendingBindingObserverList = QVarLengthArray<QPropertyBindingPrivatePtr>;
namespace QtPrivate {
// QPropertyBindingPrivatePtr operates on a RefCountingMixin solely so that we can inline

23
tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
View File

@ -34,6 +34,7 @@ private slots:
void basicDependencies();
void multipleDependencies();
void bindingWithDeletedDependency();
void bindingWithInvalidatedPropertyObserver();
void dependencyChangeDuringDestruction();
void recursiveDependency();
void bindingAfterUse();
@ -252,6 +253,28 @@ void tst_QProperty::bindingWithDeletedDependency()
QCOMPARE(propertySelector.value(), staticProperty.value());
}
void tst_QProperty::bindingWithInvalidatedPropertyObserver()
{
QProperty<bool> dynamicProperty1(false);
QProperty<bool> dynamicProperty2(false);
QProperty<bool> dynamicProperty3(false);
QProperty<bool> dynamicProperty4(false);
QProperty<bool> dynamicProperty5(false);
QProperty<bool> dynamicProperty6(false);
QProperty<bool> propertySelector([&]{
return (dynamicProperty1.value() && dynamicProperty2.value() && dynamicProperty3.value() &&
dynamicProperty4.value() && dynamicProperty5.value() && dynamicProperty6.value());
});
dynamicProperty1 = true;
dynamicProperty2 = true;
dynamicProperty3 = true;
dynamicProperty4 = true;
dynamicProperty5 = true;
QCOMPARE(propertySelector.value(), false);
dynamicProperty6 = true;
QCOMPARE(propertySelector.value(), true);
}
class ChangeDuringDtorTester : public QObject
{
Q_OBJECT