Avoid capturing same property twice

Avoid capturing the same property multiple times in a binding by
storing them in the BindingEvaluationState. We store them in a
QVarLengthArray array, as the number of properties involved in a binding
is expected to be rather low, so a linear scan is fine.

Avoiding double capture is a good idea in general, as we would otherwise
needlessly reevaluate bindings multiple times, and also needlessly
allocate memory for further observers, instead of using a binding's
inline observer array.

Even more importantantly, our notification code makes assumptions that
notify will visit bindings only exactly once. Not upholding that
invariant leads to memory corruption and subsequent crashes, as
observers allocated by the binding would get freed, even though we would
still access them later.

Fixes: QTBUG-112822
Pick-to: 6.5 6.2
Change-Id: Icdc1f43fe554df6fa69e881872b2c429d5fa0bbc
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -603,6 +603,11 @@ void QPropertyBindingData::registerWithCurrentlyEvaluatingBinding_helper(Binding
 {
     QPropertyBindingDataPointer d{this};
 
+    if (currentState->alreadyCaptureProperties.contains(this))
+        return;
+    else
+        currentState->alreadyCaptureProperties.push_back(this);
+
     QPropertyObserverPointer dependencyObserver = currentState->binding->allocateDependencyObserver();
     Q_ASSERT(QPropertyObserver::ObserverNotifiesBinding == 0);
     dependencyObserver.setBindingToNotify_unsafe(currentState->binding);

src/corelib/kernel/qproperty_p.h

@@ -195,6 +195,7 @@ struct BindingEvaluationState
     QPropertyBindingPrivate *binding;
     BindingEvaluationState *previousState = nullptr;
     BindingEvaluationState **currentState = nullptr;
+    QVarLengthArray<const QPropertyBindingData *, 8> alreadyCaptureProperties;
 };
 
 /*!

tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp

@@ -76,6 +76,7 @@ private slots:
     void metaProperty();
 
     void modifyObserverListWhileIterating();
+    void noDoubleCapture();
     void compatPropertyNoDobuleNotification();
     void compatPropertySignals();
 
@@ -1493,6 +1494,22 @@ void tst_QProperty::modifyObserverListWhileIterating()
     }
 }
 
+void tst_QProperty::noDoubleCapture()
+{
+    QProperty<long long> size;
+    size = 3;
+    QProperty<int> max;
+    max.setBinding([&size]() -> int {
+        // each loop run attempts to capture size
+        for (int i = 0; i < size; ++i) {}
+        return size.value();
+    });
+    auto bindingPriv = QPropertyBindingPrivate::get(max.binding());
+    QCOMPARE(bindingPriv->dependencyObserverCount, 1);
+    size = 4; // should not crash
+    QCOMPARE(max.value(), 4);
+}
+
 class CompatPropertyTester : public QObject
 {
     Q_OBJECT